feat: wso2 login — browser PKCE and inline client credentials - #38
Merged
Conversation
The pat kind constrained only its credentialRef, so an identity could carry clientSecretVariable with a raw secret in it and still decode. That is the one member the variable pattern exists to guard, and pat is the kind whose users are most likely to be holding a token in hand, so the document is now refused rather than silently accepted with the member ignored. Product namespaces are also walked in sorted order, so a document with more than one unreadable product is refused for the same reason on every run. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Encode rejected a v1 document only by finding a synthetic identity, and a v1 document declaring no contexts produces none. Such a document still failed, but through the schema check, reporting contexts.schema_unsupported and telling the author to update the CLI instead of saying the shell does not rewrite version 1 documents in place. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
…text --context= and --context "" resolved to the empty name, which selection treats as absent, so the shell fell through to WSO2_CONTEXT and then the default context. A user who named a context explicitly and silently got a different one has no way to see that it happened. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The mutual-exclusion cases swapped one credential-source member for the other instead of adding it, so the document was refused for the missing member and the rule under test never ran; both branches would have kept passing if deleted. They now add the offending member alongside the legitimate one. Adds the branches that had no case at all: an unreadable issuer URL, a missing issuer on client-credentials, a missing credentialRef on pat, an invalid identity name, and a product without an endpoint. identity.go now has full statement coverage. TestSyntheticIdentityIsNotEncodable asserts the problem code, without which it passed on the schema check rather than the compatibility guard, and asProblem uses errors.As like its sibling helper. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
…-safe locking Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The shell binary now carries an HTTP client, so the offline guarantee is stated where it is still true: nothing wso2 version reaches to resolve module inventory may depend on net/http. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
An abandoned login waited forever on a bound callback port: the flow honoured a cancelled context but the command never armed one. The authorization URL is an instruction to act on, not the command's result, so it moves to the diagnostic stream and leaves standard output carrying only the identity report. Also share the --context parsing both commands had grown separately, and prove the refusals that were only asserted by construction: no browser suppression, an identity token missing its nonce, and a login the issuer answers without a refresh token. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
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
…mmand A context whose identity is client-credentials needs no login: the secret is already on the machine. The shell reads it from the variable the identity names into process memory, spends it on one grant scoped to the module's own request, and applies the browser source's issued-token verification unchanged. Nothing is written to the state root or the OS secure store. A refusal to narrow and a rejected credential are told apart, because they send the user to different places. A rejected credential is never reported as a login to run: wso2 login refuses this kind of identity outright. The broker's credential read is generalized to name what the variable holds, so the guidance a user receives reads as an instruction for both kinds. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The CI rows of the acceptance chain: a client-credentials context with the secret exported hands the reference module a token the issuer vouches for, carrying only the permission it asked for; a job that forgot the variable is told which one to set; and under WSO2_NON_INTERACTIVE the login refuses as not required while the command it would have preceded still succeeds. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The client-credentials grant went out through golang.org/x/oauth2 while the refresh grant posted its own form, so the package read a token endpoint's answer two ways and turned the same invalid_scope into two different refusals. Both now share one request helper: the caller supplies the grant's own members, and how a client identifies itself, how an answer is read, and what counts as a refusal to narrow are each decided once. Doing so lets the inline grant honor the broker clock, so its expiry is the shell's own arithmetic and a test can pin it. A deployment that answers with something the shell cannot classify is now told apart from one that did not answer at all, instead of both claiming the issuer was unreachable. The fake issuer validates its scope modes rather than treating a typo as the permissive default, and decodes HTTP Basic credentials as RFC 6749 requires. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The non-interactive check asserted only that an inline identity refuses a login as not required, which it does with or without the variable set — so nothing proved the non-interactive guard was still live beside it. The test now runs an interactive and an inline deployment in the same environment: the first is refused as non-interactive, the second as needing no login, and the command the job actually runs still succeeds. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
…d tag The slice has been provable only against a fake issuer. These are the runs that prove it against a deployment that really exists: one signs a human in through `wso2 login`, reads the refresh token back out of the operating system's secure store, and brokers one acquisition on top of that session; the other answers the two questions the redirect-and-narrowing research could not settle from public sources, and prints a verdict line for recording. They are separated from the default gate by a build tag rather than a skip, because scripts/acceptance.sh runs `go test ./...` and a run that opens a browser has no place in it. A make target compiles them anyway, since the tag that keeps them out of the gate would otherwise let them rot unnoticed. The half that can be proven without a deployment is deliberately left untagged and unit-tested: reading the environment that names one, building the context document a run installs, and classifying the narrowing outcome. The last of those matters most. Its verdict is copied by hand into a research document as a finding about a product, so a misclassification becomes a false claim that outlives the run — and one misclassification was real. Every login requests openid and offline_access alongside the product permissions, so a deployment that narrows correctly can answer with those attached and fail an exact set comparison. Read naively that says "ignored" about a deployment that honored the request, which is the opposite of what happened. Nothing a run leaves behind persists. The context document goes to a temporary state root, the session is stored under a reference no human would choose for a real context and is deleted before and after every run, and no token is ever printed or written.
Nothing in the tree told a person how to get `wso2 login` to work. The knowledge existed only as the shape of a fixture and the four ports a constant names, so the first user of a real deployment would have had to read the source to learn what to register. The guide is written to be read on its own, because the person who needs it has no reason to already know which other document to open. It registers the public client in Asgardeo and in Identity Server 7.x, authors the context document by hand with a complete example to copy, runs the first login, and sets up a CI job that authenticates inline with no login step at all. Its longest section is the troubleshooting one, and that is deliberate. Every refusal this slice can produce is typed, and a typed refusal is only as useful as the reader's ability to turn it into something to change. Each code is listed with what it means and what to do about it — including the five distinct narrowing refusals, which look alike and send a reader to five different places.
…ed in The redirect-and-narrowing research left two cells reading "unknown — needs empirical test". The experiments that answer them now exist, but no live tenant has been run against, so the cells say pending rather than an answer. Recording a verdict nobody measured would be worse than leaving the question open: the document is the thing later decisions cite, and a fabricated cell is indistinguishable from a measured one once it is written down. Section 4 is what makes the cells fillable — the exact commands, what each verdict word means, and the one verdict that must be corroborated from the browser before it is believed. It also states what each possible answer would mean for the shell, so that the run produces a decision rather than a fact. The compatibility document's backend gap now carries the standing ask it implies, with its owner: a WSO2-seeded, well-known `wso2cli` public client. The gap is stated there in the abstract; what was missing was the cost, which is concrete and now measurable — two of the walkthrough's nine sections exist only because every tenant registers its own client by hand. The two documents are brought over from main, where they were written after this branch was cut. This branch already linked to one of them, so the link was broken until now.
feat: live smoke gates, asgardeo experiments, and the login walkthrough
docs: home the login plan under docs/plans and record the auth decision
WSO2 deployments publish token-signing certificates whose X.509 serial numbers are negative, which RFC 5280 forbids and which Go has rejected since 1.23. The certificate travels in the x5c field of the JWKS, and go-jose parses x5c while unmarshalling the key set, so one unparseable certificate fails the whole document and the shell is left with no keys at all. Login became impossible against such a deployment even though its signing key was always perfectly readable: n and e describe it completely, and the signature was never actually checked. Strip the certificate members from key sets at the transport, before any library parses them, and only from keys that already carry their own parameters. A key that genuinely depended on its certificate keeps it and still fails loudly; this removes a spurious failure rather than papering over an unreadable key. Responses that are not key sets are returned byte for byte. Say which way verification failed while we are here. The refusal discarded the underlying error and advised retrying, which cannot help an unreadable key set or a token minted for another application. The code stays auth.credential_unavailable because the caller is left in one place; the message no longer pretends the cause is unknown when it is not. The fixture that makes this regress noisily needs a negative serial, and crypto/x509 will not mint one, so the fakeissuer generates an ordinary certificate and edits the padding byte out of its serial afterwards - which reinterprets the same bytes as negative, exactly as the deployments encode them. Verified against the live tenant this was found on: `make smoke-login` now completes with no GODEBUG override and brokers an acquisition. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Three things the first live Asgardeo run showed were wrong, none of them about what the runs measure. The smoke run reported every auth.narrowing_unavailable refusal as "the deployment would not prove a narrowed grant", but that code covers five distinct causes and one of them is an access token bound to the wrong audience, where narrowing was never the problem. The summary now states only what holds for all five; the interpolated error still says which one happened. The narrowing verdict's `rejected` branch had no corroboration warning, unlike its any-port sibling. invalid_scope is also what a token endpoint answers when the application's resource authorization carries a policy the signing-in user does not satisfy, so an unchecked `rejected` records a registration gap as a finding about Asgardeo. The audience-unbound verdict advised fixing the API resource registration, which cannot work: Asgardeo binds aud to the client ID and offers no setting that changes it. It now names the remedy that exists. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Both questions the redirect-and-narrowing research left open are now measured against a live tenant: any-port loopback is supported, and the refresh grant honors narrowing exactly. Section 3's pending cells carry the verdicts, their date, and the deployment they were measured on. The same runs turned up a third result the document did not anticipate. Asgardeo binds an access token's aud claim to the client ID, never to the API resource whose scopes the token carries, and nothing configures it. So products.<namespace>.audience must be the client ID there, and the audience check cannot distinguish one product from another - which is a design question about the broker's policy rather than a documentation fix, and is recorded as such. The walkthrough gains what the run cost an evening to discover: that creating an API resource and authorizing it span two screens, that the "requires authorization" checkbox cannot be changed afterwards and quietly redirects the reader into a different authorization model, that the account which administers the organization is not one the application can authenticate, and what to do when a policy means the scopes need a role. Section 9 now walks the runs that need no deployment before the ones that need a browser. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The stripper read the body, then treated a failure to close it as fatal. Closing a response body is what releases the connection to the pool; it says nothing about bytes already in hand. A connection torn down between the last byte and the release would have failed the login outright - inventing exactly the kind of spurious failure this file exists to remove, and doing it on every fetch a login makes rather than only on key sets. A read that did not finish stays fatal. There the body has been consumed and cannot be handed on, so there is no response left to return. Reported by Copilot on the pull request. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
fix: log in against deployments whose signing certificate Go rejects
A second deployment made the shape of the problem obvious. The variables that name a deployment were exported by hand into whichever shell was running the live targets, so moving between Asgardeo and a local Identity Server meant retyping five of them, and the one that differs between the two is not the one anyone would guess: the audience is the client ID on Asgardeo and the API resource identifier on Identity Server. A leftover export from the last deployment then outranks whatever you meant to run against, silently. So the live targets now source a file. Go has no dotenv convention and this module carries four direct dependencies, all load-bearing, so nothing here parses anything: the file is an ordinary shell fragment, `make` sources it, and sourcing it yourself does exactly the same thing when running `go test` directly. One file per deployment, named with SMOKE_ENV, and the file overwrites what the shell already exported rather than deferring to it — which is the direction that makes switching deployments work without a fresh terminal. env.example writes the Asgardeo audience as a reference to the client ID rather than as the same string twice, because two identical placeholders read as a copy-paste slip and invite someone to "fix" one of them. *.env joins the ignore list. The existing .env and .env.* patterns did not cover a per-deployment file named for its deployment, which is the natural thing to write and would have been committed. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The walkthrough carried Identity Server as the product it had not been run against. Section 3.5 said what lands in an access token's `aud` claim there was unmeasured, section 3.3 cited the 6.0.0 documentation for loopback port flexibility, and section 3.6 pointed at the TLS problem without saying what to do about it. All three are now answered against wso2/wso2is:7.3.0. The `aud` finding is the one that matters, because it is not what the Asgardeo run implied. Asgardeo binds the claim to the client ID and offers no way to change it, and this document had recorded that as a platform fact with Identity Server flagged as an open question. It is not a platform fact. Identity Server 7.3.0 adds every entry of the application's audience list to the access token beside the client ID, so the API resource identifier is the right value there — but only once it is registered, and the field that registers it is the one that on Asgardeo reaches the ID token alone. The same-looking control does different work on the two products, which is precisely the kind of difference a walkthrough exists to state. So the shell's audience check can carry product-level meaning on Identity Server and cannot on Asgardeo. Nothing in the broker changes: it already verifies rather than assumes, which is correct under either behavior. What changes is what a context document should say, and that carrying one product's `audience` value to the other costs a browser sign-in and ends in auth.narrowing_unavailable. Loopback flexibility turned out stronger than the documentation implies. The application registers its callbacks as a regexp enumerating four ports, and a login through a fifth completed anyway, so the waiver is applied ahead of the registered pattern rather than as a fallback when none matches. Section 3.6 now writes the TLS step out, including what it costs. Go ignores SSL_CERT_FILE on macOS and the shell exposes no custom-CA option, so the keychain is the only way in — and the certificate being trusted is CA:TRUE with a private key that ships in every download and every copy of the public image behind a published password. That is worth trusting for a local test and worth removing afterwards, and a reader should be told both. The research document records the Identity Server verdicts in a table of their own. The Asgardeo cells stay as they are: these are two deployments, not two readings of one. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Every one of the plan's sixty-seven checkboxes was still open, including the definition of done, while all twelve tasks were implemented, reviewed and merged. Anyone reading the plan as a status report would conclude the slice had not been started. Sixty-six are now ticked. The definition-of-done entries carry what closed them rather than only a mark: which deployment and date each live check ran against, and which acceptance test pins each deterministic one. One stays open, and the note under it says exactly why. The live smoke run acquires through the broker asking for every scope the session already holds, so its scope comparison is a set against itself and cannot fail — a deployment that ignored narrowing entirely would still report a grant. The audience half of that check is real; the scope half is not. config.NarrowTarget() exists for this and the smoke run does not use it. The module half of that same check needs nothing further: a module never learns which issuer minted its token, so running it against a live deployment would prove nothing the deterministic chain test does not already prove. The closing note records the finding that outran the plan. The spec assumed an access token's audience could name the product resource; Asgardeo binds it to the client ID with no way to change that, and Identity Server 7.3.0 does not. That makes a definition-of-done clause satisfiable on one supported product and structurally not on the other, which is a decision for the issue rather than a task here. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
**env.example promised a password it does not contain.** The note explaining why nothing secret belongs in these files pointed at "the password below" — reasoning carried over from a real per-deployment file, which does carry a test account's password, and left in the template, which carries none. A reader searching for it finds only the sentence claiming it exists. Rewritten to say what is actually true of these variables: none of them is a credential, and the CI client secret deliberately lives in an environment variable a context names rather than anywhere on disk. The gitignore pattern in that sentence was stale too — it is `*.env` now, not `.env` and `.env.*`. **Section 3.6 read a keystore the container-first path does not have.** The section now opens by recommending a container, then exported the certificate with `keytool` from `repository/resources/security/wso2carbon.p12` — a path that exists only in an unpacked distribution. Rather than adding a `docker cp` beside it with a version-pinned in-container path, take the certificate from the port: one command, correct for either deployment shape, no keystore password, and it captures exactly what the deployment serves rather than what its keystore happens to hold. Verified against the 7.3.0 container to produce bytes identical to the keytool export, which is still named for the case where the certificate is wanted before the deployment is running. **The Makefile described sourcing with `source`.** Harmless where it stood — that sentence is about the reader's own interactive shell, not a recipe, so the `/bin/sh` the recipes use never sees it — but RUNNING.md and env.example both write `.` for the same instruction, and one file disagreeing with the other two is the kind of thing a reader stops to wonder about. Aligned on `.`. env.example was re-checked by sourcing it under plain `/bin/sh`: it reads cleanly and its audience-as-a-reference line still resolves. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
The live run asked the broker for config.Scopes — every permission the session already carries — so internal/auth/narrowing.go compared the issued scopes against an identical request. That comparison holds however the deployment behaved. A backend that disregarded the narrowing request entirely would still have been reported as granted, and the definition of done asks for a backend *proven* to satisfy the scope policy. The audience half of the same verification was real and had caught a live misconfiguration. The scope half could not fail. config.NarrowTarget() had existed for this since the package was written and nothing called it. So the run now acquires twice. The first request is the broad one and proves the broker can derive access at all, which is what a first run against a new deployment needs to know. The second asks for one permission out of the several the session holds — the request a module actually makes, and the only form the verification can disagree with. A refusal on the broad request stops the run rather than repeating one finding, because a deployment that will not issue against this audience at all says nothing about narrowing. Reusing one broker for both refused with auth.already_granted: a module gets one acquisition per command. Two invocations is not a way around that rule but the accurate model of it — two commands, in turn, against one session. Against a rotating deployment the second acquisition now also proves the first persisted its replacement, since it can only reach the token endpoint with what the first one stored. Measured against wso2/wso2is:7.3.0: 1261 characters carrying both permissions, then 1230 carrying one. Two tokens the previous single-acquisition form could not have told apart. The module half of the same definition-of-done clause needs nothing further. A module never learns which issuer minted its token, so running it against a live deployment proves nothing TestLoginThenTheModuleReceivesIssuerMintedNarrowedAccess does not already prove against the fake issuer, where the token is introspected and rotation is pinned across three runs. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Review found the smoke run's refusal example truncated. The run prints the shell's own message on an indented second line, and the example stopped before it — while the paragraph beside it said that message is what distinguishes one cause from another. A reader following that advice found nothing to follow it with. The example now carries the indented line, and the four other messages are tabulated beside it. The summary sentence above them is deliberately identical for all five, because auth.narrowing_unavailable covers five distinct causes and a summary naming one would be wrong four times out of five. Only the indented line is specific, which is precisely why it had to be in the example. Two corrections in section 8 of the walkthrough, which the new cross-reference sends readers to and which contradicted what this branch's parent measured. The audience row still said the value to configure is the API resource identifier. That is right on Identity Server and wrong on Asgardeo, where the only value ever placed in `aud` is the client ID — so a reader hitting an audience refusal on Asgardeo was being told to set the one value that cannot work. It now names both products and points at the sections that measured them. The paragraph under the table still said whether Asgardeo narrows would be recorded "once measured against a live tenant". It was measured on 2026-08-06, on both products, and both narrow. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
test: make the smoke run's narrowing check able to fail
An identity is one login session and every product the broker can derive valid access for without another login or credential. A context is one named target referencing exactly one identity; one identity may back several contexts. Sharing an issuer no longer implies sharing a login. A product that validates its own resident issuer, or needs its own authenticated client, token, or password, is a separate identity and context. Whether a session reaches a product is a runtime property, so a wrong assertion surfaces as a typed refusal rather than a malformed document. Targeting moves to the context; authentication kind, issuer, client, and credential reference stay on the identity; endpoints gain audience and scopes so derived access has something to bind to. Browser and device become login modes for one interactive OIDC identity, device only where the backend advertises the grant. Client credentials complete inline with no separate login step; a directly presented token cannot be derived from and is named as adapter territory rather than an equivalent method. Selection gains a recorded per-namespace binding, so an estate needing separate logins per product does not force --context onto every command without the shell ever inferring one. Architecture and product requirements now carry the decisions, including the five properties replacing the proof's no-command-can-write invariant. The examples document illustrates them and states that it does not extend them. Cherry-picked from 0a24fb9, written before the slice was implemented and left behind on a local main that was never pushed. Applies unchanged.
Both were written, both are referenced by the reasoning that shaped this slice, and neither was in any commit on any branch. `git log --all` found nothing for either path. A stray `git clean` would have taken 1,279 lines of work with it. `context-identity-model-feasibility.md` is the prior art behind the model architecture §4.6-4.7 codifies — one identity, one reusable login session, a context naming exactly one identity. That section was committed reasoning from its conclusions while the reasoning itself sat untracked beside it. `login-walkthroughs.md` is a proposal for the target login experience and says so in its own header: not slice 1. It is committed here as a proposal, not as a plan — the value is that a later slice can argue with it rather than rediscover it. The research index gains a row for the first and a corrected description for `product-authentication-compatibility.md`, whose entry predated the section on whether one login session can serve several modules in one context. The `asgardeo-redirect-uri-and-scope-narrowing.md` entry now mentions section 3.1, added when Identity Server 7.3.0 was measured. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
Two links named `docs/superpowers/specs/2026-08-04-login-first-slice-design.md`. That path is in no commit on any branch and `docs/superpowers/` is not a directory in this repository — the plan it refers to was homed under `docs/plans/` instead, and the design it refers to survives as the body of issue #17. Both now point at `docs/plans/login-first-slice.md`, which exists, carries the implementation decisions, and names issue #17 as its own spec. Only reachable because committing the file made its links checkable for the first time. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughSummary
WalkthroughThis change implements the first Sequence Diagram(s)sequenceDiagram
participant User
participant Shell
participant IdentityProvider
participant Keyring
participant Module
User->>Shell: Run wso2 login
Shell->>IdentityProvider: Complete browser PKCE flow
IdentityProvider-->>Shell: Return verified tokens
Shell->>Keyring: Store refresh-token session
User->>Module: Run product command
Module->>Shell: Request scoped access
Shell->>IdentityProvider: Refresh and narrow token
IdentityProvider-->>Shell: Return verified access token
Shell-->>Module: Provide short-lived access grant
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: 16
🧹 Nitpick comments (7)
internal/auth/oauthflow/login.go (1)
400-402: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider setting the remaining
http.Servertimeouts.Static analysis flags this server for a missing
ReadTimeout.ReadHeaderTimeoutalready bounds the header phase, the listener is bound to 127.0.0.1, andcloseruns on every path, so the exposure is small. SettingReadTimeout,WriteTimeout, andIdleTimeoutcloses the remaining gap and clears the finding.♻️ Proposed timeouts
- waiting.server = &http.Server{Handler: mux, ReadHeaderTimeout: callbackReadHeaderTimeout} + waiting.server = &http.Server{ + Handler: mux, + ReadHeaderTimeout: callbackReadHeaderTimeout, + ReadTimeout: callbackReadHeaderTimeout, + WriteTimeout: callbackReadHeaderTimeout, + IdleTimeout: callbackReadHeaderTimeout, + }🤖 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/oauthflow/login.go` around lines 400 - 402, Update the http.Server initialization in the OAuth callback flow around waiting.server to set ReadTimeout, WriteTimeout, and IdleTimeout in addition to the existing ReadHeaderTimeout, using appropriate established timeout values or constants. Preserve the existing handler, listener, and Serve behavior.Source: Linters/SAST tools
internal/auth/oauthflow/jwks.go (1)
132-142: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider bounding the buffered response body.
RoundTripbuffers the full body of every 200 response that passes through this transport. The documents involved are discovery, JWKS, and token responses, which are small. A malfunctioning endpoint that returns a very large body would still be read into memory in full. Anio.LimitReaderwith a generous ceiling keeps the failure bounded without changing normal behavior.♻️ Proposed bound on the buffered body
- body, err := io.ReadAll(response.Body) + // A key set, a discovery document, and a token response are all small. + const maxBody = 8 << 20 + body, err := io.ReadAll(io.LimitReader(response.Body, maxBody))🤖 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/oauthflow/jwks.go` around lines 132 - 142, Bound the body read in RoundTrip by wrapping response.Body with an io.LimitReader using a generous maximum size before io.ReadAll. Detect when the limit is exceeded and return an appropriate error instead of forwarding truncated content, while preserving normal handling of successful small responses and body closure.internal/auth/session/session_test.go (1)
128-135: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the peak-concurrency update atomic.
The load of
maxInsideand the store are separate operations. Two goroutines can interleave between them and lose an update. The consequence is a false pass, not a false failure, so the test is not wrong today. Since this assertion guards the single-writer property of the lock, a compare-and-swap loop makes it dependable.♻️ Proposed atomic peak update
_ = store.WithLock("acme-cloud-login", func() error { now := atomic.AddInt32(&inside, 1) - if now > atomic.LoadInt32(&maxInside) { - atomic.StoreInt32(&maxInside, now) - } + for { + peak := atomic.LoadInt32(&maxInside) + if now <= peak || atomic.CompareAndSwapInt32(&maxInside, peak, now) { + break + } + } time.Sleep(5 * time.Millisecond) atomic.AddInt32(&inside, -1) return nil })🤖 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/session/session_test.go` around lines 128 - 135, Update the peak-concurrency tracking in the WithLock callback to use an atomic compare-and-swap loop when raising maxInside, retrying until the observed value is already at least now or the CAS succeeds. Preserve the existing inside counter updates and lock-concurrency assertion.test/acceptance/acceptance_test.go (1)
183-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImprove the failure diagnostics of this test.
Two small changes make a CI failure actionable.
First,
command.Output()discards stderr, so ago listfailure reports only the exit status. Capture stderr and include it.Second, the single
go list -depscall covers all listed packages at once, so the failure message cannot name the package that introducednet/http. Run one call per package and name it.♻️ Proposed change
root := repoRoot(t) - command := exec.Command("go", append([]string{"list", "-deps"}, inventoryPackages...)...) - command.Dir = root - command.Env = os.Environ() - output, err := command.Output() - if err != nil { - t.Fatalf("go list -deps failed: %v", err) - } - - for _, dependency := range strings.Fields(string(output)) { - if dependency == "net/http" { - t.Fatal("inventory resolution depends on net/http; wso2 version must resolve inventory from local state only") - } - } + for _, pkg := range inventoryPackages { + command := exec.Command("go", "list", "-deps", pkg) + command.Dir = root + command.Env = os.Environ() + var stderr bytes.Buffer + command.Stderr = &stderr + output, err := command.Output() + if err != nil { + t.Fatalf("go list -deps %s failed: %v: %s", pkg, err, stderr.String()) + } + for _, dependency := range strings.Fields(string(output)) { + if dependency == "net/http" { + t.Fatalf("%s depends on net/http; wso2 version must resolve inventory from local state only", pkg) + } + } + }Add
bytesto the import block.🤖 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 `@test/acceptance/acceptance_test.go` around lines 183 - 204, Improve TestVersionResolvesInventoryWithoutNetworkAccessInItsDependencyGraph by iterating over inventoryPackages and running go list -deps separately for each package, naming the package in failures and when net/http is detected. Capture stderr with bytes.Buffer while executing each command, and include its contents alongside the command error in t.Fatalf; add the bytes import.test/smoke/RUNNING.md (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fenced code blocks.
markdownlint reports MD040 on these four blocks. They contain command output, so
textis the correct fence language.Also applies to: 91-91, 135-135, 185-185
🤖 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 `@test/smoke/RUNNING.md` at line 69, Update the four fenced code blocks in RUNNING.md, including the blocks around the referenced sections, to specify text as their language identifier. Preserve their command-output contents unchanged.Source: Linters/SAST tools
test/smoke/verdict.go (1)
121-168: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider tightening two classification edges.
Two inputs reach a verdict that does not describe them precisely:
- A rendered list of
nonereturnsVerdictInconclusiveUnreadable. That constant is documented as "the refusal did not name the permissions the deployment issued", but in this case the deployment did name them: it issued none.VerdictIgnoreddescribes it, or a dedicated constant does.sameSetcompares length and membership only. A list carrying a duplicate, for example[a, a]against[a, b], compares equal. Current inputs are deduplicated, so this is a robustness gap rather than a present defect. Sorting both copies and usingslices.Equalremoves it.Both feed a verdict a human copies into a research document, so precision here has value beyond the run.
♻️ Proposed change to sameSet
// sameSet reports whether two permission lists carry the same members. func sameSet(left, right []string) bool { if len(left) != len(right) { return false } - for _, scope := range left { - if !slices.Contains(right, scope) { - return false - } - } - return true + sortedLeft, sortedRight := slices.Clone(left), slices.Clone(right) + slices.Sort(sortedLeft) + slices.Sort(sortedRight) + return slices.Equal(sortedLeft, sortedRight) }🤖 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 `@test/smoke/verdict.go` around lines 121 - 168, Update scopeMismatchVerdict so a parsed rendered value of “none” is classified as VerdictIgnored rather than VerdictInconclusiveUnreadable, while preserving unreadable handling for missing or invalid scope lists. Harden sameSet by copying and sorting both input slices, then comparing them with slices.Equal so duplicate members cannot produce a false match.test/acceptance/login_test.go (1)
439-462: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider guarding the goroutine reporting after the timeout branch.
If the
selectreachestime.Afterand callst.Fatal, the goroutine can still callt.Errorfanddeployment.status(t)after the test function returns. The Go testing package panics on logging from a goroutine after test completion, which replaces the intended timeout message with a panic. One option is to record the goroutine outcomes into variables and assert them only afterdoneis closed.Also applies to: 649-668
🤖 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 `@test/acceptance/login_test.go` around lines 439 - 462, Guard the goroutine in the login acceptance test so it cannot call t.Errorf or deployment.status(t) after the timeout path exits. Record the shell and status results, including any error details needed for assertions, in goroutine-owned outcomes; after <-done, assert those outcomes, while preserving the existing timeout failure and stderr checks.
🤖 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/architecture.md`:
- Around line 214-272: Update docs/architecture.md lines 214-272 to distinguish
the target behavior for browser PKCE, device authorization, client credentials,
and PAT from current first-slice execution: document browser PKCE and client
credentials as implemented, and device authorization and PAT as accepted
configuration that returns stable auth.kind_not_implemented. In
docs/examples/authentication-contexts.md lines 220-295, remove or clearly mark
the PAT login example as unavailable. In
docs/examples/authentication-contexts.md lines 427-456, update the
authentication-kind matrix and adapter description to reflect these
implementation statuses.
In `@docs/examples/authentication-contexts.md`:
- Line 13: Update the introductory schema wording in authentication-contexts.md
to remove the claim that field names are merely proposed, or replace it with the
supported schema-evolution and compatibility policy. Keep the documented field
names aligned with the serialization and validation behavior in
internal/contexts.
In `@docs/product-requirements.md`:
- Around line 178-183: Align the first-slice requirements in the device-code
sections around lines 178-183 and 457-468 and the namespace-binding section
around lines 265-270 with the implementation: mark device authorization and
namespace binding as deferred, unless implementing support in the corresponding
login and invoke flows before release. Ensure no first-slice requirement
promises unsupported --device-code, KindOAuthDevice, or namespace-resolution
behavior.
In `@docs/research/asgardeo-redirect-uri-and-scope-narrowing.md`:
- Around line 164-170: Remove the curl -k option from the refresh-token example
so the credentialed request performs standard TLS certificate verification while
preserving the existing endpoint, headers, and form data.
- Around line 262-273: Reconcile access-token audience guidance across all three
documented sites: in docs/research/asgardeo-redirect-uri-and-scope-narrowing.md
lines 262-273, preserve the measured unconfigured Asgardeo client-ID result and
keep it distinct from the audience-table guidance; in
docs/research/product-authentication-compatibility.md lines 156-157, describe
API-resource audiences as deployment-specific rather than universal; and in
docs/research/wso2-authentication-landscape.md lines 101-105, state that
configured or registered audiences use the API-resource identifier, while
otherwise the application client audience is used.
In `@docs/research/context-identity-model-feasibility.md`:
- Line 46: Label every affected fenced code block: in
docs/research/context-identity-model-feasibility.md at lines 46, 107, 118, and
157, add the appropriate language labels for the AWS configuration, kubeconfig
structure, shell command, and configuration-output blocks; in
docs/research/asgardeo-redirect-uri-and-scope-narrowing.md at line 164, label
the curl command block as sh.
In `@docs/research/product-authentication-compatibility.md`:
- Around line 32-36: Synchronize the research with the August 6 empirical
results: in docs/research/product-authentication-compatibility.md lines 32-36,
document supported any-port loopback redirects; lines 152-155, document honored
refresh scope narrowing; and lines 350-356, remove the completed empirical-test
prerequisite. In docs/research/asgardeo-redirect-uri-and-scope-narrowing.md
lines 305-308, remove the pending-verdict statement. In
docs/research/wso2-authentication-landscape.md lines 79-82, link to the measured
loopback result; line 428, update the compatibility-table qualifier; and lines
466-471, replace the obsolete loopback verification gap.
In `@internal/app/invoke_test.go`:
- Around line 173-208: Extend TestContextSelectionOrder and the associated
context-selection implementation to support a persisted namespace context
binding. Add a binding field to contexts.Document, validate it, and have
selection resolve in order: --context flag, WSO2_CONTEXT, recorded namespace
binding, then document default; include test cases covering binding selection
and each higher-priority source overriding it.
In `@internal/app/invoke.go`:
- Around line 211-214: Update the argument parsing branch in the surrounding
context-selection function so a second argument beginning with "-" is treated as
a missing --context value, returning the existing missing-value status instead
of using it as the context name. Preserve the current behavior for valid
non-option context names and for calls with fewer than two arguments.
In `@internal/auth/fakeissuer/fakeissuer.go`:
- Around line 407-447: Update refreshGrant to keep i.mutex held from the
presented-token lookup through refresh-token rotation, so concurrent requests
cannot both consume and rotate the same token. Ensure the critical section does
not retire the token before scope validation when rejected refreshes must leave
it usable; perform the scope decision before locking or otherwise preserve that
behavior.
In `@internal/auth/narrowing.go`:
- Around line 36-41: The tokenResponse.expiry method must not return a zero or
epoch expiry when neither ExpiresIn nor facts.ExpiresAt provides lifetime
metadata. Update the expiry/grant-validation flow around tokenResponse.expiry to
reject such grants, while preserving valid ExpiresIn and access-token exp
handling.
In `@internal/auth/session/lock.go`:
- Around line 70-76: Replace the file-existence/stale-removal takeover logic in
the lock implementation with an OS-level advisory lock using
github.com/gofrs/flock, ensuring acquisition remains atomic across processes and
removing the stale file deletion path. In internal/auth/session/lock.go lines
70-76, update the lock acquisition flow around the existing lock function and
preserve execution of fn only after successful acquisition. In
internal/auth/session/session_test.go lines 162-185, keep the stale-lock
takeover test and add a separate in-process concurrency test covering stale
files to verify only one invocation can run at a time.
In `@internal/auth/source_clientcred.go`:
- Around line 120-129: The errors.As(err, &refusal) branch in the token request
handling should use a grant-refusal code such as auth.access_not_issued instead
of auth.discovery_failed. Keep the existing refusal message and remediation text
unchanged, while preserving auth.discovery_failed for unreachable token
endpoints.
In `@internal/contexts/contexts_test.go`:
- Around line 128-160: The TestValidateV2 case for “unknown kind rejected”
currently expects Decode to fail, but decoding must accept unsupported identity
kinds so other contexts remain readable. Update the validation and selection
flow around contexts.Decode to allow unknown kinds during document decoding, and
return the stable unsupported-kind refusal only when that identity is selected;
adjust the test expectation accordingly while preserving malformed-document
checks for other cases.
In `@test/smoke/config.go`:
- Around line 167-184: Update the required-variable validation in Load to check
the parsed config.Scopes value rather than read(ScopeVar), so separator-only
scope strings are treated as missing. Preserve the existing missing-variable
reporting and validation for Issuer, ClientID, and Audience.
In `@test/smoke/RUNNING.md`:
- Around line 213-268: Add entries to “Reading the narrowing verdict” for
VerdictInconclusiveUnstated (`inconclusive (deployment stated no scope)`) and
VerdictInconclusiveUnreadable (`inconclusive (unrecognized narrowing refusal)`),
describing what each result means and how the reader should record or follow up
on it. Keep the existing verdict guidance unchanged.
---
Nitpick comments:
In `@internal/auth/oauthflow/jwks.go`:
- Around line 132-142: Bound the body read in RoundTrip by wrapping
response.Body with an io.LimitReader using a generous maximum size before
io.ReadAll. Detect when the limit is exceeded and return an appropriate error
instead of forwarding truncated content, while preserving normal handling of
successful small responses and body closure.
In `@internal/auth/oauthflow/login.go`:
- Around line 400-402: Update the http.Server initialization in the OAuth
callback flow around waiting.server to set ReadTimeout, WriteTimeout, and
IdleTimeout in addition to the existing ReadHeaderTimeout, using appropriate
established timeout values or constants. Preserve the existing handler,
listener, and Serve behavior.
In `@internal/auth/session/session_test.go`:
- Around line 128-135: Update the peak-concurrency tracking in the WithLock
callback to use an atomic compare-and-swap loop when raising maxInside, retrying
until the observed value is already at least now or the CAS succeeds. Preserve
the existing inside counter updates and lock-concurrency assertion.
In `@test/acceptance/acceptance_test.go`:
- Around line 183-204: Improve
TestVersionResolvesInventoryWithoutNetworkAccessInItsDependencyGraph by
iterating over inventoryPackages and running go list -deps separately for each
package, naming the package in failures and when net/http is detected. Capture
stderr with bytes.Buffer while executing each command, and include its contents
alongside the command error in t.Fatalf; add the bytes import.
In `@test/acceptance/login_test.go`:
- Around line 439-462: Guard the goroutine in the login acceptance test so it
cannot call t.Errorf or deployment.status(t) after the timeout path exits.
Record the shell and status results, including any error details needed for
assertions, in goroutine-owned outcomes; after <-done, assert those outcomes,
while preserving the existing timeout failure and stderr checks.
In `@test/smoke/RUNNING.md`:
- Line 69: Update the four fenced code blocks in RUNNING.md, including the
blocks around the referenced sections, to specify text as their language
identifier. Preserve their command-output contents unchanged.
In `@test/smoke/verdict.go`:
- Around line 121-168: Update scopeMismatchVerdict so a parsed rendered value of
“none” is classified as VerdictIgnored rather than
VerdictInconclusiveUnreadable, while preserving unreadable handling for missing
or invalid scope lists. Harden sameSet by copying and sorting both input slices,
then comparing them with slices.Equal so duplicate members cannot produce a
false match.
🪄 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: fac28cf8-86ba-403c-8dea-6b9f237f5953
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (64)
.gitignoreMakefiledocs/README.mddocs/adr/0004-shell-brokered-authentication.mddocs/architecture.mddocs/examples/authentication-contexts.mddocs/examples/login-walkthroughs.mddocs/guides/login.mddocs/plans/login-first-slice.mddocs/product-requirements.mddocs/research/README.mddocs/research/asgardeo-redirect-uri-and-scope-narrowing.mddocs/research/context-identity-model-feasibility.mddocs/research/product-authentication-compatibility.mddocs/research/wso2-authentication-landscape.mdgo.modinternal/app/app.gointernal/app/invoke.gointernal/app/invoke_test.gointernal/app/login.gointernal/app/login_deadline_test.gointernal/app/login_helpers_test.gointernal/app/login_test.gointernal/auth/auth.gointernal/auth/auth_test.gointernal/auth/claims.gointernal/auth/discovery.gointernal/auth/fakeissuer/fakeissuer.gointernal/auth/fakeissuer/fakeissuer_test.gointernal/auth/narrowing.gointernal/auth/oauthflow/browser.gointernal/auth/oauthflow/browser_test.gointernal/auth/oauthflow/jwks.gointernal/auth/oauthflow/jwks_internal_test.gointernal/auth/oauthflow/login.gointernal/auth/oauthflow/login_test.gointernal/auth/session/lock.gointernal/auth/session/session.gointernal/auth/session/session_test.gointernal/auth/source.gointernal/auth/source_browser.gointernal/auth/source_browser_test.gointernal/auth/source_clientcred.gointernal/auth/source_clientcred_test.gointernal/auth/source_dev.gointernal/auth/tokenrequest.gointernal/contexts/contexts.gointernal/contexts/contexts_test.gointernal/contexts/fixture/fixture.gointernal/contexts/identity.gointernal/contexts/legacy.gointernal/contexts/legacy_test.gotest/acceptance/acceptance_test.gotest/acceptance/broker_test.gotest/acceptance/login_test.gotest/smoke/RUNNING.mdtest/smoke/asgardeo_empirical_test.gotest/smoke/config.gotest/smoke/config_test.gotest/smoke/env.exampletest/smoke/harness_smoke_test.gotest/smoke/login_smoke_test.gotest/smoke/verdict.gotest/smoke/verdict_test.go
Four defects, each of which could pass unnoticed because the check meant to catch it could not fail. `test/smoke/config.go` required a non-empty WSO2_SMOKE_SCOPE by looking at the raw variable rather than the parsed list, so a value of "," passed and a run reached a live browser sign-in asking for no permissions at all. `internal/app/invoke.go` took whatever followed --context as the context name, so `--context --output json` refused as an unknown context named "--output" and sent the reader looking for a context they never asked for, rather than at the flag they left empty. No context name may begin with "-", so rejecting one cannot swallow a real name. `internal/auth/narrowing.go` accepted a grant whose lifetime nobody stated. When the response omits expires_in — only RECOMMENDED by RFC 6749 section 5.1 — and the token claims no exp, the expiry is the zero time, which reaches a module as an epoch expiry and reads as access that died in 1970. It now refuses. `internal/auth/fakeissuer` looked a refresh token up in one critical section and rotated it in another, so two concurrent refreshes presenting the same token both passed the lookup and both rotated. That matters more in a fixture than it would in a product: the rotation assertions in test/acceptance/login_test.go and source_browser_test.go treat this issuer as the oracle for "the previous refresh token is dead", and an oracle admitting the replay would let a real double-rotation defect through. The scope decision moved inside the same lock, because a refusal must leave the presented token alive. Both new tests were checked against the unfixed code and fail there. The documentation findings were the same shape: text that promised more than the build does. The architecture, the product requirements and the context examples described device authorization, personal access tokens and namespace binding as though they were available; all three are deferred, and the first two refuse at use with auth.kind_not_implemented. Each now says what ships today without rewriting the requirement, because these are targets for the product rather than descriptions of the current build. The context examples also called the schema field names proposed after the schema shipped. The research documents gain dated pointers to the 2026-08-06 measurements rather than edits to their findings. Those documents record what public sources said at their research date, and that is exactly what makes a later measurement worth having — overwriting it would destroy the distinction the whole set rests on. The quoted Asgardeo `curl -k` sample stays verbatim for the same reason: the section is evidence about what that published sample contains, so it is annotated as not-to-be-copied instead of corrected. Two review findings are not acted on and are answered on their threads: a document naming a kind the schema does not know still fails closed, which is distinct from a kind this slice defers; and the namespace-binding tests cover what resolution actually does, the documentation having been the thing that overpromised. Claude-Session: https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp
sachiniSam
approved these changes
Aug 6, 2026
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
2 participants
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.
Closes #17.
The
wso2 loginfirst slice. 45 commits, 65 files, and two ways to authenticate that did not exist before: a browser login a developer runs once, and an inline client-credentials acquisition a CI job never has to think about.What a developer gets
A browser opens for Authorization Code + PKCE, the authorization URL is always printed so a machine that cannot open a browser is still usable, the refresh token goes into the OS secure store, and every later command refreshes silently. CI sets one environment variable and runs the command — no login step, no persisted session, the secret never leaving process memory.
The design decision worth reviewing
The broker verifies rather than assumes. When a module asks for one permission out of the several a session holds, the shell does not take the deployment's word that it narrowed the grant. It checks that the issued scopes equal the request and that the token's audience covers what was asked, and refuses with
auth.narrowing_unavailablewhen it cannot prove either.That refusal is the designed outcome, not a degraded mode. A module that silently received the whole session's authority would hold access nobody decided to give it. Every failure in this slice carries a stable problem code and the auth-policy exit class, so a script can tell "you may not" from "it is broken" without parsing prose.
What is in it
internal/contextsinternal/auth/sessioninternal/auth/oauthflowinternal/authSessions live under one secure-store entry per credential reference. Nothing in a context document can hold a credential; credential-shaped values are rejected on read, so writing configuration grants nothing.
Tested
103 test functions in files this slice adds, on top of the existing gate.
The definitive one is
TestLoginThenTheModuleReceivesIssuerMintedNarrowedAccess: it logs in, runs the real reference module as a subprocess, and has the issuer introspect the token that module presented — proving it carried exactly the one permission requested out of the several the session held, bound to the audience it named. It runs three times to prove each refresh-token rotation was persisted, then restores a superseded token and proves it is refused.Backing that: a fake OIDC issuer that signs real JWTs with knobs for honor/ignore/reject narrowing and token rotation; disclosure sweeps asserting no token material reaches stdout, stderr, problems, logs, module environments, or any file left behind; and v1-compatibility tests so the architecture proof keeps passing unmodified.
Proven against real deployments
Both products the issue names, both measured 2026-08-06 and recorded in the research documents:
audThat last row is the one finding that outran the plan, and it is why #37 exists: the definition-of-done clause about audiences is satisfiable on one supported product and structurally not on the other. Nothing in the broker needs to change — it verifies rather than assumes, which is correct either way — but what
products.<namespace>.audiencecan mean differs by product. That is a design position to take deliberately, not a measurement still outstanding.Two live findings became fixes rather than documentation:
Documentation
docs/guides/login.mdtakes a reader from an empty deployment to a working login and a CI job: registering the application in Asgardeo (§2) or Identity Server 7.x (§3, with a one-line container recipe), writing the context document by hand (§4), logging in (§5), what got stored and where (§6), the CI path (§7), one troubleshooting entry per problem code (§8), and how to prove it against a real deployment (§9).Section 3.6 documents the TLS step for a local Identity Server including what it costs: the default certificate is
CA:TRUEand its private key ships in every download and every copy of the public image behind a published password, so the section gives the removal command beside the setup command.docs/plans/login-first-slice.mdis fully ticked, definition of done included, with each entry carrying what closed it.Verification
The live targets are behind the
smokebuild tag and read a per-deployment env file; CI never builds them.Out of scope, as the issue defines it
Device Authorization Grant and personal access tokens validate as configuration and refuse at use with a stable not-implemented code. Context authoring UX, workspace routing, organization-switch token exchange, and fresh-machine cloud bootstrap are all later slices. The module contract is unchanged: modules request an audience and scopes and receive access material, nothing more.
The standing backend ask — a WSO2-seeded, well-known
wso2clipublic client — is recorded indocs/research/product-authentication-compatibility.md. Until it exists, registration is manual per tenant, which is what the walkthrough covers.https://claude.ai/code/session_01RojiAgW9hi3b9f9G6ZXBVp