Treat a rejected refresh token as an auth failure and say what to run

Merged
JE
2mo ago
+633-19

What

  • A refresh the server refuses with invalid_grant (refresh token expired, revoked, or reused) is now an auth_required error (exit 3) that says what to run, instead of a generic api_error (exit 7) carrying the raw token error: invalid_grant - … string.
  • That dead credential is deleted from the store on the spot. Basecamp's abuse tracker bans the client and IP after five invalid_grant failures in 48 hours, and until now every later command re-tried the same dead refresh token, so one expired session could turn into a lockout. Only invalid_grant deletes: invalid_request, network faults and 5xx keep the credential and their existing error classes.
  • Every auth_required error that reaches the user carries a remedy. output.AsError and convertSDKError now default an empty hint to Run: basecamp auth login (a 401 from the API arrives with none), and every auth error the credential manager raises names the active profile: Run: basecamp auth login -P <profile>.
  • Regression test pinning that auth refresh preserves user_id, user_email, scope and resource across a token rotation.

The invalid_grant detection matches the SDK's untyped error message (token error: invalid_grant); the SDK's exchanger returns token-endpoint errors as plain fmt.Errorf values, so the OAuth error code is recoverable only from the text. That coupling is named in a comment next to the constant; a re-pin that types the error can replace the match.

Why

Smoke-test finding "Refresh with a rejected refresh token exits 7 api_error instead of 3 auth_required": after a refresh token is revoked server-side, basecamp auth refresh and every ordinary command failed with exit 7, no hint, and the credential left on disk to fail again next time. Scripts keyed on exit 3 to trigger a re-login never saw it, and a person saw an OAuth wire string with nothing to do about it.

Before / After

Before:

$ basecamp auth refresh -P work
Error: token refresh failed: token error: invalid_grant - The refresh token has been revoked
$ echo $?
7
$ basecamp projects list -P work
Error: token refresh failed: token error: invalid_grant - The refresh token has been revoked

After:

$ basecamp auth refresh -P work
Error: Your session has expired or was revoked (The refresh token has been revoked)
Hint: Run: basecamp auth login -P work
$ echo $?
3
$ basecamp projects list -P work
Error: Not authenticated for profile:work: credentials not found for profile:work
Hint: Run: basecamp auth login -P work

A 401 on an ordinary command:

$ basecamp projects list --json
{"ok":false,"error":"authentication required","code":"auth_required","retryable":false,"hint":"Run: basecamp auth login"}

Borrowed from Codex

Codex maps refresh failures to plain sentences ("Your access token could not be refreshed because your refresh token has expired. Please log out and sign in again."); the wording here does the same and adds the exact command, profile included.

Testing

  • internal/auth: TestRefresh_InvalidGrantForgetsTheCredential, TestRefresh_InvalidGrantWithoutDescription, TestRefresh_InvalidRequestKeepsTheCredential, TestAccessToken_InvalidGrantHintsTheProfile, TestRefresh_PreservesIdentityAndBinding (httptest token endpoints; no network).
  • internal/commands: TestProjectsList401CarriesTheLoginHint (401 transport, hint asserted on the converted error and in the rendered JSON envelope).
  • TestRefreshLocked_LegacyBC3RequiresReauth now asserts the typed code and hint instead of the old inline "re-authenticate" wording.
  • Gates: make fmt-check vet lint, go test -tags dev ./internal/..., make check-surface check-skill-drift, make check.

Summary by cubic

Refresh refused with invalid_grant is now an auth_required error (exit 3) that says the session expired and what to run, instead of a generic api_error (exit 7); the refused credential is deleted on the spot so later commands stop re-trying it and one expired session can't accumulate into a Basecamp abuse-tracker lockout. The delete only touches a BC5 credential while the store still holds the exact refused token, so a concurrent rotation's fresh credential is kept and the refresh succeeds by proxy — all other refusals keep the credential and their existing error classes.

Bug Fixes

  • Every auth error carries a remedy: API 401s get the active profile's login (Run: basecamp auth login -P <profile>), or a BASECAMP_TOKEN note when set; stored-credential commands that ignore the env token keep their profile-named hint.
  • The refresh's other auth failures — unsafe stored endpoint, missing token endpoint, half-configured OAuth client, removed bc3 flow — now name that profile too and drop the old inline "re-authenticate" wording.
  • Profile names are shell-quoted in the remedy so a pasted command can't select the wrong profile or run something unexpected.
  • A command's own note appended to the default hint (a partial reorder's rerun guidance) survives; the remedy replaces only the login prefix.
  • Token rotation preserves user_id, user_email, scope, and resource.
  • A Launchpad refusal keeps the credential and its message names both readings — a dead grant, or BASECAMP_OAUTH_CLIENT_ID/SECRET naming a different client — instead of declaring the session over.
  • invalid_grant detection matches the SDK's untyped error message, documented in a comment so a later typed SDK error can replace the string match.