feat: device authorization grant login (oauth-device) - #44
Conversation
The deterministic issuer gains a device authorization endpoint, advertises it and the device-code grant in its discovery document, and models the behaviours a device login has to survive: an approval that arrives after several pending polls, a refusal, an expired code, a deployment that asks the client to slow down, and a deployment that offers no device grant at all. Every poll is timestamped. That record is what lets a test assert the shell honoured the interval it was given and backed off when told to — a property that is only real from the deployment's side, and that a shell merely holding the right number in a variable would not satisfy. Refs #42
`wso2 login` could only be finished by someone sitting at a browser on the same machine: the browser mode binds a loopback listener and waits to be redirected back to it, which a developer over SSH or inside a container cannot reach. The `oauth-device` kind was legal configuration that refused at use, so such a user was told the shell knew what they wanted and would not do it. RFC 8628 removes the constraint. The shell prints a short URI and a user code, the approval happens on any other device, and what it leaves behind is the same session a browser login leaves — so nothing downstream changes and no module can tell the two apart. x/oauth2 owns the polling, deliberately: it already implements the rule this most needs right, which is to poll no faster than the deployment asked and to add five seconds for every later request once told to slow down. What this package owns is what no library can — refusing a deployment that does not advertise the grant *before* a code is printed, so nobody carries a code to an approval screen that does not exist. Three decisions worth stating. No new problem codes. The four endings leave the caller in one place, holding no session, so they share auth.credential_unavailable and differ in their sentence — the way identityNotVerified already varies four messages across one code. authorization_pending and slow_down are loop control and reach no user at all. This leaves the question in #40 open rather than answering it in passing. No device source. Deriving module access from a device-established session is identical to deriving it from a browser one at every step, so browserSource becomes sessionSource — named for what it derives from rather than for how the session was established — and both interactive kinds resolve to it. A parallel source would have duplicated the rotation lock and the narrowing proof for no behaviour. A missing identity token does not fail a device login, unlike a browser one. RFC 8628 defines no nonce, whether WSO2 deployments return an identity token from this grant is unmeasured, and the session is the refresh token — so the subject is reported when it was verified and is absent when it was not, rather than letting an unmeasured behaviour decide whether the flow works at all. Refs #42
One seam, the same one the browser login is proven at: `wso2 login` run in process against the deterministic issuer, with the reference module launched as a real subprocess and presenting its token to a real service. Nothing reaches into the flow — what is asserted is the two streams, the exit class, what landed in the secure store, and what the product service was shown. The interval and back-off are read from the issuer's record of when each poll arrived, because "the deployment was not polled faster than it asked" is a fact about the deployment. That test spends real seconds and cannot avoid it: RFC 8628 fixes the back-off at five, and the shell honours the RFC rather than a knob this suite could turn down. Covered: an approved login establishes a session and a module then receives access narrowed to one permission out of four; the code and URI each stand on their own line so they survive being read aloud; neither the device code nor any token material reaches any stream; a declined login and an abandoned one read differently under one code; a deployment without the grant is refused before any code is shown and without a single poll; and a login whose grant returned no identity token still establishes a usable session while claiming no subject. Refs #42
The deterministic suite proves the flow. What a live run adds is evidence about a deployment: that Asgardeo and Identity Server really advertise the endpoint the shell looks for, really answer the polling the way RFC 8628 describes, and really leave a refresh token the broker can narrow. It reads exactly the variables the browser run reads. The only thing it needs that the browser run does not is the device grant enabled on the same application — no registration value is specific to it — and the run exists partly to keep that claim honest. Config gains one field for the kind, defaulting to browser so nothing existing changes. It also reports whether the deployment's device grant returned an identity token. That is unmeasured on both products today, and it is the reason the shell refuses to depend on one. Refs #42
Section 5.1 of the walkthrough covers the device mode end to end: when the kind is the right choice, the one grant to enable, the one word that changes in the context document, and what the terminal actually prints. It is explicit about the gap this slice leaves. `oauth-device` is the kind for an identity that can *only* be established that way; a developer who merely happens to be on a headless machine today needs a second identity, because `wso2 login --device-code` is not in this release. Saying so is better than letting a reader discover it. Troubleshooting gains the four device endings and the missing-grant refusal, and its auth.kind_not_implemented entry narrows to `pat`, which is now the only kind that still refuses. The architecture and requirements notes are corrected to match what ships, and the examples table moves the kind to implemented. CONTEXT.md gains one term. "Login mode" was used across three documents and defined in none, and it is exactly the distinction this slice would otherwise blur with "authentication kind". Refs #42
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary
WalkthroughThis change adds RFC 8628 device authorization login for Sequence Diagram(s)sequenceDiagram
participant User
participant WSO2CLI
participant OAuthProvider
User->>WSO2CLI: run login for oauth-device context
WSO2CLI->>OAuthProvider: discover device authorization metadata
WSO2CLI->>OAuthProvider: request device authorization
OAuthProvider-->>WSO2CLI: verification URI and user code
WSO2CLI-->>User: display verification instructions
User->>OAuthProvider: approve on another device
WSO2CLI->>OAuthProvider: poll for token
OAuthProvider-->>WSO2CLI: access and refresh tokens
WSO2CLI-->>User: report login result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
docs/guides/login.md (1)
576-576: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fenced code block.
markdownlint reports MD040 for this block. Use
textso the terminal transcript renders without syntax highlighting.📝 Proposed documentation change
-``` +```text $ wso2 login🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/login.md` at line 576, Update the fenced code block near the login command in the documentation to specify the text language, using text after the opening fence so markdownlint MD040 is satisfied and the terminal transcript has no syntax highlighting.Source: Linters/SAST tools
internal/auth/source_session_test.go (1)
110-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider naming these tests after the shared session source.
internal/auth/source.gonow routesKindOAuthBrowserandKindOAuthDeviceto the samesessionSource. The test names state "BrowserSource", so they describe a narrower subject than the code under test. A reader may conclude that the device kind is unverified at this layer. Renaming toTestSessionSource...would match the routing, and the browser kind can remain the fixture's kind.Also applies to: 136-136, 154-154, 184-184, 207-207, 298-298
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/auth/source_session_test.go` at line 110, Rename the affected tests in source_session_test.go from the BrowserSource naming to SessionSource, including TestBrowserSourceNarrowsTheSessionToWhatTheModuleAsked and the other referenced tests, while preserving the browser kind in their fixtures and leaving test behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/login.md`:
- Around line 898-908: Move the device-login endings table and its explanatory
paragraph from their current position before the browser identity-token
continuation to after the browser identity-token table, keeping the device
section content unchanged and grouping each login mode’s narrative together.
- Line 467: Update the auth.credentialRef documentation row to state that
credentialRef is required for oauth-device, alongside oauth-browser, while
preserving the existing client-credentials restriction.
In `@internal/auth/fakeissuer/fakeissuer.go`:
- Around line 645-680: Update the device-grant handling around the poll logic
and issueDeviceTokens so an approved grant is marked consumed while holding
i.mutex before tokens are issued. Treat consumed grants like unknown or spent
device codes by returning invalid_grant on subsequent polls, while preserving
the existing pending, denial, expiration, and first successful issuance
behavior.
In `@test/acceptance/login_device_test.go`:
- Around line 337-342: Update the gap assertion in the login device polling test
to require the advertised DeviceInterval plus the five-second slow-down
increment, so with DeviceInterval: 1 the next poll must occur at least six
seconds later. Keep the existing RFC 8628 growth validation and error reporting.
---
Nitpick comments:
In `@docs/guides/login.md`:
- Line 576: Update the fenced code block near the login command in the
documentation to specify the text language, using text after the opening fence
so markdownlint MD040 is satisfied and the terminal transcript has no syntax
highlighting.
In `@internal/auth/source_session_test.go`:
- Line 110: Rename the affected tests in source_session_test.go from the
BrowserSource naming to SessionSource, including
TestBrowserSourceNarrowsTheSessionToWhatTheModuleAsked and the other referenced
tests, while preserving the browser kind in their fixtures and leaving test
behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 908eb18f-1219-4483-ad00-b77cb9188445
📒 Files selected for processing (19)
CONTEXT.mdMakefiledocs/architecture.mddocs/examples/authentication-contexts.mddocs/guides/login.mddocs/product-requirements.mdinternal/app/login.gointernal/app/login_test.gointernal/auth/auth_test.gointernal/auth/fakeissuer/fakeissuer.gointernal/auth/oauthflow/device.gointernal/auth/oauthflow/login.gointernal/auth/source.gointernal/auth/source_session.gointernal/auth/source_session_test.gotest/acceptance/login_device_test.gotest/smoke/config.gotest/smoke/env.exampletest/smoke/login_device_smoke_test.go
Four findings from review on #44, all valid. The fake issuer left an approved device grant redeemable. A real deployment spends the code, this file's own exchangeCode already does, and the fixture is the oracle the device tests read "the session came from one approval" off — one that answered a replay would have let a real double-redemption defect through. The grant is now marked redeemed under the same lock that draws its counters down, and a later poll is answered invalid_grant. Two fixture tests pin it. The back-off assertion required five seconds where the property is the advertised interval plus five. With a 1s interval a client that replaced it with 5s rather than increasing it by 5s would have passed. It now requires 6s, written as advertised+increment so the number explains itself. The credentialRef row named only oauth-browser, though the schema has required it for both interactive kinds since before this branch and section 5.1 says so. The device endings table sat between the browser paragraph and its own continuation, so a reader passed from device endings straight back into browser narrative. It moves below the browser identity-token material, leaving each mode as one block. Also renamed TestBrowserSource* to TestSessionSource*, a leftover from renaming the type they cover. Declined: adding a `text` language to one fenced block for markdownlint MD040. Nothing in this repository runs markdownlint — CI is golangci-lint only — and 25 fenced blocks in that same file already carry no language, so tagging one would be inconsistent rather than correct. Refs #42
|
On the two nitpicks: Test naming — done in aed330d. Fair catch: renaming the type to MD040 on the fenced block — declined. Nothing in this repository runs markdownlint; |
A deployment chooses the polling interval, and x/oauth2 substitutes RFC 8628's default only when the member is exactly zero — every other value reaches time.NewTicker, which panics on a non-positive duration. Two answers get there: a negative interval directly, and one above roughly nine billion seconds by overflowing the conversion to nanoseconds. Both were confirmed against the library. The cost is not tidiness. A panic escapes the problem type: it prints a Go stack trace instead of a refusal and exits outside the class list a script branches on, and nothing in main recovers. This shell renders typed problems, and what a deployment says must not be able to stop it doing that. So the advertised interval is sanitized before any of it is used. A value that is not a wait falls back to the specification's default; one past the ceiling is clamped rather than reinterpreted, because a deployment asking for an enormous wait has asked for something, and polling sooner than it consented to is the abuse RFC 8628 section 3.5 exists to prevent. The ceiling sits above any deadline a login can run under, which an internal test pins so the clamp can never start meaning "poll sooner". Severity is low — it needs a hostile or broken deployment, and the issuer is TLS-verified and named by the user's own context document. It is fixed because the fix is small and because "typed refusal, never a crash" is what this shell promises. Also from the same review pass: An identity token that is present and does not verify is now refused, where before it was reported as no identity at all. Absent and invalid are different answers. Tolerating silence is a judgement about an unmeasured protocol; tolerating a bad signature would be a judgement to stop looking — it means a client identifier naming another application, an issuer that did not sign what it sent, or a clock that disagrees, and each is a fault the user can fix and none is visible if the shell carries on quietly. Not a security boundary either way: an unverified token was never trusted. And the test gaps the review found: the non-interactive environment variable on a device identity, an absent verification_uri_complete, a device grant that returns no refresh token, and the report naming a subject it did verify. Refs #42
|
Pushed a02c89b — a security/robustness fix found by running A deployment could panic the shell. RFC 8628 lets the authorization server choose the polling interval, and The cost is that a panic escapes the problem type entirely: a Go stack trace instead of a typed refusal, and an exit code outside the class list, with no The advertised interval is now sanitized before use: not-a-wait falls back to the spec default, past-the-ceiling is clamped rather than reinterpreted (a deployment asking for an enormous wait has asked for something, and polling sooner than it consented to is exactly what §3.5 exists to prevent). An internal test pins that the ceiling sits above any deadline a login can run under, so the clamp can never start meaning "poll sooner". Also: an identity token that is present and fails verification is now refused rather than reported as no identity. Absent and invalid are different answers — tolerating silence is a judgement about an unmeasured protocol, tolerating a bad signature would be a judgement to stop looking. Not a security boundary either way (an unverified token was never trusted), but a wrong Plus the four test gaps the spec axis found. Issue #42 updated to match what shipped — the earlier "exported constants" wording described a design I did not build.
Not changed: the |
There was a problem hiding this comment.
Pull request overview
This PR implements the oauth-device authentication kind (OAuth 2.0 Device Authorization Grant, RFC 8628), closing #42. It lets wso2 login complete on a machine whose user has no reachable browser (SSH sessions, containers) by printing a verification URI and user code to approve on any other device, then polling the token endpoint until approval. The device flow produces the same Result as the browser flow, so everything downstream of a session (per-module scoped-refresh narrowing, rotation-safe persistence) is identical — reflected in the browserSource → sessionSource rename.
Changes:
- New
oauthflow.DeviceLoginflow (discovery-gated ondevice_authorization_endpoint, interval sanitization to avoid a ticker panic, typed refusals that never echo the issuer's words, tolerant id-token handling). - Broker/login wiring:
oauth-devicenow resolves to the sharedsessionSourceand logs in interactively; a device-specific deadline and CI/non-interactive refusal are added. - Test + fixture support: fakeissuer gains a device-authorization endpoint and polling model; new acceptance, internal, fixture, and smoke tests; plus documentation across guides/architecture/requirements.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/auth/oauthflow/device.go |
New RFC 8628 device login flow, interval clamping, presentation, id-token identification, typed error mapping. |
internal/auth/oauthflow/login.go |
Package doc updated to describe both interactive login modes. |
internal/app/login.go |
Adds establishSession dispatch, device deadline, mode-aware non-interactive refusal, conditional Subject reporting. |
internal/auth/source.go |
Routes oauth-device through the shared session source; updates unimplemented-kind messaging. |
internal/auth/source_session.go |
Renames browserSource → sessionSource with updated doc rationale. |
internal/auth/source_session_test.go |
Renames the corresponding TestBrowserSource* tests to TestSessionSource*. |
internal/auth/auth_test.go |
Replaces the device "unimplemented" case with a product-policy case reaching the session source. |
internal/auth/fakeissuer/fakeissuer.go |
Adds device-authorization endpoint, polling states, options, and inspection accessors. |
internal/auth/fakeissuer/fakeissuer_test.go |
Adds device single-use / unknown-code fixture tests. |
internal/auth/oauthflow/device_internal_test.go |
Unit tests for usableInterval and the clamp-safety invariant. |
internal/app/login_test.go |
Adds non-interactive device refusal cases. |
test/acceptance/login_device_test.go |
End-to-end device login coverage (narrowing, presentation, non-disclosure, refusals, back-off, hostile intervals). |
test/smoke/login_device_smoke_test.go, test/smoke/config.go, test/smoke/env.example |
Live smoke run against a real deployment with a device-kind document. |
Makefile |
Adds the smoke-login-device target. |
docs/*, CONTEXT.md |
Documents the device grant, glossary term, and updates implemented-status notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #42.
Implements the
oauth-deviceauthentication kind — RFC 8628 — sowso2 loginworks on a machine whose user has no browser that can reach it.Why
The browser login binds a loopback listener and waits to be redirected back to
127.0.0.1on this machine. A developer over SSH, or inside a container, has a browser somewhere else, so they cannot finish it.WSO2_NO_BROWSER=1helps with a missing browser, not with a machine you are not sitting at.The schema already accepted
kind: oauth-device, so those users got a document that loaded and validated and then refused at use — told the shell knew what they wanted and would not do it. The workaround people reach for is aclient-credentialsidentity, which authenticates as an application rather than a person and puts a client secret on a developer's machine.What a user sees
Both values stand on their own line so they survive being read aloud over a call. The user code is printed verbatim, separator and all — RFC 8628 §6.1 already asks the deployment to make it readable, and the deployment is what validates it back, so the shell does not prettify a value it does not own. No browser is opened: this flow exists for the machine that has none.
Three decisions worth review
No new problem codes. The brief asked for one per RFC error. Two of the four (
authorization_pending,slow_down) are loop control and never reach a user; the other two leave the caller in the same place, holding no session. They shareauth.credential_unavailableand differ in their sentence, the wayidentityNotVerifiedalready varies four messages across one code. A missingdevice_authorization_endpointreusesauth.discovery_failed, mirroring the missing-S256 refusal. This leaves #40 open rather than answering it in passing.No
source_device.go. Deriving module access from a device-established session is identical to a browser one at every step, sobrowserSourcebecomessessionSource— named for what it derives from — and both interactive kinds resolve to it. The rename is the load-bearing part: it makes the real fact visible, which is that the login mode stops existing the moment a session does.A missing identity token does not fail a device login, unlike a browser one. RFC 8628 defines no nonce, the session is the refresh token, and whether Asgardeo or IS 7.x return an
id_tokenfrom this grant is unmeasured. The subject is reported when verified and absent when not, rather than letting an unmeasured behaviour decide whether the flow works.make smoke-login-devicereports which it saw, so the answer can be recorded.Tests
One seam —
wso2 loginat the existing acceptance seam, in process against the fake issuer, with the reference module launched as a real subprocess presenting its token to a real service. The fake issuer gained a device endpoint; that is a fixture extension, not a second seam.Interval and back-off are asserted from the issuer's record of when each poll arrived, because "the deployment was not polled faster than it asked" is a fact about the deployment. That one test spends ~7 real seconds and cannot avoid it: the RFC fixes the back-off at five, and the shell honours the RFC rather than a knob the suite could turn down.
Covered: approval after pending polls, then a module receiving access narrowed to one permission out of four; presentation; non-disclosure of the device code and all token material; declined vs abandoned reading differently; a deployment without the grant refused before any code is shown and without a single poll; a grant returning no identity token still establishing a usable session.
make test vet lint acceptanceandmake smoke-buildall pass. Acceptance grew by ~24s.Scope
Out:
wso2 login --device-code, which is the login-time mode for an identity that also supports browser login. This slice is the stored kind only, so a developer who merely happens to be on a headless machine still needs a second identity — §5.1 of the walkthrough says so plainly rather than letting them find out. Also out: any Thunder path (no device grant handler exists),pat, and any schema change.Shared files
internal/auth/source.go(one case label),docs/guides/login.md(new §5.1 and new troubleshooting entries, no rewrites),test/smoke/config.go(one defaulted field),test/smoke/env.example(one comment block). All additive for the Thunder session; every new test is in a new file.