TW-6922: Add CLI OAuth login (PKCE) - #123
Open
radenkovic wants to merge 5 commits into
Open
radenkovic wants to merge 5 commits into
radenkovic wants to merge 5 commits into
Conversation
Stage 1a of integrating the dashboard-account OAuth 2.1 authorization
server into the CLI: the domain types, the port, and the HTTP adapter.
The adapter does not reuse dashboard.AccountClient because these
endpoints are plain RFC 6749/7009/7591 — no house {"data":...} envelope
and no DPoP proof. It resolves every endpoint from the RFC 8414
discovery document rather than assuming a path, which matters because
dashboard-account builds them all from OAUTH_ISSUER and that is a
different host from the local port in a tunnelled dev setup.
PKCE is generated fresh rather than reusing auth.generatePKCEPair: that
helper computes base64std(hex(sha256(v))) for Nylas hosted auth, and the
authorization server enforces /^[A-Za-z0-9\-_]{43}$/, which only the RFC
form satisfies. A test pins the divergence so the two cannot be merged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage 1b: the app-layer service that runs the browser authorization code flow and owns the stored session. Reuses the existing loopback callback server and browser adapters unchanged. The client is registered dynamically as a public client the first time it is needed, against the redirect URI http://localhost/callback with no port: RFC 8252 lets the server free the port of a loopback URI at request time, so the ephemeral port the callback server picks still matches. The registration is pinned to the issuer that produced it, because a dev tunnel URL changes between sessions and a client_id does not survive it. Refresh handling is the subtle part. The server rotates the refresh token on every use and burns the whole family if a consumed one reappears, so the service persists exactly what came back and never carries the previous refresh token forward to fill an empty field. A test covers that specific mistake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage 1c: the user-facing commands, wired into the root command. Named as its own subtree rather than folded into `nylas auth`, which in this CLI means connecting an end user's mailbox as a provider grant, or `nylas dashboard login`, which opens a dashboard management session. This authenticates the person running the CLI. The authorization server is hosted by dashboard-account, so the base URL resolution is shared rather than duplicated: getDashboardAccountBaseURL is now exported as dashboard.AccountBaseURL and loses a parameter it never read. NYLAS_DASHBOARD_ACCOUNT_URL therefore points both command groups at a local server. `oauth token` prints the bare token so it can be substituted into a curl header, and `oauth status` deliberately never prints the token itself. That file needs `git add -f`: .gitignore has a broad `*token*` rule, and the two token.go files already tracked were added the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage 1d: integration tests that run the real adapter against a running dashboard-account, covering discovery, dynamic registration, the full PKCE code exchange, userinfo, refresh rotation, revocation, and the two rejections that matter (replayed code, mismatched verifier). They seed their own user, consent grant and authorization code through the /dev routes. That is what removes the browser from the loop: the consent screen needs a UAS-connected mailbox, which a local stack does not have. The tests front the server with a proxy that rewrites the issuer origin in the discovery document. dashboard-account builds every advertised endpoint from OAUTH_ISSUER, and locally that is often a tunnel hostname that is stale or unreachable, while the client is spec-correct and follows whatever the document says. The proxy is confined to the test — no workaround leaks into the client. Confirmed live, and worth recording: the server really does burn the whole refresh family when a consumed token is replayed, which is the behaviour the storage rules in oauthlogin were written against. Skips unless NYLAS_OAUTH_AS_URL is set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nylas oauth login,status,token, andlogoutcommandsTest plan
internal/adapters/oauthas,internal/app/oauthlogin,internal/domain/oauth,internal/cli/oauthinternal/cli/integration/oauth_test.go🤖 Generated with Claude Code