feat: broker derives module access from the login session by scoped refresh - #27
Merged
Merged
Conversation
The broker used to dispatch on the authentication method in three places: a namespace guard before anything else, a context check that hard-coded the development credential, and a mint that only ever built a fixture token. An identity kind is now resolved once, in one switch, and the source it names mints the grant. Schema version 2 policy arrives with the seam: a production identity must register the product namespace asking, with the audience and permissions the module requests, and a context may not point an identity at an organization outside its home tenant. Device and personal access token identities are legal in a document and refused at use. Two orderings changed as a consequence. The module receipt is now checked before the identity, so a module asking beyond its installation is told so whatever context is selected. A non-reference namespace with no identity now refuses auth.context_not_selected rather than auth.namespace_not_brokered: the proof-namespace guard belongs to the development source, and reaching it means an identity was resolved. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
An oauth-browser identity's access now comes from the session wso2 login stored: the refresh grant carries exactly the permissions the module asked for, and the shell verifies what came back before any of it reaches the module. The effective scopes must equal the request and the token must be bound to the requested audience, read from the response and the token's own claims. Every shortfall — narrowing ignored, narrowing refused, an audience the deployment registered differently, or a token whose claims cannot be read at all — is auth.narrowing_unavailable with no grant, because a module handed more than it asked for holds authority nobody decided to give it. The derivation runs under the session's rotation lock, and a rotated refresh token is persisted before the grant is returned: a crash after that point costs an access token, and before it would have cost the session. Acquire now restates any typed problem a source raised as a broker denial, so the session store's own auth.login_required reaches a module in the one shape every other refusal has. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The chain end to end: wso2 login establishes a session against the fake issuer, the reference module asks the broker for one permission, and what it presents to its product service is a token the issuer's introspection confirms it minted — carrying exactly the requested scope out of the several the session holds, bound to the product's audience. The assertion is made from the service that received the bearer token and from introspection, never by printing it. The issuer rotates refresh tokens, so consecutive runs also prove each one persisted its replacement, and that the session the login left stops working once it has been superseded. The refusals get the same treatment: a run with no login names wso2 login, a context outside the identity's home tenant is refused, and an issuer that ignores narrowing, refuses it, or registers another audience produces auth.narrowing_unavailable with the product service never reached. These tests run the shell in-process, which is the one departure from this package's rule, because the OS secure store must be go-keyring's in-process mock. The module is still launched as a real subprocess over the real protocol, so the chain under test is the whole of it. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Review findings, no behaviour change. The identity kind was switched on twice four lines apart, and the product registration the second switch took was computed, threaded through, and discarded. There is now one switch, and the registration check is what it always was: a check, returning only whether the request is admitted. Proving an issued token is what the module asked for moves out of the seam file and onto the token response it was reaching into, so source.go answers for which source serves an identity and narrowing.go answers for whether a grant may be handed over. A new identity kind and a change to what counts as proof no longer land in the same file. A failed renewal POST no longer borrows the discovery refusal's wording. By that point the issuer's configuration has been read successfully, so telling the user the shell could not read it sent them to look at something working. The acceptance package documented running everything as built binaries while the login chain runs the shell in-process for the keyring mock. The package doc now states the exception and what it costs rather than being quietly untrue, and the browser broker fixture is built from the shared production identity instead of repeating it. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Review findings. invalid_scope was read as "this deployment will not scope the session down" whatever status carried it, so a failing or unauthorized token endpoint that happened to mention it was reported as a registration problem — sending the user to change something that was never wrong. It is now read on HTTP 400, the status RFC 6749 defines it on, and every other failure stays the login-required refusal it was. Rotation no longer blanks the access token and expiry the session carries. Only the refresh token is replaced; what a derivation mints is narrower than the session and belongs to one module for one command, so it is handed over and never stored. Two test gaps close. The reordering the source seam introduced — a namespace outside the proof with no identity refusing auth.context_not_selected rather than auth.namespace_not_brokered — was described in a commit body and pinned nowhere. And the test that a fully configured identity reaches its source asserted only that seven policy codes were absent, so any unrelated failure passed it; it now names the refusal it expects. The production broker fixture also took its session lock relative to the working directory, creating internal/auth/cli/locks inside the source tree on every run. It gets an isolated state root like every other test. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This was referenced Aug 5, 2026
kanushka
requested review from
axewilledge,
kaje94 and
sachiniSam
and
a lite review from Copilot
August 5, 2026 04:17
There was a problem hiding this comment.
Pull request overview
Implements the broker-side “scoped refresh” derivation for oauth-browser login sessions, including a new internal token-source seam and end-to-end acceptance coverage proving strict scope narrowing, audience binding, refresh-token rotation persistence, and token-material non-disclosure.
Changes:
- Adds an internal
sourceseam ininternal/authand routes access acquisition through a single per-kind resolver with new typed policy refusals. - Implements browser-session derivation via refresh-grant scoped to the module request, with verification of effective scopes and audience coverage plus safe refusal modes.
- Adds new unit + acceptance tests covering narrowing modes, issuer mismatch, rotation persistence, and disclosure sweeps; wires the shell state root into the broker for session-rotation locking.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/acceptance/login_test.go | New end-to-end acceptance suite for login → broker acquisition → module → product service, including rotation and disclosure assertions. |
| test/acceptance/acceptance_test.go | Documents why login acceptance runs the shell in-process (keyring mocking). |
| internal/auth/source.go | Adds token-source seam and per-kind policy + source resolution (including new refusal codes). |
| internal/auth/source_dev.go | Moves development-fixture token minting into a dedicated source. |
| internal/auth/source_browser.go | Implements scoped refresh derivation + rotation persistence under lock + refusal mapping. |
| internal/auth/source_browser_test.go | Unit tests for narrowing, rotation persistence, issuer mismatch, and refusal/no-disclosure behavior. |
| internal/auth/narrowing.go | Adds token-response parsing and strict scope/audience verification helpers. |
| internal/auth/discovery.go | Adds OIDC discovery helper to resolve token endpoint safely. |
| internal/auth/claims.go | Adds JWT-claim extraction for audience/scope/expiry facts used in verification. |
| internal/auth/auth.go | Refactors Acquire to resolve a source and mint via it; adds broker state-root + HTTP client plumbing. |
| internal/auth/auth_test.go | Adds production-policy tests for the new per-kind switch and policy ordering. |
| internal/app/invoke.go | Passes shell state root into the broker so refresh-token rotation uses the correct lock location. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+133
to
+137
| func (b *Broker) checkHomeTenant() error { | ||
| organization := b.Selection.Context.Organization | ||
| if organization == "" || organization == b.Selection.Identity.Auth.Tenant { | ||
| return nil | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Intended fail-closed behavior, per the plan (Task 8) and spec user story 9: if the identity declares no home tenant, the shell cannot verify the named organization matches the session's tenant, so it refuses rather than acting on an unverifiable claim. The walkthrough documents the rule (docs/guides/login.md §4.3): leave organization out, or set it to the identity's auth.tenant. The fix for an affected user is declaring tenant on the identity.
sachiniSam
approved these changes
Aug 5, 2026
7 tasks
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
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.
Implements #21 (part of #17). Fourth PR of the
wso2 loginstack — stacked on #26; only the last five commits are new here.What this does
sourceper identity kind — a single switch replaces the three scatteredAuth.Kinddispatches; the development fixture source remains, restricted to the reference namespace. v2 policy lands with it:auth.product_not_configured(namespace/audience/scope mismatch against the identity's product entry),auth.organization_switch_unsupported(context outside the identity's home tenant),auth.login_required,auth.session_issuer_mismatch,auth.kind_not_implementedfor device/PAT. No new exported symbols ininternal/auth.auth.narrowing_unavailable— never a broader grant. Rotated refresh tokens persist under the session lock before the grant returns.wso2 reference statusproven end to end — the fake issuer's introspection confirms the bearer the status service recorded is active with exactly the requested scope and product audience; a second invocation survives rotation; a disclosure sweep asserts no token material on any output surface.Verification
Full
go test ./... -race -count=1green (all 19 packages, acceptance gate 151s);golangci-lint run0 issues; port-contention re-run of the two suites sharing loopback ports with-count=2clean. Review passes fixed, among others, a token-endpoint handler that readinvalid_scopeoff any error status, and a test broker writing its lock directory into the source tree.Stack
feature/loginfeat/login-context-schema-v2wso2 loginPKCE →feat/login-auth-testbedfeat/login-browser-pkcehttps://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp