feat: oauth authentication#320
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: This PR introduces a new OAuth authentication flow with PKCE, token refresh, and credential migration across multiple commands and core libraries, which is a high-risk change to critical authentication logic that requires human review for security, correctness, and integration with existing API key
Re-trigger cubic
There was a problem hiding this comment.
7 issues found
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 6 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
5 issues found across 11 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Implements OAuth authentication with PKCE flow, token refresh, and secure credential storage across the CLI. This is a major architectural change affecting auth, credential management, and API client construction.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Adds OAuth authentication, a major new feature affecting core login, credential storage, and multiple commands. High risk; requires human review.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
Felipe review (#1): getJwtExp cast the decoded payload without guards, so a malformed access token would throw a cryptic error or store NaN/undefined as the expiry, leaving the credential in an inconsistent state. Validate segment count, JSON payload, and numeric exp, throwing a clear re-login message.
Felipe review (#6): both refreshOAuthGrant and exchangeAuthorizationCode cast the JSON body with `as` after only checking response.ok. A 200 with an unexpected body (proxy error page, partial payload) would be persisted as a grant. Add parseTokenResponse() to validate the required fields and reuse it in both token calls.
Felipe review (#8): the refresh and exchange fetches had no timeout, so a hung connection would make the CLI wait forever. Route both through a shared fetchOAuthToken helper that aborts after 30s (AbortSignal.timeout) and maps TimeoutError / network failures to clear, actionable messages.
Felipe review (#7): the loopback callback always rendered "Authentication complete", even when the provider returned ?error= or omitted code/state. Render a distinct "Authentication failed" page in those cases so the user isn't told success while the terminal reports failure. Promise resolve/reject logic is unchanged.
Felipe review (#4): storeOAuthGrant wrote the entire grant (access_token AND refresh_token) into credentials.json in plaintext, never touching the keychain. Mirror storeApiKeyAsync: when a secure backend is available, persist the grant as a JSON blob in the OS keychain and keep only non-secret metadata ({type, scope}) in the file; fall back to inline plaintext only when no secure backend exists. Split the type into OAuthGrantData (the full grant, in memory / keychain / file fallback) and OAuthGrant (the file entry, token fields optional). resolveAuthentication now reads the grant from the keychain blob when secure and validates it.
Felipe review (#5): migrateRawProfile cast oauth_grant entries with `as` and no validation, so a hand-edited or corrupted file would surface inconsistent state later. Require a string scope, and require token fields (when any are present) to form a complete, well-typed grant; throw CorruptedCredentialsError otherwise. Metadata-only secure-storage entries (scope only) remain valid.
Felipe review (#2): doctor's API Key and API Validation checks called resolveAuthentication without catching errors, so an expired/failed OAuth refresh threw an unhandled error and crashed the command instead of reporting status. Wrap both calls and return a failed check with a 'Run: resend login' hint. Also makes the doctor tests hermetic: they previously read the developer's real ~/.config/resend/credentials.json (no XDG_CONFIG_HOME), which the stricter grant validation surfaced. Point them at an isolated config dir.
Felipe review (#3): whoami's help says it is local-only, which is now accurate — it passes { refresh: false } so it reads the grant from keychain/file without a network call. Tighten the help wording to 'active credential' (not 'API key') and label OAuth credentials as 'Token' rather than 'API Key' in the human output. Add a test asserting whoami never calls fetch for an OAuth profile.
Refines #5: throwing CorruptedCredentialsError when an oauth_grant had partial or wrong-typed token fields made one bad profile unreadable for the whole file. Real files written by older clients can carry e.g. refresh_token_expires_at: null. Instead, drop invalid token fields during read so the profile degrades to a re-login while the rest of the file stays usable; the grant is still validated at point-of-use (resolveAuthentication / parseOAuthGrantBlob).
My earlier #6 validation required refresh_token_expires_in and broke login: the /oauth/token response does not contain it. Confirmed against the server source (apps/public-api): token.ts returns { access_token, token_type, expires_in, refresh_token, scope } for both authorization_code and refresh_token grants (token.ts:199-205, 384-388); the refresh-token expiry is stored server-side only (issue-tokens.ts:101-103) and never sent to the client. This is standard OAuth 2.0 — refresh timing is driven by the access-token expiry (JWT exp), and an expired/revoked refresh token is signalled by an invalid_grant error on refresh (token.ts:242-247), not by a client-known timestamp. Changes: - parseTokenResponse: validate access_token, refresh_token, scope (the fields the server actually returns); drop the refresh_token_expires_in requirement. - Remove refresh_token_expires_at from the grant model entirely (OAuthGrantData, storeOAuthGrant, login). The CLI's old 'now + refresh_token_expires_in' computed NaN -> persisted null; that phantom field is gone. - refreshOAuthGrant: no client-side expiry pre-check (unknowable); attempt refresh and surface re-login on a non-OK response. - Tests updated to the real response shape; doctor refresh-failure test now mocks a 400 invalid_grant instead of the removed pre-check.
Tighten the verbose comments added across this branch's OAuth work and drop the PR-review references; keep only the non-obvious rationale (e.g. why there is no client-side refresh-token expiry, why backend.set needs no explicit delete).
The sourceLabel/permissionLabel ternaries were nested 4 levels deep and hard to read. Replace them with Record<ApiKeySource|ApiKeyPermission, string> lookup maps (matching the existing Record<ApiKeyPermission, number> in client.ts), keeping the one real branch (OAuth on a config source) explicit. The Record types also make label coverage exhaustive — adding a new union member now fails to compile instead of silently falling through.
cubic P2: storeOAuthGrant only set storage when the backend was secure, so an inline (no-keychain) fallback over a previously secure_storage file kept the stale 'secure_storage' flag — the file then claimed secure storage while the tokens sat inline, misleading resolveAuthentication and deletion. Set it explicitly per backend.
cubic P1: storeOAuthGrant wrote the secret to the keychain, then the metadata to the file. If the file write failed after the keychain write, the file could still record the profile as api_key while the keychain held an OAuth blob — and resolveAuthentication would return that blob as an invalid API key. Wrap the file write so a failure deletes the just-written secret, keeping keychain and file in agreement (the profile then reads as not-authenticated → re-login).
cubic P2: doctor reported any resolveAuthentication error as a hard fail (exit 1), inconsistent with its other checks that warn on network/timeout. Two changes: - The API Key presence check no longer refreshes (refresh: false) — presence is a local check; the credential is exercised in the validation check instead. A transient network blip can no longer fail the presence check. - Tag timed-out/unreachable token requests (OAUTH_NETWORK_ERROR_NAME) so the validation check can warn on transient failures while still failing hard when the server rejects the session (re-login).
The transient-network doctor test left its fetch spy unrestored (unused-var lint error + a leaked global mock); restore it. Apply biome formatting to the two test files touched in this branch.
…nd reuse helper to avoid drift solutions
3 participants
this will only work with the staging api for now, but that's fine. the cleint id is hard coded, and, for consistency, I think we should ensure this exact client id also used in the production oauth client for the CLI
I've verified this does indeed work with the non-sending apis
Summary by cubic
Adds OAuth authentication to the CLI with a browser PKCE flow, secure token storage, and unified credential resolution across commands while keeping API keys working. Updates
login,whoami,doctor, and client auth to support tokens, scopes, and clearer UX.New Features
resend login: browser-based PKCE with a local loopback callback (state checks), styled success/failure page, 5‑minute timeout, exchanges the code, closes the server, and saves the grant per profile. UsesRESEND_BASE_URLandOAUTH_CLIENT_ID. API key login remains (open API‑keys page or manual entry).credentials.json(per‑profiletypeandstorageset tosecure_storageorfile). Inline fallback when no keychain. Overwrites replace old secrets and roll back on metadata write failure. Reads degrade gracefully if legacy/partial OAuth fields are present.exp./oauth/tokenresponses are schema‑validated and fetched with 30s timeouts; refresh happens on demand with a 60s leeway. Transient network/timeout errors are labeled fordoctor.resolveAuthenticationreturns an API key or an OAuth grant; clients use the right token. OAuth scopes (full_access,emails:send) map to CLI permissions; unknown scopes are blocked.whoamiis local‑only (no refresh), labels OAuth as “Token,” masks values, and shows the source (flag,env,config,secure_storage).doctorchecks presence locally, validates against the API, shows OAuth details, warns on transient refresh failures, and fails only when the server rejects the credential.Migration
credentials.jsonnow stores atypeper profile and OAuth metadata; secrets live in secure storage when available.resend loginand choose “Login with Resend.”Written for commit bd63eeb. Summary will update on new commits.