From 6587b052816e6828432944ba470def8795b7f235 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 01:44:41 +0530 Subject: [PATCH 1/4] fix: read a key set for its keys, not the certificates beside them 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 --- internal/auth/fakeissuer/fakeissuer.go | 93 ++++++++++- internal/auth/oauthflow/jwks.go | 148 ++++++++++++++++++ internal/auth/oauthflow/jwks_internal_test.go | 100 ++++++++++++ internal/auth/oauthflow/login.go | 54 ++++++- internal/auth/oauthflow/login_test.go | 34 ++++ 5 files changed, 423 insertions(+), 6 deletions(-) create mode 100644 internal/auth/oauthflow/jwks.go create mode 100644 internal/auth/oauthflow/jwks_internal_test.go 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..e7d133e --- /dev/null +++ b/internal/auth/oauthflow/jwks.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 ( + "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) + closeErr := response.Body.Close() + if err != nil { + return nil, err + } + if closeErr != nil { + return nil, closeErr + } + 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..cb480a3 --- /dev/null +++ b/internal/auth/oauthflow/jwks_internal_test.go @@ -0,0 +1,100 @@ +// 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" + "strings" + "testing" +) + +// 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) + } + } +} 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{} From fe8f5734c16563ab68c9570d1ddb24aa61fe7a8d Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 01:44:52 +0530 Subject: [PATCH 2/4] test: correct what the smoke run and its guidance claim 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 --- test/smoke/RUNNING.md | 31 ++++++++++++++++++++++++++++++- test/smoke/login_smoke_test.go | 14 +++++++++++--- 2 files changed, 41 insertions(+), 4 deletions(-) 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) } From de7367f10bb6f48987454219b894f4806bc4d474 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 01:45:04 +0530 Subject: [PATCH 3/4] docs: fold the live asgardeo run back into the walkthrough 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..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 --- docs/guides/login.md | 265 ++++++++++++++++-- ...gardeo-redirect-uri-and-scope-narrowing.md | 40 ++- 2 files changed, 275 insertions(+), 30 deletions(-) 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 From 697be207502798ea36ea70e7256622f2a2b63a1a Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 02:10:24 +0530 Subject: [PATCH 4/4] fix: keep a key set that arrived intact but would not close 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 --- internal/auth/oauthflow/jwks.go | 11 +++-- internal/auth/oauthflow/jwks_internal_test.go | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/internal/auth/oauthflow/jwks.go b/internal/auth/oauthflow/jwks.go index e7d133e..482d707 100644 --- a/internal/auth/oauthflow/jwks.go +++ b/internal/auth/oauthflow/jwks.go @@ -130,13 +130,16 @@ func (s certificateStripper) RoundTrip(request *http.Request) (*http.Response, e return response, err } body, err := io.ReadAll(response.Body) - closeErr := response.Body.Close() + // 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 } - if closeErr != nil { - return nil, closeErr - } stripped, changed := withoutCertificates(body) if !changed { stripped = body diff --git a/internal/auth/oauthflow/jwks_internal_test.go b/internal/auth/oauthflow/jwks_internal_test.go index cb480a3..790d791 100644 --- a/internal/auth/oauthflow/jwks_internal_test.go +++ b/internal/auth/oauthflow/jwks_internal_test.go @@ -18,10 +18,25 @@ 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 @@ -98,3 +113,36 @@ func TestWithoutCertificatesDropsTheChainAndItsThumbprints(t *testing.T) { } } } + +// 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) + } +}