diff --git a/docs/guides/login.md b/docs/guides/login.md index 3367203..2f53c35 100644 --- a/docs/guides/login.md +++ b/docs/guides/login.md @@ -1,7 +1,7 @@ # Logging in with the WSO2 CLI **Status:** Working draft -**Last reviewed:** 2026-08-05 +**Last reviewed:** 2026-08-06 **Related:** [Architecture](../architecture.md), [product requirements](../product-requirements.md), [shell commands](../reference/commands.md), @@ -87,30 +87,83 @@ of the URLs listed in section 1.3, one at a time. Asgardeo matches redirect URIs exactly by default, so a missing entry becomes a mismatch error for whichever developer's machine happens to have that port busy. -Whether Asgardeo waives the port for loopback addresses the way Identity Server -6.0.0 and later document is -[not settled from public sources](../research/asgardeo-redirect-uri-and-scope-narrowing.md). -Register all four regardless; it costs nothing and does not depend on the answer. +Asgardeo does in fact waive the port when matching loopback redirect URIs, the +way Identity Server 6.0.0 and later document and as RFC 8252 §7.3 asks. That was +[measured against a live tenant on 2026-08-06](../research/asgardeo-redirect-uri-and-scope-narrowing.md): +a login completed through `127.0.0.1:16000`, a port the application did not +register. + +Register all four anyway. The verdict was measured on one tenant, it is +undocumented by Asgardeo and so may change without notice, and the shell binds +only these four ports regardless — so nothing is gained by registering fewer, +and a deployment that stops waiving the port breaks every developer whose first +choice is busy. ### 2.4 Add the API resource and its scopes The audience a module asks for is an API resource identifier, and the permissions it asks for are that resource's scopes. -1. **API Resources → New API Resource**. -2. Give it an **Identifier**. This exact string is the audience — the shell - checks the issued token's `aud` claim against it. Record it. -3. Add the scopes the module needs, for example `reference:status:read` and - `reference:status:write`. -4. Back on the application's **API Authorization** tab, authorize the resource - and select those scopes. +This is two screens, not one. An API resource is an organization-level object +that many applications can share, so it is created outside your application; +authorizing it *for* your application is a separate step afterwards. + +**First, create the resource.** **API Resources** is a top-level item in the +Console's left navigation — a sibling of Applications, not a tab inside the one +you just made. -### 2.5 Issue JWT access tokens +1. **API Resources → New API Resource**. +2. Give it an **Identifier** and record it. This is the string a module's + `audience` names. Read section 2.5 before assuming it is also what lands in + an issued token's `aud` claim on Asgardeo — it is not. +3. Give it a **Display Name**. This is what a user sees on a consent screen. +4. Add the scopes the module needs, for example `reference:status:read` and + `reference:status:write`. Register at least two even when the module only + uses one: the narrowing experiment in section 9 works by asking for a strict + subset of what a session carries, and it has nothing to measure against a + single scope. +5. The wizard's last step offers **Requires authorization**, checked by default. + **This field cannot be changed after the resource is created.** Checked means + these scopes only ever reach a token through a role. Clear it if you want the + application's own authorization to be enough on its own. Section 2.7 covers + the role path, which is also the way out if you left it checked. + +**Then authorize it on the application.** Back in **Applications → your +application → Authorization → Authorize resource**: select the resource, then +select its scopes. + +Watch the policy shown beside the resource on that tab. It can read +`Role Based Access Control (RBAC)` even when the resource itself did not require +authorization — the resource setting decides whether a policy is *mandatory*, +and this tab is where one is actually chosen. `No Authorization Policy` means +the scopes selected here are sufficient by themselves. Anything else means +section 2.7 applies, and skipping it produces a login that succeeds followed by +a refusal that names scopes rather than roles. + +### 2.5 Issue JWT access tokens, and know what `aud` will say On the application's **Protocol** tab, under **Access Token**, set the token type to **JWT**. An opaque access token cannot be checked, and the broker refuses what it cannot check. +**Asgardeo binds an access token's `aud` claim to the client ID, not to the API +resource whose scopes the token carries.** Measured against a live tenant on +2026-08-06: a token issued for `reference:status:read reference:status:write`, +from an application authorized against the `reference-status` API resource, +carried `"aud": ""` and nothing else. There is no setting for this. +The **Access Token** section offers only a token type and an attribute list; the +Audience field you will find nearby belongs to **ID Token** and does not affect +access tokens. + +So on Asgardeo, `products..audience` in your context document must be +**the client ID**, not the API resource identifier, or every brokered +acquisition refuses with `auth.narrowing_unavailable`. Section 4.3 says the same +where the field is defined, and the consequence is recorded in +[the research document](../research/asgardeo-redirect-uri-and-scope-narrowing.md): +the audience check still proves a token was minted for this client, but it +cannot distinguish one product from another. Whether Identity Server 7.x behaves +this way is not yet measured. + ### 2.6 Record what you need From the **Protocol** and **Info** tabs: @@ -124,6 +177,42 @@ From the **Protocol** and **Info** tabs: from that document and checks that the document belongs to the issuer it was fetched from, so a value that is close but not exact fails at login. +### 2.7 Create a user who can sign in, and grant it the scopes + +**The account you sign in to the Console with is not, by default, an account +your application can authenticate.** Console access and application sign-in are +two different populations: your own account administers the organization, while +what the application asks for is a user in the organization's user store. If you +signed up through Google or GitHub there is no password in that store at all, +and no amount of typing your real one will work. + +Create a user for this instead: + +1. **User Management → Users → Add User** — *Users*, not *Administrators*. +2. Give it a username or email, for example `cli-smoke@example.com`. +3. Choose to **set a password directly** rather than emailing an invitation. The + invitation path needs a working inbox, and login waits only five minutes. + +**If — and only if — section 2.4 left you with an authorization policy**, that +user also needs a role carrying the scopes. Authorizing the resource on the +application establishes what the application *may* ask for; under a policy it +does not establish what a user is *entitled to*, and the gap surfaces at the +first brokered acquisition as `auth.narrowing_unavailable` naming permissions. + +1. **Applications → your application → Roles → New Role**, with **Role Audience** + set to **Application**. +2. Attach the API resource and select **every** scope the context document + lists, not just the one a module uses — a session that carries less than it + later asks for cannot be narrowed. +3. Assign the user to that role, from the role's users list or from + **User Management → Users → your user → Roles**. + +A console change never reaches an existing session. Sign in again after either +step — and note that a browser SSO session will complete that sign-in without +showing you a login form, which is expected and does not mean the change was +skipped. Scopes are computed when a token is issued, not frozen into the browser +session. + --- ## 3. Register the application in Identity Server 7.x @@ -159,6 +248,9 @@ configuration valid on Asgardeo, where that behavior is not documented. **API Resources → New API Resource**, with an identifier and scopes as in section 2.4, then authorize it on the application's **API Authorization** tab. +Section 2.4's two warnings apply here too: the resource is created on a +different screen than the one that authorizes it, and the **Requires +authorization** setting cannot be changed afterwards. ### 3.5 Issue JWT access tokens @@ -166,6 +258,16 @@ Identity Server issues JWT access tokens by default. If the deployment has been changed to opaque, change it back for this application — see section 2.5 for why. +**What Identity Server puts in a token's `aud` claim is not yet measured.** +Section 2.5 records that Asgardeo binds it to the client ID rather than the API +resource; whether Identity Server does the same is an open question. Register +the API resource identifier as your `audience` first, and if brokered +acquisition refuses with `auth.narrowing_unavailable` naming the audience, the +client ID is the value to try instead. Either outcome is worth recording in +[the research document](../research/asgardeo-redirect-uri-and-scope-narrowing.md), +because it decides whether that clause of the broker's policy can mean anything +product-specific at all. + ### 3.6 Record what you need - **Client ID**. @@ -178,6 +280,10 @@ why. not in the OS trust store fails discovery. See `auth.discovery_failed` in section 8. +You also need a user to sign in as, and possibly a role granting the scopes. +Section 2.7 describes both; the reasoning is identical on Identity Server, only +the console differs. + --- ## 4. Write the context document @@ -244,7 +350,11 @@ Replace: - `issuer` — the value you confirmed in section 2.6 or 3.6. - `clientId` — the client ID you recorded. -- `audience` — the API resource identifier from section 2.4 or 3.4. +- `audience` — **on Asgardeo, the client ID again**, because that is the only + value Asgardeo puts in an access token's `aud` claim (section 2.5). On a + deployment that binds tokens to API resources, the resource identifier from + section 2.4 or 3.4. The example above shows the resource-identifier form, so + against Asgardeo it needs the client ID substituted here. - `scopes` — the scopes you authorized on the application. For an Identity Server deployment, also set `"type": "onprem"` and use the @@ -265,7 +375,7 @@ For an Identity Server deployment, also set `"type": "onprem"` and use the | `auth.credentialRef` | The name the session is stored under in the OS secure store. **Required** for `oauth-browser`; **not allowed** for `client-credentials`. Same character rules as an identity name. | | `products.` | What this identity may reach for one module. The namespace is the module's own name, and follows the same character rules as an identity name. | | `products..endpoint` | The product's base URL. **Required** on every product entry, and must be an absolute `http` or `https` URL with a host. | -| `products..audience` | The API resource identifier. A module asking for any other audience is refused. | +| `products..audience` | What the issued token's `aud` claim must carry. A module asking for any other audience is refused. Conceptually this is the API resource identifier — but on Asgardeo it must be **the client ID**, because that is the only thing Asgardeo puts in `aud`. See section 2.5. | | `products..scopes` | The permissions this identity carries. A module asking for one that is not listed is refused. | | `contexts[].organization` | The organization to act within. Either leave it out, or set it to the identity's `auth.tenant` — this release cannot switch a session out of its home tenant, and any other value is refused. See `auth.organization_switch_unsupported` in section 8. | @@ -403,6 +513,12 @@ Two differences from section 4.2, and the schema enforces both: The secret itself never goes in this file, and the file is safe to commit. +`audience` follows the same rule as section 4.2: on Asgardeo it must be the M2M +application's own client ID, not the API resource identifier the example shows. +Under RBAC there is one further difference from a browser login — a +client-credentials grant has no user, so a role granting the scopes must be +assigned to the **application** rather than to a person. + ### 7.3 Wire the job The secret comes from the CI system's own secret store into the named variable. @@ -586,6 +702,42 @@ On a browser login: the flow ended without producing tokens — you closed the browser, the consent was denied, or the deployment redirected back with an error. +The browser reached "Login complete" and the code exchange succeeded, but the +identity token that came back was not one the shell would accept. The message +says which kind of failure it was: + +| The message says | What it means | What to change | +| --- | --- | --- | +| "was not signed by the identity provider's keys" | The signature did not check out against the key set the issuer publishes. | Usually the `issuer` in your context document names a different deployment than the one that signed you in. | +| "was issued for a different application" | The token's `aud` does not carry your `clientId`. | Confirm `clientId` names the application this issuer signed you in to. | +| "had already expired" | The token was outside its validity window on arrival. | Check this machine's clock. | +| "the shell could not read the signing keys the identity provider publishes" | The key set could not be fetched or parsed. | Confirm the machine can reach the issuer's `jwks_uri`. If it is reachable, see below. | + +That last one has a known cause worth naming, because it is not your +configuration. Many WSO2 deployments — Asgardeo tenants and Identity Servers +alike — publish a token-signing certificate whose X.509 serial number is +negative, which RFC 5280 forbids and which Go has rejected since 1.23. The +certificate travels in the `x5c` field of the JWKS, and a library that parses +it eagerly fails the entire key set over it. + +**The shell no longer reads that field.** A key's own parameters describe it +completely, so the certificate beside it is discarded before anything tries to +parse it, and such a deployment logs in normally. Nothing needs to be set, and +in particular the `GODEBUG=x509negativeserial=1` workaround that circulated +before this was fixed is no longer required. + +If you want to confirm a deployment has such a certificate — a leading minus +sign on the serial is the whole diagnosis: + +```sh +curl -s "$(curl -s /.well-known/openid-configuration | python3 -c 'import json,sys; print(json.load(sys.stdin)["jwks_uri"])')" \ + | python3 -c 'import base64,json,sys; sys.stdout.buffer.write(base64.b64decode(json.load(sys.stdin)["keys"][0]["x5c"][0]))' \ + | openssl x509 -inform der -noout -serial +``` + +A serial printed as, for example, `serial=-3A4F8369` is that defect. It no +longer stops a login. + ### `auth.login_not_required` You ran `wso2 login` on a context whose identity carries its own credential. @@ -611,13 +763,88 @@ now names. You changed the `issuer` after logging in. Run `wso2 login` again. ## 9. Proving it against a real deployment This repository ships a live smoke run and two one-time experiments, both behind -the `smoke` build tag so they never execute in the default test gate. +the `smoke` build tag so they never execute in the default test gate. Neither +touches your own `~/.wso2`: they write a context document into a temporary state +root and store their session under the secure-store reference `wso2-cli-smoke`, +deleted before and after every run. + +### 9.1 First, the runs that need no deployment + +Nothing below is worth a browser sign-in until these pass. The deterministic +suite already drives the whole chain — login, session, brokered acquisition — +against a fake OIDC issuer that signs real JWTs, so what the live runs add is +evidence about a *deployment*, not about the shell. + +```sh +make test # the default gate, including the acceptance suite +make acceptance # the architecture-proof gate +make smoke-build # proves the live runs still compile against the shell +make lint +``` + +Confirm the live runs skip honestly while you are still unconfigured: + +```sh +go test -tags smoke ./test/smoke -run TestLoginSmoke -v +# --- SKIP: TestLoginSmoke — no live deployment is configured: set WSO2_SMOKE_ISSUER, ... +``` + +### 9.2 Describe the deployment + +```sh +export WSO2_SMOKE_ISSUER='https://api.asgardeo.io/t//oauth2/token' +export WSO2_SMOKE_CLIENT_ID='' +export WSO2_SMOKE_AUDIENCE='' # on Asgardeo — see section 2.5 +export WSO2_SMOKE_SCOPE='reference:status:read reference:status:write' +``` + +`WSO2_SMOKE_CLIENT_ID` and `WSO2_SMOKE_AUDIENCE` are different fields that +Asgardeo happens to force to the same value: the first says who is asking, the +second says what the issued token must be bound to. On a deployment that binds +tokens to API resources they differ, and the second is the resource identifier +from section 2.4. + +Confirm the issuer against the deployment's own document before spending a +sign-in on a value that is close but not exact: + +```sh +curl -s "$WSO2_SMOKE_ISSUER/.well-known/openid-configuration" \ + | python3 -c 'import json,sys; d=json.load(sys.stdin); print(d["issuer"]); print(d["code_challenge_methods_supported"])' +``` + +The printed issuer must equal `WSO2_SMOKE_ISSUER` character for character, and +`S256` must appear. Those are the two most common reasons a first login fails +before it reaches a browser. + +### 9.3 The live runs ```sh make smoke-login # log in, prove the session persisted, broker one acquisition make empirical-asgardeo # answer the two open questions about Asgardeo's behavior ``` -Both skip cleanly when no deployment is configured. -[`test/smoke/RUNNING.md`](../../test/smoke/RUNNING.md) lists the environment -variables they read and explains how to read and record their verdicts. +A passing smoke run ends with the acquisition granted: + +``` +LOGIN SMOKE: granted — access of 1219 characters bound to "", expiring 20:07:22Z +``` + +A run that ends in `auth.narrowing_unavailable` **also passes**, and that is +deliberate: the shell refusing to hand a module more authority than it asked for +is the designed outcome, not a fallback. Section 8 decodes which of the five +narrowing refusals you got. + +The experiments print one verdict line each. Their answers belong in section 3 +of +[`docs/research/asgardeo-redirect-uri-and-scope-narrowing.md`](../research/asgardeo-redirect-uri-and-scope-narrowing.md), +with the date and the `deployment:` line the run printed beneath each verdict — +the verdicts are per-deployment and a second tenant is not covered by the first +one's cells. Both questions were answered against a live Asgardeo tenant on +2026-08-06: any-port loopback **supported**, refresh narrowing **honored**. + +Read +[`test/smoke/RUNNING.md`](../../test/smoke/RUNNING.md) before recording +anything. It lists every variable these runs read and, more importantly, +explains which verdicts are catch-all branches that need corroborating — an +`ASGARDEO ANY-PORT LOOPBACK: rejected` is what the experiment prints for *any* +login that did not complete, including one where you simply closed the browser. diff --git a/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md b/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md index db34591..ca5cb03 100644 --- a/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md +++ b/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md @@ -242,19 +242,37 @@ negative finding, and nothing in the shell should be designed as though it were. | Question | Confirmed (Asgardeo docs) | Inferred (WSO2 IS only) | Empirical verdict | |---|---|---|---| -| Fixed-port loopback (`127.0.0.1:`) registrable | `localhost:` proven registrable via quickstart; REST schema imposes no blocking restriction | — | **Pending live run.** Any successful `make smoke-login` answers this incidentally: the walkthrough registers the literal `127.0.0.1` form on all four ports and the login binds one of them. Record: verdict, date, deployment. | -| Any-port loopback (RFC 8252 §7.3) | Not documented at all | IS 6.0.0+: exact port match waived for loopback IPs | **Pending live run.** `make empirical-asgardeo`, experiment A. Record the `ASGARDEO ANY-PORT LOOPBACK: {supported\|rejected}` verdict, its date, and the `deployment:` line the run printed under it. | +| Fixed-port loopback (`127.0.0.1:`) registrable | `localhost:` proven registrable via quickstart; REST schema imposes no blocking restriction | — | **Registrable. 2026-08-06**, deployment `https://api.asgardeo.io/t/kanushka/oauth2/token`. Answered incidentally by a passing `make smoke-login`: all four `127.0.0.1` callback URLs were registered literally and the login bound and returned to `127.0.0.1:10425`. | +| Any-port loopback (RFC 8252 §7.3) | Not documented at all | IS 6.0.0+: exact port match waived for loopback IPs | **Supported. 2026-08-06**, deployment `https://api.asgardeo.io/t/kanushka/oauth2/token`. `make empirical-asgardeo` experiment A completed a login through `127.0.0.1:16000`, a port the application does not register, so Asgardeo waives the port when matching loopback redirect URIs as RFC 8252 §7.3 asks. Matches the IS 6.0.0+ inference. | | Redirect URI validation rules | Exact match by default; `regexp=(url1\|url2)` prefix for OR-ing multiple exact URLs | Regex support IS-version-gated (5.2.0+); loopback flexibility IS-version-gated (6.0.0+) | **Not measured, and no experiment planned.** The open part is whether a true single-URL wildcard syntax exists, and an experiment can only ever fail to find one — absence of a syntax is not observable by trying one. This stays a documentation question. | -| Refresh-grant scope narrowing | Docs show no `scope` param on refresh_token grant at all (asymmetric vs. client_credentials/password sections, which do show one) | `RefreshGrantHandler.validateScope()`: subset requests honored and narrow the token; over-broad requests rejected with `invalid_scope`; omitted scope keeps full original grant | **Pending live run.** `make empirical-asgardeo`, experiment B. Record the `ASGARDEO REFRESH NARROWING: {honored\|ignored\|rejected}` verdict, its date, and the `deployment:` line. | +| Refresh-grant scope narrowing | Docs show no `scope` param on refresh_token grant at all (asymmetric vs. client_credentials/password sections, which do show one) | `RefreshGrantHandler.validateScope()`: subset requests honored and narrow the token; over-broad requests rejected with `invalid_scope`; omitted scope keeps full original grant | **Honored. 2026-08-06**, deployment `https://api.asgardeo.io/t/kanushka/oauth2/token`. `make empirical-asgardeo` experiment B established a session for `reference:status:read reference:status:write`, then ran a refresh grant for `reference:status:read` alone and received exactly that one permission — not the plain verdict's qualified form, so no protocol scopes were retained either. Matches the IS-source inference. | -Both questions remain genuinely open for Asgardeo specifically; the WSO2 IS -evidence is suggestive (shared codebase lineage, per the parent document's -landscape findings) but not a substitute for a live test against an Asgardeo -tenant. The broker decision does not assume Asgardeo parity with IS on either -point: the shell verifies the narrowing it asked for and refuses -(`auth.narrowing_unavailable`) when it cannot prove it, which is the behavior -that is correct under every one of the three possible verdicts rather than the -behavior that bets on one. +Both questions are now answered for Asgardeo, and both answers match what the +WSO2 IS source suggested — the shared codebase lineage held. Measured +2026-08-06 against `https://api.asgardeo.io/t/kanushka/oauth2/token`; the +verdicts are per-deployment, so a second tenant is not covered by these cells. + +The broker decision never assumed that parity: the shell verifies the narrowing +it asked for and refuses (`auth.narrowing_unavailable`) when it cannot prove it, +which is correct under every one of the three possible verdicts rather than the +behavior that bets on one. The favorable verdict does not change that design; it +means the refusal path is now the exceptional one on Asgardeo rather than the +expected one. + +**A third finding, not anticipated by this document.** The same runs established +that Asgardeo binds a JWT access token's `aud` claim to the **client ID**, never +to the API resource identifier whose scopes the token carries. An access token +issued for `reference:status:read reference:status:write` against an application +authorized for the `reference-status` API resource carried +`"aud": ""` and nothing else. Asgardeo exposes an Audience field only +under **ID Token** in the application's Protocol tab; there is no equivalent for +access tokens, because the value is not configurable. The consequence for broker +policy is direct: on Asgardeo the only value that can satisfy the shell's +audience check is the client ID, so `products..audience` cannot carry +product-level meaning there, and the check cannot distinguish a token brokered +for one namespace from one brokered for another. Whether Identity Server 7.x +behaves the same way is unmeasured and is the open question this finding +replaces the previous two with. ## 4. Producing and recording the verdicts diff --git a/internal/auth/fakeissuer/fakeissuer.go b/internal/auth/fakeissuer/fakeissuer.go index 108ab22..5f3ef16 100644 --- a/internal/auth/fakeissuer/fakeissuer.go +++ b/internal/auth/fakeissuer/fakeissuer.go @@ -29,10 +29,14 @@ import ( "crypto/rsa" "crypto/sha256" "crypto/subtle" + "crypto/x509" + "crypto/x509/pkix" "encoding/base64" + "encoding/binary" "encoding/hex" "encoding/json" "fmt" + "math/big" "net/http" "net/http/httptest" "net/url" @@ -90,6 +94,15 @@ type Options struct { // OmitRefreshToken answers the authorization code grant without a refresh // token, modeling an application that was never granted offline access. OmitRefreshToken bool + // NegativeSerialCertificate publishes an x5c certificate chain on the JWKS + // key whose certificate carries a negative serial number: forbidden by RFC + // 5280 section 4.1.2.2, emitted by WSO2 deployments for years, and rejected + // outright by Go's x509 parser since 1.23. + // + // The signing key itself stays valid — n and e are untouched — so this + // models the real failure exactly: a deployment whose keys can verify a + // token perfectly well, behind a certificate that nothing needs to read. + NegativeSerialCertificate bool } // Issuer is one running fake issuer. Its URL doubles as the issuer identifier. @@ -100,6 +113,9 @@ type Issuer struct { key *rsa.PrivateKey keyID string client *http.Client + // certificate is the DER published in the key's x5c chain, empty unless a + // test asked for one. + certificate []byte mutex sync.Mutex codes map[string]codeGrant @@ -142,6 +158,9 @@ func New(t *testing.T, opts Options) *Issuer { refreshTokens: map[string][]string{}, accessTokens: map[string]tokenRecord{}, } + if opts.NegativeSerialCertificate { + issuer.certificate = negativeSerialCertificate(t, key) + } mux := http.NewServeMux() mux.HandleFunc("GET /.well-known/openid-configuration", issuer.handleDiscovery) mux.HandleFunc("GET /jwks", issuer.handleJWKS) @@ -210,9 +229,79 @@ func (i *Issuer) handleDiscovery(w http.ResponseWriter, _ *http.Request) { } func (i *Issuer) handleJWKS(w http.ResponseWriter, _ *http.Request) { - writeJSON(w, http.StatusOK, jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + key := jose.JSONWebKey{ Key: i.key.Public(), KeyID: i.keyID, Algorithm: string(jose.RS256), Use: "sig", - }}}) + } + if len(i.certificate) == 0 { + writeJSON(w, http.StatusOK, jose.JSONWebKeySet{Keys: []jose.JSONWebKey{key}}) + return + } + // The chain is attached after go-jose has rendered the key, because + // go-jose cannot marshal a certificate it would refuse to parse — which is + // the entire point of this fixture. + rendered, err := key.MarshalJSON() + if err != nil { + http.Error(w, "fakeissuer: render key: "+err.Error(), http.StatusInternalServerError) + return + } + var members map[string]json.RawMessage + if err := json.Unmarshal(rendered, &members); err != nil { + http.Error(w, "fakeissuer: reread key: "+err.Error(), http.StatusInternalServerError) + return + } + chain, err := json.Marshal([]string{base64.StdEncoding.EncodeToString(i.certificate)}) + if err != nil { + http.Error(w, "fakeissuer: render chain: "+err.Error(), http.StatusInternalServerError) + return + } + members["x5c"] = chain + writeJSON(w, http.StatusOK, map[string]any{"keys": []any{members}}) +} + +// negativeSerialCertificate returns a self-signed certificate for key whose +// serial number is negative. +// +// It cannot be minted directly: crypto/x509.CreateCertificate refuses a +// negative serial ("serial number must be positive"), which is why the value +// is edited into the encoding afterwards. A serial whose leading value byte +// has its high bit set is encoded by Go with a 0x00 pad to keep it positive; +// removing that pad reinterprets the same four bytes as a negative +// two's-complement integer, which is precisely the encoding real deployments +// publish. Removing a byte shortens the two enclosing SEQUENCEs by one each. +func negativeSerialCertificate(t *testing.T, key *rsa.PrivateKey) []byte { + t.Helper() + const padded = "\x02\x05\x00\xc5\xb0\x7c\x97" // INTEGER, 5 bytes, positive + const negative = "\x02\x04\xc5\xb0\x7c\x97" // INTEGER, 4 bytes, negative + template := &x509.Certificate{ + SerialNumber: big.NewInt(0xC5B07C97), + Subject: pkix.Name{CommonName: "fakeissuer negative serial"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatalf("fakeissuer: create certificate: %v", err) + } + at := strings.Index(string(der), padded) + if at < 0 { + t.Fatal("fakeissuer: the serial number is not encoded where this fixture expects it") + } + edited := make([]byte, 0, len(der)-1) + edited = append(edited, der[:at]...) + edited = append(edited, negative...) + edited = append(edited, der[at+len(padded):]...) + // Both the Certificate and the TBSCertificate SEQUENCE use a long-form + // two-byte length, and each now describes one byte less. + for _, offset := range []int{2, 6} { + if edited[offset-2] != 0x30 || edited[offset-1] != 0x82 { + t.Fatalf("fakeissuer: unexpected DER header at offset %d", offset-2) + } + binary.BigEndian.PutUint16(edited[offset:], binary.BigEndian.Uint16(edited[offset:])-1) + } + if _, err := x509.ParseCertificate(edited); err == nil { + t.Fatal("fakeissuer: the certificate this fixture exists to make unparseable parses") + } + return edited } // handleAuthorize auto-approves: the "user" always consents, so a test's whole diff --git a/internal/auth/oauthflow/jwks.go b/internal/auth/oauthflow/jwks.go new file mode 100644 index 0000000..482d707 --- /dev/null +++ b/internal/auth/oauthflow/jwks.go @@ -0,0 +1,151 @@ +// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package oauthflow + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "strconv" +) + +// certificateFields are the members of a JSON Web Key that describe an X.509 +// certificate rather than the key itself. x5t and x5t#S256 are thumbprints of +// the chain in x5c, and go-jose checks them against it, so they leave together +// or not at all. +var certificateFields = []string{"x5c", "x5t", "x5t#S256"} + +// keyParameters are the members that fully define a public key, by key type. +// A key carrying its own parameters needs nothing from a certificate. +var keyParameters = map[string][]string{ + "RSA": {"n", "e"}, + "EC": {"crv", "x", "y"}, + "OKP": {"crv", "x"}, +} + +// withoutCertificates drops the certificate members from every self-describing +// key in a JSON Web Key Set, and reports whether it changed anything. +// +// This exists because go-jose parses x5c eagerly while unmarshalling a key set +// and fails the whole document when any certificate in it does not parse. Go +// has rejected certificates with negative serial numbers since 1.23, and WSO2 +// deployments have published them for years — so an issuer whose keys can +// verify a token perfectly well becomes an issuer with no readable keys at +// all, over a field the verification never needed. +// +// Only keys that already carry their own parameters are stripped. A key that +// somehow depended on its certificate keeps it and fails loudly, which is the +// right outcome: this removes a spurious failure, it does not paper over a key +// the shell genuinely cannot read. +func withoutCertificates(body []byte) ([]byte, bool) { + var document map[string]json.RawMessage + if err := json.Unmarshal(body, &document); err != nil { + return nil, false + } + rawKeys, present := document["keys"] + if !present { + return nil, false + } + var keys []map[string]json.RawMessage + if err := json.Unmarshal(rawKeys, &keys); err != nil { + return nil, false + } + stripped := false + for _, key := range keys { + if !selfDescribing(key) { + continue + } + for _, field := range certificateFields { + if _, carried := key[field]; carried { + delete(key, field) + stripped = true + } + } + } + if !stripped { + return nil, false + } + rewrittenKeys, err := json.Marshal(keys) + if err != nil { + return nil, false + } + document["keys"] = rewrittenKeys + rewritten, err := json.Marshal(document) + if err != nil { + return nil, false + } + return rewritten, true +} + +// selfDescribing reports whether a key carries every parameter its type needs, +// so that discarding its certificate loses nothing. +func selfDescribing(key map[string]json.RawMessage) bool { + var keyType string + if err := json.Unmarshal(key["kty"], &keyType); err != nil { + return false + } + required, known := keyParameters[keyType] + if !known { + return false + } + for _, parameter := range required { + if _, carried := key[parameter]; !carried { + return false + } + } + return true +} + +// certificateStripper removes certificate members from key sets on their way +// back from the issuer, before any library parses them. +// +// It sits at the transport because that is the one place every fetch the OIDC +// library makes passes through, including the key set it fetches lazily on +// first verification. Responses that are not key sets are returned exactly as +// they arrived, byte for byte. +type certificateStripper struct{ base http.RoundTripper } + +func (s certificateStripper) RoundTrip(request *http.Request) (*http.Response, error) { + base := s.base + if base == nil { + base = http.DefaultTransport + } + response, err := base.RoundTrip(request) + if err != nil || response.Body == nil || response.StatusCode != http.StatusOK { + return response, err + } + body, err := io.ReadAll(response.Body) + // Closing is what releases the connection, and a failure to do so says + // nothing about the bytes already read. Refusing the response over it + // would invent exactly the kind of spurious failure this file exists to + // remove. A read that did not finish is different: the body has been + // consumed and cannot be handed on, so there is no response left to + // return. + _ = response.Body.Close() + if err != nil { + return nil, err + } + stripped, changed := withoutCertificates(body) + if !changed { + stripped = body + } + response.Body = io.NopCloser(bytes.NewReader(stripped)) + response.ContentLength = int64(len(stripped)) + response.Header.Set("Content-Length", strconv.Itoa(len(stripped))) + return response, nil +} diff --git a/internal/auth/oauthflow/jwks_internal_test.go b/internal/auth/oauthflow/jwks_internal_test.go new file mode 100644 index 0000000..790d791 --- /dev/null +++ b/internal/auth/oauthflow/jwks_internal_test.go @@ -0,0 +1,148 @@ +// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package oauthflow + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// unclosableBody reads to completion and then fails to close, as a connection +// torn down between the last byte and the release can. +type unclosableBody struct{ io.Reader } + +func (unclosableBody) Close() error { return errors.New("connection reset by peer") } + +// answering is a transport that hands back one prepared response. +type answering struct{ response *http.Response } + +func (a answering) RoundTrip(*http.Request) (*http.Response, error) { return a.response, nil } + +// TestWithoutCertificatesTouchesOnlyKeySets proves the stripper is inert +// everywhere except the one document it exists for. Every fetch a login makes +// passes through it — discovery, the token exchange, the key set — so a +// rewrite it applied too eagerly would corrupt a response nobody asked it to +// read. +func TestWithoutCertificatesTouchesOnlyKeySets(t *testing.T) { + for name, body := range map[string]string{ + "a token response": `{"access_token":"a.b.c","expires_in":3600,"scope":"read"}`, + "a discovery document": `{"issuer":"https://example.test","jwks_uri":"https://example.test/jwks"}`, + "a key set without x5c": `{"keys":[{"kty":"RSA","n":"abc","e":"AQAB"}]}`, + "not JSON at all": `gateway timeout`, + "a JSON array": `[1,2,3]`, + } { + t.Run(name, func(t *testing.T) { + if _, changed := withoutCertificates([]byte(body)); changed { + t.Fatalf("the stripper rewrote %s:\n%s", name, body) + } + }) + } +} + +// TestWithoutCertificatesKeepsWhatTheKeyNeeds proves the stripper only +// discards a certificate when the key beside it is already complete. A key +// that genuinely depended on its certificate must keep it and fail loudly: +// this removes a spurious failure, it does not hide an unreadable key. +func TestWithoutCertificatesKeepsWhatTheKeyNeeds(t *testing.T) { + for name, body := range map[string]string{ + "an RSA key missing its exponent": `{"keys":[{"kty":"RSA","n":"abc","x5c":["MII"]}]}`, + "an EC key missing its curve": `{"keys":[{"kty":"EC","x":"a","y":"b","x5c":["MII"]}]}`, + "a key type nothing knows": `{"keys":[{"kty":"XYZ","x5c":["MII"]}]}`, + } { + t.Run(name, func(t *testing.T) { + if _, changed := withoutCertificates([]byte(body)); changed { + t.Fatalf("the stripper discarded a certificate the key may need:\n%s", body) + } + }) + } +} + +// TestWithoutCertificatesDropsTheChainAndItsThumbprints proves the whole +// certificate group leaves together. go-jose checks x5t and x5t#S256 against +// the chain in x5c, so a thumbprint left behind after the chain has gone is a +// check with nothing to check against. +func TestWithoutCertificatesDropsTheChainAndItsThumbprints(t *testing.T) { + body := `{"keys":[{"kty":"RSA","n":"abc","e":"AQAB","use":"sig","kid":"k1",` + + `"x5c":["MII"],"x5t":"t1","x5t#S256":"t256"}],"other":"preserved"}` + + rewritten, changed := withoutCertificates([]byte(body)) + if !changed { + t.Fatal("the stripper left a certificate on a key that did not need one") + } + for _, gone := range []string{"x5c", "x5t", "x5t#S256", "MII"} { + if strings.Contains(string(rewritten), gone) { + t.Fatalf("%q survived the strip:\n%s", gone, rewritten) + } + } + + var document struct { + Keys []map[string]json.RawMessage `json:"keys"` + Other string `json:"other"` + } + if err := json.Unmarshal(rewritten, &document); err != nil { + t.Fatalf("the stripper produced something that is not a key set: %v", err) + } + if document.Other != "preserved" { + t.Fatalf("a member outside keys was lost: %s", rewritten) + } + if len(document.Keys) != 1 { + t.Fatalf("the key set lost its key: %s", rewritten) + } + for _, needed := range []string{"kty", "n", "e", "use", "kid"} { + if _, carried := document.Keys[0][needed]; !carried { + t.Fatalf("the strip took %q with it: %s", needed, rewritten) + } + } +} + +// TestRoundTripKeepsAResponseWhoseBodyWillNotClose proves a key set that +// arrived intact is not thrown away because releasing the connection failed +// afterwards. +// +// Closing a response body is what returns the connection to the pool; it says +// nothing about bytes already read. Refusing the response over it would invent +// exactly the kind of spurious failure this file exists to remove — and it +// would do so on every fetch a login makes, not only on key sets. +func TestRoundTripKeepsAResponseWhoseBodyWillNotClose(t *testing.T) { + const keySet = `{"keys":[{"kty":"RSA","n":"abc","e":"AQAB","x5c":["MII"]}]}` + stripper := certificateStripper{base: answering{response: &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{}, + Body: unclosableBody{strings.NewReader(keySet)}, + }}} + + response, err := stripper.RoundTrip( + httptest.NewRequest(http.MethodGet, "https://example.test/jwks", nil)) + if err != nil { + t.Fatalf("a key set that arrived intact was refused because its body would not close: %v", err) + } + delivered, err := io.ReadAll(response.Body) + if err != nil { + t.Fatalf("reading the delivered body: %v", err) + } + if strings.Contains(string(delivered), "x5c") { + t.Fatalf("the certificate survived the strip:\n%s", delivered) + } + if !strings.Contains(string(delivered), `"n":"abc"`) { + t.Fatalf("the key did not survive the strip:\n%s", delivered) + } +} diff --git a/internal/auth/oauthflow/login.go b/internal/auth/oauthflow/login.go index 9999134..f92098a 100644 --- a/internal/auth/oauthflow/login.go +++ b/internal/auth/oauthflow/login.go @@ -32,6 +32,7 @@ import ( "crypto/rand" "crypto/subtle" "encoding/base64" + "errors" "fmt" "io" "net" @@ -192,8 +193,7 @@ func (l Login) Run(ctx context.Context) (Result, error) { } idToken, err := provider.Verifier(&oidc.Config{ClientID: l.ClientID}).Verify(ctx, rawIDToken) if err != nil { - return Result{}, notCompleted("the identity token this login returned did not verify", - "Retry wso2 login. The shell does not accept an identity it cannot verify against the issuer's keys.") + return Result{}, identityNotVerified(err) } // The nonce proves the identity token was minted for this login and not // replayed from another one. @@ -227,11 +227,17 @@ func (l Login) ports() []int { return LoopbackPorts() } +// httpClient is the client every fetch this login makes goes through, wrapped +// so that a key set is read for its keys and not for the certificates beside +// them. See certificateStripper. func (l Login) httpClient() *http.Client { + base := http.DefaultClient if l.HTTPClient != nil { - return l.HTTPClient + base = l.HTTPClient } - return http.DefaultClient + stripped := *base + stripped.Transport = certificateStripper{base: base.Transport} + return &stripped } func (l Login) out() io.Writer { @@ -301,6 +307,46 @@ func notCompleted(message, recovery string) problem.Problem { return problem.New(problem.CategoryAuthPolicy, "auth.credential_unavailable", message).WithRecovery(recovery) } +// identityNotVerified reports an identity token the shell would not accept, +// and says which kind of failure it was. +// +// The code stays the same for all of them, because the caller is left in one +// place. The message does not, because the reader is not: a token the issuer's +// keys did not sign, a token minted for a different application, and a key set +// the shell could not read are three different things to go and fix, and only +// one of them is helped by trying again. Retrying is the default advice +// precisely because it is the honest one when the cause is unknown, and it is +// the wrong advice for a cause the shell can name. +// +// The library states these failures in prose rather than in typed errors, so +// the classification reads its words. A wording change upstream costs the +// specific message and falls back to the general one; it cannot cost the +// refusal itself. +func identityNotVerified(err error) problem.Problem { + var expired *oidc.TokenExpiredError + switch reason := err.Error(); { + case errors.As(err, &expired): + return notCompleted("the identity token this login returned had already expired", + "Check that this machine's clock is correct, then retry wso2 login.") + case strings.Contains(reason, "fetching keys"): + return notCompleted("the shell could not read the signing keys the identity provider publishes", + "Confirm this machine can reach the issuer's JWKS endpoint. If it is reachable, the "+ + "deployment is publishing a key set this shell cannot parse; report it with the "+ + "issuer URL.") + case strings.Contains(reason, "expected audience"): + return notCompleted("the identity token this login returned was issued for a different application", + "Confirm the client identifier in the selected context names the OAuth application this "+ + "issuer signed you in to.") + case strings.Contains(reason, "failed to verify signature"): + return notCompleted("the identity token this login returned was not signed by the identity provider's keys", + "Retry wso2 login. If it keeps failing, confirm the issuer in the selected context is the "+ + "deployment that signed you in.") + default: + return notCompleted("the identity token this login returned did not verify", + "Retry wso2 login. The shell does not accept an identity it cannot verify against the issuer's keys.") + } +} + // callback is the loopback listener one login waits on. type callback struct { server *http.Server diff --git a/internal/auth/oauthflow/login_test.go b/internal/auth/oauthflow/login_test.go index 2d0b0fd..a2b5e0c 100644 --- a/internal/auth/oauthflow/login_test.go +++ b/internal/auth/oauthflow/login_test.go @@ -170,6 +170,40 @@ func TestBrowserLoginRoundTrip(t *testing.T) { } } +// TestLoginVerifiesThroughACertificateItCannotParse proves a key set is read +// for the keys in it and not for the certificates published beside them. +// +// WSO2 deployments — Asgardeo tenants and Identity Servers alike — publish +// signing certificates whose serial numbers are negative, which RFC 5280 +// forbids and which Go's x509 parser has rejected since 1.23. go-jose parses +// x5c eagerly while unmarshalling a key set and fails the whole document when +// one certificate in it does not parse, so such a deployment leaves the shell +// with no readable keys and no login is possible at all. The signing key was +// never the problem: n and e describe it completely. +func TestLoginVerifiesThroughACertificateItCannotParse(t *testing.T) { + issuer := fakeissuer.New(t, fakeissuer.Options{ + Audience: "reference-status", + AllowAnyLoopbackPort: true, + NegativeSerialCertificate: true, + }) + printed := &recorder{} + login := browserLogin(issuer, printed, func(authURL string) error { + go visit(issuer, authURL) + return nil + }) + + result, err := login.Run(testContext(t, 30*time.Second)) + if err != nil { + t.Fatalf("login refused an issuer whose signing keys are perfectly readable: %v", err) + } + if result.Token == nil || result.Token.RefreshToken == "" { + t.Fatal("no refresh token issued") + } + if result.Subject != "user-1" { + t.Fatalf("identity subject %q, want user-1", result.Subject) + } +} + func TestLoginCompletesFromThePrintedURLWhenTheBrowserCannotOpen(t *testing.T) { issuer := fakeissuer.New(t, fakeissuer.Options{Audience: "reference-status", AllowAnyLoopbackPort: true}) printed := &recorder{} diff --git a/test/smoke/RUNNING.md b/test/smoke/RUNNING.md index 3e6b869..b7566fe 100644 --- a/test/smoke/RUNNING.md +++ b/test/smoke/RUNNING.md @@ -141,12 +141,41 @@ only verdicts whose deployment line names the deployment you mean to record. above the verdict prints both permission sets; copy them into the research document with the verdict. - `rejected` — the token endpoint answered `invalid_scope`. + **Corroborate this one before recording it.** `invalid_scope` is also exactly + what the token endpoint answers when the application's API resource + authorization carries an authorization policy (RBAC) that the signing-in user + does not satisfy — a registration gap, not a protocol finding about the + deployment. The first live Asgardeo run hit it twice before producing a real + verdict, and from the verdict line alone it is indistinguishable from a + genuine "this deployment refuses to narrow" result. Before recording `rejected`, go to the application's + Authorization tab and confirm the resource's policy reads `No Authorization + Policy`, or, if it reads `Role Based Access Control (RBAC)`, that the + signing-in user holds a role granting every scope the resource lists. Only + once that is confirmed does `invalid_scope` say something about the + deployment rather than about who was signed in when the experiment ran. + Recording it without checking puts a false claim about Asgardeo into a + research document whose whole purpose is being trustworthy about exactly + that. - `inconclusive (opaque access token)` — the deployment issues opaque access tokens, so nothing can be proven about what they carry. Configure the application to issue JWT access tokens and run it again; until then this question has no answer on this deployment. - `inconclusive (audience not bound)` — a token came back that is not bound to - the configured audience. Fix the API resource registration first. + the configured audience. On Asgardeo this is not a registration defect to + fix: Asgardeo binds a JWT access token's `aud` claim to the **client ID**, + never to the API resource identifier whose scopes the token carries, and this + is not configurable — the application's Protocol tab exposes an Audience + field only under **ID Token**, and the Access Token section has no audience + control at all. See section 2.5 of + [the walkthrough](../../docs/guides/login.md), and + [the research document](../../docs/research/asgardeo-redirect-uri-and-scope-narrowing.md) + this file already links above. The remedy is to set `WSO2_SMOKE_AUDIENCE` + here, and `products..audience` in a real context document, to the + **client ID** — that is the only value Asgardeo ever puts in `aud`. On a + deployment that does bind tokens to API resources, the resource identifier is + the correct value to configure, and an unbound token there means the resource + is not authorized on the application instead. Whether Identity Server 7.x + behaves like Asgardeo here is not yet measured. ### Recording the verdicts diff --git a/test/smoke/login_smoke_test.go b/test/smoke/login_smoke_test.go index 30a0764..16a634d 100644 --- a/test/smoke/login_smoke_test.go +++ b/test/smoke/login_smoke_test.go @@ -119,9 +119,17 @@ func TestLoginSmoke(t *testing.T) { case refusalCode(err) == codeNarrowingUnavailable: // Documented, correct behavior. See this test's own doc comment and // docs/guides/login.md's troubleshooting section. - t.Logf("LOGIN SMOKE: refused %s — the deployment would not prove a narrowed grant. "+ - "Login and session persistence passed; this refusal is the designed outcome, "+ - "not a failure.\n %v", codeNarrowingUnavailable, err) + // + // auth.narrowing_unavailable covers five distinct causes — see + // internal/auth/narrowing.go's verify() and the table under + // auth.narrowing_unavailable in docs/guides/login.md section 8 — so this + // summary must not name one of them (a "narrowed grant" specifically). + // The interpolated error text below is what actually says which of the + // five happened; this sentence only states what is true regardless: the + // shell declined to hand the module more authority than it asked for. + t.Logf("LOGIN SMOKE: refused %s — the shell would not hand the module a grant it could not "+ + "prove was exactly what it asked for. Login and session persistence passed; this refusal "+ + "is the designed outcome, not a failure.\n %v", codeNarrowingUnavailable, err) default: t.Fatalf("the broker refused for a reason this slice does not accept: %v", err) }