From 8bd0ed86f1e92b903844d949a3d3d2fec08017f7 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 19:48:26 +0530 Subject: [PATCH 1/8] feat(contexts): say what a deployment is and how access derives from it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asgardeo and Identity Server decide an access token's audience from the application's registration, so one login session serves every product an identity declares. ThunderID decides it per request, from an RFC 8707 resource indicator it requires at authorization and accepts only one of. Measured at v1.0.0-beta: without one the authorization request is refused outright. An identity may now name the provider behind its issuer, and may name the derivation explicitly when a deployment does not match what its provider ordinarily requires. Both are optional, both are names of behaviour rather than locations of secrets, and an identity that declares neither derives exactly as every identity did before — so every document that works today keeps working and no migration is needed. Naming the provider is what the person writing the document knows. What that product requires of a token request is what the shell knows, and the pair exists so the second does not have to be written by hand. An explicit derivation wins over the one a provider implies, because a Thunder deployment whose resource server is not registered yet is a real state and the document has to be able to say so. Two refusals are decided when the document is read rather than at the end of a browser sign-in the user cannot act on: an identity that derives by resource and declares more than one product cannot be served by one session at all, and a product that names no audience leaves nothing to bind access to. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- internal/contexts/contexts_test.go | 7 +- internal/contexts/derivation_test.go | 142 +++++++++++++++++++++++++++ internal/contexts/identity.go | 117 ++++++++++++++++++++++ 3 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 internal/contexts/derivation_test.go diff --git a/internal/contexts/contexts_test.go b/internal/contexts/contexts_test.go index 0f8edf0..9711b79 100644 --- a/internal/contexts/contexts_test.go +++ b/internal/contexts/contexts_test.go @@ -214,7 +214,12 @@ func TestAContextRecordsNoCredentialValue(t *testing.T) { // types rather than from every writer of them. allowedContext := []string{"name", "identity", "organization", "project"} allowedIdentity := []string{"name", "type", "auth", "products"} - allowedAuth := []string{"kind", "issuer", "clientId", "tenant", "credentialRef", "clientSecretVariable"} + // provider and narrowing say which deployment this is and how access is + // derived from it. Both are names of behaviour, not locations of secrets. + allowedAuth := []string{ + "kind", "issuer", "clientId", "tenant", "credentialRef", "clientSecretVariable", + "provider", "narrowing", + } allowedProduct := []string{"endpoint", "audience", "scopes"} if got := jsonMembers(t, contexts.Context{}); !slices.Equal(got, allowedContext) { diff --git a/internal/contexts/derivation_test.go b/internal/contexts/derivation_test.go new file mode 100644 index 0000000..539c8f1 --- /dev/null +++ b/internal/contexts/derivation_test.go @@ -0,0 +1,142 @@ +// 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 contexts_test + +import ( + "strings" + "testing" + + "github.com/wso2/wso2-cli/internal/contexts" +) + +// A document written before this shell knew any deployment derived access +// differently must keep working, and must keep deriving the way it always has. +func TestAnIdentityDeclaringNothingDerivesByScopedRefresh(t *testing.T) { + document, err := contexts.Decode([]byte(validV2())) + if err != nil { + t.Fatalf("decode: %v", err) + } + if got := document.Identities[0].Auth.Derivation(); got != contexts.DerivationScopedRefresh { + t.Fatalf("an identity declaring no derivation derived by %q, want %q", + got, contexts.DerivationScopedRefresh) + } +} + +// Naming the identity provider is what a person writing the document knows. +// The derivation it implies is what the shell knows, and it should not have to +// be written twice. +func TestNamingThunderImpliesResourceBoundDerivation(t *testing.T) { + document, err := contexts.Decode([]byte(withProvider(contexts.ProviderThunder))) + if err != nil { + t.Fatalf("decode: %v", err) + } + if got := document.Identities[0].Auth.Derivation(); got != contexts.DerivationTokenResource { + t.Fatalf("a Thunder identity derived by %q, want %q", + got, contexts.DerivationTokenResource) + } +} + +// Thunder is pre-1.0, and a deployment whose resource server is not registered +// is a state the first several of them will be in. Saying so explicitly has to +// win, or the provider declaration becomes a straitjacket nobody can leave. +func TestAnExplicitDerivationOverridesWhatTheProviderImplies(t *testing.T) { + document, err := contexts.Decode([]byte(withDerivation( + contexts.ProviderThunder, contexts.DerivationScopedRefresh))) + if err != nil { + t.Fatalf("decode: %v", err) + } + if got := document.Identities[0].Auth.Derivation(); got != contexts.DerivationScopedRefresh { + t.Fatalf("an explicit derivation was overridden: got %q, want %q", + got, contexts.DerivationScopedRefresh) + } +} + +// Asgardeo and Identity Server take no audience at authorization time, so one +// session serves every product there. Naming either of them must not change how +// access is derived. +func TestNamingAsgardeoOrIdentityServerLeavesTheDerivationAlone(t *testing.T) { + for _, provider := range []string{contexts.ProviderAsgardeo, contexts.ProviderIdentityServer} { + t.Run(provider, func(t *testing.T) { + document, err := contexts.Decode([]byte(withProvider(provider))) + if err != nil { + t.Fatalf("decode: %v", err) + } + if got := document.Identities[0].Auth.Derivation(); got != contexts.DerivationScopedRefresh { + t.Fatalf("provider %q derived by %q, want %q", + provider, got, contexts.DerivationScopedRefresh) + } + }) + } +} + +// An identity provider this shell has never heard of is a document it cannot +// act on, and guessing a derivation for it would be the shell inventing policy. +func TestAnUnknownProviderIsRefused(t *testing.T) { + _, err := contexts.Decode([]byte(withProvider("acme-idp"))) + assertProblemCode(t, err, "contexts.document_malformed") +} + +// A derivation this shell does not implement is refused for the same reason. +func TestAnUnknownDerivationIsRefused(t *testing.T) { + _, err := contexts.Decode([]byte(withDerivation(contexts.ProviderThunder, "token-exchange"))) + assertProblemCode(t, err, "contexts.document_malformed") +} + +// Thunder requires a resource indicator at authorization time and accepts only +// one, so one session reaches exactly one product. An identity that declares +// two is refused when the document is read, rather than at the end of a browser +// sign-in the user cannot act on. +func TestAThunderIdentityServingSeveralProductsIsRefused(t *testing.T) { + _, err := contexts.Decode([]byte(withSecondProduct(withProvider(contexts.ProviderThunder)))) + assertProblemCode(t, err, "contexts.document_malformed") +} + +// The same document is legal on a deployment that binds no audience at +// authorization time, so the refusal above must be about Thunder and not about +// serving two products. +func TestSeveralProductsStayLegalWithoutThunder(t *testing.T) { + if _, err := contexts.Decode([]byte(withSecondProduct(validV2()))); err != nil { + t.Fatalf("two products on a non-Thunder identity should validate: %v", err) + } +} + +// Deriving access bound to a resource means naming the resource, and a product +// with no audience names none. +func TestAThunderIdentityWhoseProductNamesNoAudienceIsRefused(t *testing.T) { + document := withProvider(contexts.ProviderThunder) + document = strings.Replace(document, `"audience": "reference-status",`, ``, 1) + _, err := contexts.Decode([]byte(document)) + assertProblemCode(t, err, "contexts.document_malformed") +} + +func withProvider(provider string) string { + return strings.Replace(validV2(), + `"kind": "oauth-browser",`, + `"kind": "oauth-browser", "provider": "`+provider+`",`, 1) +} + +func withDerivation(provider, derivation string) string { + return strings.Replace(withProvider(provider), + `"kind": "oauth-browser",`, + `"kind": "oauth-browser", "narrowing": "`+derivation+`",`, 1) +} + +func withSecondProduct(document string) string { + return strings.Replace(document, + `"reference": {`, + `"second": {"endpoint": "https://api.example.test", "audience": "second-api", "scopes": ["second:read"]}, "reference": {`, 1) +} diff --git a/internal/contexts/identity.go b/internal/contexts/identity.go index 21e9f29..09c8d11 100644 --- a/internal/contexts/identity.go +++ b/internal/contexts/identity.go @@ -47,6 +47,52 @@ var legalKinds = map[string]bool{ KindClientCredentials: true, KindPAT: true, } +// The identity providers a document may name. +// +// Naming one is how a document says what it points at, which is what the person +// writing it knows. What that product requires of a token request is what this +// shell knows, and the list exists so the two do not have to be written twice. +// Omitting the member entirely is the open-world case: any conforming OpenID +// provider stays describable, and derives the way every deployment did before +// this member existed. +const ( + // ProviderAsgardeo is WSO2's identity cloud. + ProviderAsgardeo = "asgardeo" + // ProviderIdentityServer is a self-hosted WSO2 Identity Server. + ProviderIdentityServer = "identity-server" + // ProviderThunder is a ThunderID deployment. + ProviderThunder = "thunder" +) + +// The derivations a document may declare. +const ( + // DerivationScopedRefresh narrows the login session by asking the refresh + // grant for the module's own permissions. It is what every deployment this + // shell served before resource indicators existed, and the default. + DerivationScopedRefresh = "scoped-refresh" + // DerivationTokenResource binds each request to one protected resource with + // an RFC 8707 resource indicator, and narrows permissions alongside it. + DerivationTokenResource = "token-resource" +) + +// providerDerivation is the derivation each named product requires. +// +// Asgardeo and Identity Server take no audience at authorization time, so one +// session serves every product and the scoped refresh answers for both. Thunder +// requires a resource indicator on the authorization request and accepts only +// one, so its sessions are bound to a single protected resource from the moment +// they are established. +var providerDerivation = map[string]string{ + ProviderAsgardeo: DerivationScopedRefresh, + ProviderIdentityServer: DerivationScopedRefresh, + ProviderThunder: DerivationTokenResource, +} + +// legalDerivations are the derivations this shell implements. +var legalDerivations = map[string]bool{ + DerivationScopedRefresh: true, DerivationTokenResource: true, +} + // refPattern constrains a credential reference to one readable word, exactly // as context names are constrained. A credential value pasted where a // reference belongs — a JWT, anything with dots, equals signs, or upper-case @@ -88,10 +134,39 @@ type IdentityAuth struct { // ClientSecretVariable names the environment variable holding the client // secret for the client-credentials kind. It is a name, never a value. ClientSecretVariable string `json:"clientSecretVariable,omitempty"` + // Provider names the identity provider behind the issuer. It is optional, + // and it implies a derivation rather than being one. + Provider string `json:"provider,omitempty"` + // Narrowing names the derivation explicitly, for a deployment that does not + // match what its provider ordinarily requires. It is optional and wins over + // what Provider implies. + Narrowing string `json:"narrowing,omitempty"` // CredentialVariable exists only on synthetic v1 identities. Never encoded. CredentialVariable string `json:"-"` } +// Derivation is how access for one module is derived under this identity. +// +// It is decided in one place because everything downstream — the login that +// establishes a session, and the grant that narrows it — has to agree, and a +// disagreement between them is a token bound to the wrong thing rather than a +// failure anyone can read. +// +// The order is a default and an override, not two assertions that could +// contradict each other: a provider states what its product ordinarily +// requires, and an explicit derivation states what this deployment actually +// does. Saying both is legal, because a deployment that has not registered a +// resource server is a real state and the document has to be able to say so. +func (a IdentityAuth) Derivation() string { + if a.Narrowing != "" { + return a.Narrowing + } + if derivation, named := providerDerivation[a.Provider]; named { + return derivation + } + return DerivationScopedRefresh +} + // Product is one product service reachable under an identity. type Product struct { // Endpoint is the product service's base URL. @@ -116,6 +191,9 @@ func (i Identity) validate() error { if err := i.Auth.validate(i.Name); err != nil { return err } + if err := i.validateDerivation(); err != nil { + return err + } // The namespaces are walked in sorted order so a document with more than // one unreadable product is refused for the same reason on every run. for _, namespace := range slices.Sorted(maps.Keys(i.Products)) { @@ -129,7 +207,46 @@ func (i Identity) validate() error { return nil } +// validateDerivation refuses a document whose derivation cannot be carried out +// as written. +// +// A resource-bound derivation names the protected resource it binds to, and +// takes that name from the product the module asks for. Two consequences +// follow, and both are refused here rather than at the end of a browser +// sign-in: a product that names no audience leaves nothing to bind to, and an +// identity serving several products cannot be served by one session at all, +// because the deployments that require a resource indicator accept only one per +// authorization. +func (i Identity) validateDerivation() error { + if i.Auth.Derivation() != DerivationTokenResource { + return nil + } + if len(i.Products) > 1 { + return malformed(fmt.Sprintf( + "declares the identity %q against a deployment that binds one login to one product, "+ + "and gives it %d products", i.Name, len(i.Products))) + } + for _, namespace := range slices.Sorted(maps.Keys(i.Products)) { + if i.Products[namespace].Audience == "" { + return malformed(fmt.Sprintf( + "declares the %q product on the identity %q without the audience its deployment "+ + "binds access to", namespace, i.Name)) + } + } + return nil +} + func (a IdentityAuth) validate(identity string) error { + if a.Provider != "" { + if _, known := providerDerivation[a.Provider]; !known { + return malformed(fmt.Sprintf( + "declares an identity provider for %q that this shell does not read", identity)) + } + } + if a.Narrowing != "" && !legalDerivations[a.Narrowing] { + return malformed(fmt.Sprintf( + "declares a derivation for the identity %q that this shell does not implement", identity)) + } if !legalKinds[a.Kind] { return malformed(fmt.Sprintf("declares an authentication kind for the identity %q that this shell does not read", identity)) } From da4e56325f48531fe56e596699be5edf415f4fe5 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 19:48:41 +0530 Subject: [PATCH 2/8] feat(auth): bind access to a resource where the deployment requires one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deployment that decides the audience per request will not issue access at all without being told which protected resource it is for. ThunderID at v1.0.0-beta answers invalid_target — on the authorization request and on the client-credentials grant alike. The indicator is sent where the identity says the deployment reads one, and nowhere else, so no deployment already working starts receiving a parameter it never agreed to interpret. It belongs to the login rather than to the grant that follows because these deployments decide the audience once, at authorization: a session established without it cannot be bound afterwards, and one established with it reaches that resource and no other. The refresh grant needs nothing — the binding is inherited, measured. The client-credentials grant has no earlier authorization to inherit from, so it carries the indicator itself, taking it from the module's own request. invalid_target is read as its own refusal rather than falling through to the answer for a deployment nobody can classify. It is the one failure here caused by the context document rather than by the deployment: the identity did not say what kind of deployment this is, and the fix is a line in a document rather than a change to a registration. The stable code list does not grow — it stays auth.narrowing_unavailable, which is where a caller is left — but the guidance says the thing the reader can act on. The fake issuer grows the deployment behaviour rather than the tests growing a second fake: a request carrying an indicator is bound to it, and one that omits it against a deployment requiring one is refused exactly as the measured deployment refuses it. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- internal/app/login.go | 21 +++++ internal/app/login_resource_test.go | 98 ++++++++++++++++++++++ internal/auth/fakeissuer/fakeissuer.go | 53 ++++++++++-- internal/auth/narrowing.go | 11 +++ internal/auth/oauthflow/login.go | 29 ++++++- internal/auth/oauthflow/resource_test.go | 101 +++++++++++++++++++++++ internal/auth/resource_test.go | 60 ++++++++++++++ internal/auth/source_clientcred.go | 18 +++- internal/auth/tokenrequest.go | 11 +++ 9 files changed, 392 insertions(+), 10 deletions(-) create mode 100644 internal/app/login_resource_test.go create mode 100644 internal/auth/oauthflow/resource_test.go create mode 100644 internal/auth/resource_test.go diff --git a/internal/app/login.go b/internal/app/login.go index 49c2441..1d07fb1 100644 --- a/internal/app/login.go +++ b/internal/app/login.go @@ -111,6 +111,7 @@ func (s Shell) login(args []string) error { Issuer: selected.Identity.Auth.Issuer, ClientID: selected.Identity.Auth.ClientID, Scopes: productScopeUnion(selected.Identity), + Resource: productResource(selected.Identity), OpenBrowser: s.OpenBrowser, // The authorization URL is an instruction to act on, not this // command's result, so it goes to the diagnostic stream: a user who @@ -198,6 +199,26 @@ func productScopeUnion(identity contexts.Identity) []string { return union } +// productResource is the protected resource this login binds its session to, +// and is empty for a deployment that decides the audience from the +// application's registration instead. +// +// It reads the identity's only product, which is all there can be: a deployment +// that takes a resource indicator accepts one per authorization, so the context +// schema refuses an identity that derives this way and serves more than one +// product. The comment on productScopeUnion says a per-product login would mean +// one browser login per product; on these deployments that is not a choice the +// shell is making, it is what the deployment allows. +func productResource(identity contexts.Identity) string { + if identity.Auth.Derivation() != contexts.DerivationTokenResource { + return "" + } + for _, namespace := range slices.Sorted(maps.Keys(identity.Products)) { + return identity.Products[namespace].Audience + } + return "" +} + // parseLoginArgs reads the flags wso2 login owns and refuses everything else. func parseLoginArgs(args []string) (loginFlags, error) { var flags loginFlags diff --git a/internal/app/login_resource_test.go b/internal/app/login_resource_test.go new file mode 100644 index 0000000..20f0820 --- /dev/null +++ b/internal/app/login_resource_test.go @@ -0,0 +1,98 @@ +// 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 app_test + +import ( + "net/http" + "net/url" + "strings" + "testing" + + "github.com/wso2/wso2-cli/internal/auth/fakeissuer" + "github.com/wso2/wso2-cli/internal/auth/session" + "github.com/wso2/wso2-cli/internal/contexts" + "github.com/wso2/wso2-cli/internal/exit" + "github.com/zalando/go-keyring" +) + +// thunderDoc is browserDoc against a deployment that decides the audience at +// authorization time. +func thunderDoc(issuerURL string) contexts.Document { + document := browserDoc(issuerURL) + document.Identities[0].Auth.Provider = contexts.ProviderThunder + return document +} + +// A deployment that requires a resource indicator refuses a login that carries +// none, so the login has to take the one its product names. Without this the +// shell cannot log in against such a deployment at all. +func TestLoginBindsTheSessionToTheResourceTheProductNames(t *testing.T) { + keyring.MockInit() + issuer := fakeissuer.New(t, fakeissuer.Options{RequireResource: true}) + shell, _, errOut := newLoginShell(t) + installLogin(t, shell, thunderDoc(issuer.URL)) + shell.OpenBrowser = func(authURL string) error { + go func() { + response, err := http.Get(authURL) + if err == nil { + _ = response.Body.Close() + } + }() + return nil + } + + if code := shell.Run([]string{"login"}); code != exit.OK { + t.Fatalf("login failed: exit %d, stderr %s", code, errOut) + } + + stored, err := session.Store{StateRoot: shell.StateRoot}.Load(credentialRef) + if err != nil { + t.Fatalf("session not stored: %v", err) + } + if stored.RefreshToken == "" { + t.Fatal("the stored session holds no refresh token") + } + if !strings.Contains(errOut.String(), "resource="+url.QueryEscape("reference-status")) { + t.Fatalf("the authorization URL carried no resource indicator:\n%s", errOut) + } +} + +// An identity that names no such deployment must keep asking exactly as it did +// before, or every deployment already working would start receiving an +// indicator it never agreed to interpret. +func TestLoginSendsNoResourceIndicatorForAnOrdinaryDeployment(t *testing.T) { + keyring.MockInit() + issuer := fakeissuer.New(t, fakeissuer.Options{Audience: "reference-status"}) + shell, _, errOut := newLoginShell(t) + installLogin(t, shell, browserDoc(issuer.URL)) + shell.OpenBrowser = func(authURL string) error { + go func() { + response, err := http.Get(authURL) + if err == nil { + _ = response.Body.Close() + } + }() + return nil + } + + if code := shell.Run([]string{"login"}); code != exit.OK { + t.Fatalf("login failed: exit %d, stderr %s", code, errOut) + } + if strings.Contains(errOut.String(), "resource=") { + t.Fatalf("an ordinary login sent a resource indicator:\n%s", errOut) + } +} diff --git a/internal/auth/fakeissuer/fakeissuer.go b/internal/auth/fakeissuer/fakeissuer.go index d7c25bf..7167010 100644 --- a/internal/auth/fakeissuer/fakeissuer.go +++ b/internal/auth/fakeissuer/fakeissuer.go @@ -73,8 +73,19 @@ type Options struct { // secret, so only a test whose subject is a wrong credential has to state // one. ClientSecret string - // Audience is the aud claim minted into access tokens. + // Audience is the aud claim minted into access tokens. A request carrying a + // resource indicator overrides it, exactly as a deployment that binds tokens + // to a named resource server does. Audience string + // RequireResource refuses any authorization request that carries no RFC 8707 + // resource indicator, and mints the audience from the one it was given. + // + // It models a deployment that decides the audience at authorization time + // rather than from the application's registration — ThunderID, measured at + // v1.0.0-beta, which answers invalid_target without one. The consequence + // worth modeling is not the refusal but what follows from it: a session + // established this way reaches exactly one protected resource. + RequireResource bool // RotateRefreshTokens issues a new refresh token on every refresh, // invalidating the one presented. RotateRefreshTokens bool @@ -129,6 +140,7 @@ type codeGrant struct { redirectURI string nonce string clientID string + resource string } type tokenRecord struct { @@ -316,6 +328,16 @@ func (i *Issuer) handleAuthorize(w http.ResponseWriter, r *http.Request) { http.Error(w, "redirect_uri is not a registered loopback callback", http.StatusBadRequest) case query.Get("code_challenge") == "" || query.Get("code_challenge_method") != "S256": http.Error(w, "an S256 code challenge is required", http.StatusBadRequest) + case i.opts.RequireResource && query.Get("resource") == "": + // The refusal is redirected rather than served, because that is what a + // deployment requiring a resource indicator does: the browser comes back + // to the callback carrying an error, and the login reads it there. + refusal := url.Values{ + "error": {"invalid_target"}, + "error_description": {"No resource parameter supplied and no default resource server is configured"}, + "state": {query.Get("state")}, + } + http.Redirect(w, r, redirectURI+"?"+refusal.Encode(), http.StatusFound) default: code := randomToken("code") i.mutex.Lock() @@ -325,6 +347,7 @@ func (i *Issuer) handleAuthorize(w http.ResponseWriter, r *http.Request) { redirectURI: redirectURI, nonce: query.Get("nonce"), clientID: query.Get("client_id"), + resource: query.Get("resource"), } i.mutex.Unlock() callback := url.Values{"code": {code}, "state": {query.Get("state")}} @@ -379,7 +402,7 @@ func (i *Issuer) exchangeCode(w http.ResponseWriter, r *http.Request) { return } response := map[string]any{ - "access_token": i.mintAccessToken("user-1", grant.scopes), + "access_token": i.mintAccessTokenFor("user-1", grant.scopes, grant.resource), "token_type": "Bearer", "expires_in": 300, "id_token": i.mintIDToken(grant.clientID, grant.nonce), @@ -481,6 +504,14 @@ func (i *Issuer) clientCredentialsGrant(w http.ResponseWriter, r *http.Request) oauthError(w, http.StatusUnauthorized, "invalid_client") return } + resource := r.PostForm.Get("resource") + // There is no earlier authorization for this grant to inherit a binding + // from, so a deployment that decides the audience per request has nothing to + // go on and refuses outright. + if i.opts.RequireResource && resource == "" { + oauthError(w, http.StatusBadRequest, "invalid_target") + return + } requested := splitScopes(r.PostForm.Get("scope")) issued := requested switch i.opts.ClientScopeMode { @@ -495,7 +526,7 @@ func (i *Issuer) clientCredentialsGrant(w http.ResponseWriter, r *http.Request) return } writeJSON(w, http.StatusOK, map[string]any{ - "access_token": i.mintAccessToken("client-1", issued), + "access_token": i.mintAccessTokenFor("client-1", issued, resource), "token_type": "Bearer", "expires_in": 300, "scope": strings.Join(issued, " "), @@ -563,11 +594,23 @@ func (i *Issuer) handleIntrospect(w http.ResponseWriter, r *http.Request) { // mintAccessToken signs a real RS256 access token and records it for // introspection. func (i *Issuer) mintAccessToken(subject string, scopes []string) string { + return i.mintAccessTokenFor(subject, scopes, "") +} + +// mintAccessTokenFor mints access bound to one named resource, falling back to +// the registration's audience when the request named none. A deployment that +// takes a resource indicator binds the token to it and to nothing else, which +// is the whole reason the indicator is worth sending. +func (i *Issuer) mintAccessTokenFor(subject string, scopes []string, resource string) string { + audience := i.opts.Audience + if resource != "" { + audience = resource + } now := time.Now() token := i.sign(map[string]any{ "iss": i.URL, "sub": subject, - "aud": i.opts.Audience, + "aud": audience, "scope": strings.Join(scopes, " "), "exp": now.Add(5 * time.Minute).Unix(), "iat": now.Unix(), @@ -575,7 +618,7 @@ func (i *Issuer) mintAccessToken(subject string, scopes []string) string { i.mutex.Lock() i.accessTokens[token] = tokenRecord{ scopes: append([]string(nil), scopes...), - audience: i.opts.Audience, + audience: audience, subject: subject, } i.mutex.Unlock() diff --git a/internal/auth/narrowing.go b/internal/auth/narrowing.go index 4025bde..30a0a62 100644 --- a/internal/auth/narrowing.go +++ b/internal/auth/narrowing.go @@ -46,6 +46,17 @@ const narrowingRecovery = "Check the deployment's API resource registration and "granted to the registered OAuth application, then retry. The shell does not hand a module " + "broader access than it asked for." +// indicatorRecovery is the way back from a deployment that will not issue +// access without being told which protected resource it is for. +// +// It is a different instruction from every other narrowing refusal, because +// nothing about the deployment is wrong: the context document did not say what +// kind of deployment this is, so the shell asked in a shape this one does not +// accept. +const indicatorRecovery = "This deployment binds access to one named resource and will not issue " + + "any without being told which. Name the identity provider on this identity in the context " + + "document, then retry." + // verify proves an issued token is exactly what the module asked for. // // It is the check the whole derivation exists to make. A deployment may answer diff --git a/internal/auth/oauthflow/login.go b/internal/auth/oauthflow/login.go index f92098a..f375b5a 100644 --- a/internal/auth/oauthflow/login.go +++ b/internal/auth/oauthflow/login.go @@ -92,6 +92,16 @@ type Login struct { // defaults to standard output, because a login whose URL goes nowhere is // a login nobody can complete. Out io.Writer + // Resource is the protected resource this session is for, sent as an RFC + // 8707 resource indicator. It is empty for a deployment that decides the + // audience from the application's registration instead, and naming one + // there would ask for a narrowing the deployment has no way to honor. + // + // It belongs to the login rather than to the grant that follows because the + // deployments that take it decide the audience once, at authorization: a + // session established without it cannot be bound to a resource afterwards, + // and one established with it reaches that resource and no other. + Resource string // Ports overrides LoopbackPorts. Tests bind an ephemeral port with []int{0}. Ports []int } @@ -165,9 +175,18 @@ func (l Login) Run(ctx context.Context) (Result, error) { RedirectURL: redirectURL(listener), Scopes: l.scopes(), } - authURL := config.AuthCodeURL(state, + authOptions := []oauth2.AuthCodeOption{ oauth2.S256ChallengeOption(verifier), - oidc.Nonce(nonce)) + oidc.Nonce(nonce), + } + // The indicator is sent on both legs. The authorization request is what + // binds the session, and the exchange repeats it because a deployment is + // entitled to check that the code it is redeeming was asked for on the same + // terms it was issued under. + if l.Resource != "" { + authOptions = append(authOptions, oauth2.SetAuthURLParam("resource", l.Resource)) + } + authURL := config.AuthCodeURL(state, authOptions...) if _, err := fmt.Fprintf(l.out(), "Open this URL to log in:\n%s\n", authURL); err != nil { return Result{}, notCompleted("the shell could not print the authorization URL this login needs", "Run wso2 login with standard output attached to your terminal.") @@ -180,7 +199,11 @@ func (l Login) Run(ctx context.Context) (Result, error) { if err != nil { return Result{}, err } - token, err := config.Exchange(ctx, code, oauth2.VerifierOption(verifier)) + exchangeOptions := []oauth2.AuthCodeOption{oauth2.VerifierOption(verifier)} + if l.Resource != "" { + exchangeOptions = append(exchangeOptions, oauth2.SetAuthURLParam("resource", l.Resource)) + } + token, err := config.Exchange(ctx, code, exchangeOptions...) if err != nil { return Result{}, notCompleted("the identity provider refused to exchange this login for a session", "Retry wso2 login. If it keeps failing, confirm the client identifier and the registered "+ diff --git a/internal/auth/oauthflow/resource_test.go b/internal/auth/oauthflow/resource_test.go new file mode 100644 index 0000000..2739dce --- /dev/null +++ b/internal/auth/oauthflow/resource_test.go @@ -0,0 +1,101 @@ +// 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_test + +import ( + "net/url" + "slices" + "strings" + "testing" + "time" + + "github.com/wso2/wso2-cli/internal/auth/fakeissuer" +) + +// theResource is the protected resource a resource-binding deployment mints +// access for. It is an absolute URI because the deployments that require one +// refuse anything else. +const theResource = "https://deployment.example.test/reference-status" + +// A deployment that decides the audience at authorization time gives the +// session one protected resource, and the session is only useful for it. The +// login has to say which, because nothing later in the exchange can. +func TestLoginBindsTheSessionToTheResourceItWasGiven(t *testing.T) { + issuer := fakeissuer.New(t, fakeissuer.Options{ + RequireResource: true, + AllowAnyLoopbackPort: true, + }) + printed := &recorder{} + login := browserLogin(issuer, printed, func(authURL string) error { + go visit(issuer, authURL) + return nil + }) + login.Resource = theResource + + result, err := login.Run(testContext(t, 30*time.Second)) + if err != nil { + t.Fatalf("login: %v", err) + } + _, _, audience := issuer.Introspect(t, result.Token.AccessToken) + if !slices.Contains(audience, theResource) { + t.Fatalf("the session was not bound to the resource it named: audience %v", audience) + } + if !strings.Contains(printed.String(), url.QueryEscape(theResource)) { + t.Fatalf("the authorization URL carried no resource indicator:\n%s", printed.String()) + } +} + +// The refusal belongs to the deployment, and the login reports it as a login +// that did not complete rather than inventing a reason of its own. +func TestALoginWithoutAResourceIsRefusedByADeploymentThatRequiresOne(t *testing.T) { + issuer := fakeissuer.New(t, fakeissuer.Options{ + RequireResource: true, + AllowAnyLoopbackPort: true, + }) + printed := &recorder{} + login := browserLogin(issuer, printed, func(authURL string) error { + go visit(issuer, authURL) + return nil + }) + + _, err := login.Run(testContext(t, 30*time.Second)) + if err == nil { + t.Fatal("a login carrying no resource indicator completed against a deployment that requires one") + } + requireProblem(t, err, "auth.credential_unavailable") +} + +// A deployment that binds no audience at authorization time must be unaffected, +// or naming a resource would change what every existing login asks for. +func TestALoginCarriesNoResourceIndicatorUnlessItWasGivenOne(t *testing.T) { + issuer := fakeissuer.New(t, fakeissuer.Options{ + Audience: "reference-status", + AllowAnyLoopbackPort: true, + }) + printed := &recorder{} + login := browserLogin(issuer, printed, func(authURL string) error { + go visit(issuer, authURL) + return nil + }) + + if _, err := login.Run(testContext(t, 30*time.Second)); err != nil { + t.Fatalf("login: %v", err) + } + if strings.Contains(printed.String(), "resource=") { + t.Fatalf("a login that was given no resource sent one anyway:\n%s", printed.String()) + } +} diff --git a/internal/auth/resource_test.go b/internal/auth/resource_test.go new file mode 100644 index 0000000..0595e80 --- /dev/null +++ b/internal/auth/resource_test.go @@ -0,0 +1,60 @@ +// 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 auth_test + +import ( + "testing" + + "github.com/wso2/wso2-cli/internal/auth/fakeissuer" + "github.com/wso2/wso2-cli/internal/contexts" +) + +// A deployment that decides the audience per request will not issue at all +// without being told which resource the access is for. The module asked for an +// audience; the shell has to carry it as the indicator the deployment reads. +func TestAnInlineIdentityBindsAccessToTheResourceItsProductNames(t *testing.T) { + deployment := deployInline(t, fakeissuer.Options{RequireResource: true}) + broker := deployment.broker(t) + broker.Selection.Identity.Auth.Provider = contexts.ProviderThunder + + grant, err := broker.Acquire(declaredRequest()) + if err != nil { + t.Fatalf("acquire: %v", err) + } + _, scopes, audiences := deployment.issuer.Introspect(t, grant.Token) + if len(audiences) != 1 || audiences[0] != audience { + t.Fatalf("access was not bound to the resource the module named: audience %v", audiences) + } + if len(scopes) != 1 || scopes[0] != readScope { + t.Fatalf("access carried %v, want exactly [%s]", scopes, readScope) + } +} + +// The indicator is sent because the identity says this deployment reads one. +// An identity that says nothing must keep asking exactly as it did before, or +// every deployment already working would start receiving a parameter it never +// agreed to interpret. +func TestAnInlineIdentityWithoutAResourceDerivationSendsNoIndicator(t *testing.T) { + deployment := deployInline(t, fakeissuer.Options{RequireResource: true}) + broker := deployment.broker(t) + + refusal := denied(t, broker, declaredRequest()) + + if refusal.Problem.Code != "auth.narrowing_unavailable" { + t.Errorf("code = %q, want auth.narrowing_unavailable", refusal.Problem.Code) + } +} diff --git a/internal/auth/source_clientcred.go b/internal/auth/source_clientcred.go index 54ef968..f6205da 100644 --- a/internal/auth/source_clientcred.go +++ b/internal/auth/source_clientcred.go @@ -75,10 +75,19 @@ func (s clientCredentialsSource) mint(request Request, now time.Time) (Grant, er // registered for every permission the deployment will ever need from // automation, and asking for that whole set would hand one module the // authority of all of them. - issued, err := requestToken(ctx, s.client, endpoint, url.Values{ + form := url.Values{ "grant_type": {"client_credentials"}, "scope": {strings.Join(request.Scopes, " ")}, - }, clientAuth{id: s.identity.Auth.ClientID, secret: s.secret}) + } + // A deployment that decides the audience per request is told which one, and + // the module's own request is what names it. There is no earlier + // authorization for this grant to inherit a binding from, so the indicator + // is the only thing that can bind the token at all. + if s.identity.Auth.Derivation() == contexts.DerivationTokenResource { + form.Set("resource", request.Audience) + } + issued, err := requestToken(ctx, s.client, endpoint, form, + clientAuth{id: s.identity.Auth.ClientID, secret: s.secret}) if err != nil { return Grant{}, s.refusedGrant(err) } @@ -98,6 +107,11 @@ func (s clientCredentialsSource) mint(request Request, now time.Time) (Grant, er func (s clientCredentialsSource) refusedGrant(err error) error { var refusal issuerRefusal switch { + case errors.As(err, &refusal) && refusal.requiresResourceIndicator(): + return denial("auth.narrowing_unavailable", + fmt.Sprintf("the deployment will not issue access for the %q module without being told "+ + "which protected resource it is for", s.namespace), + indicatorRecovery) case errors.As(err, &refusal) && refusal.refusedToNarrow(): return denial("auth.narrowing_unavailable", fmt.Sprintf("the deployment refused to issue access limited to the permissions the %q "+ diff --git a/internal/auth/tokenrequest.go b/internal/auth/tokenrequest.go index bba01a6..23bcef1 100644 --- a/internal/auth/tokenrequest.go +++ b/internal/auth/tokenrequest.go @@ -80,6 +80,17 @@ func (r issuerRefusal) refusedToNarrow() bool { return r.status == http.StatusBadRequest && r.code == "invalid_scope" } +// requiresResourceIndicator reports RFC 8707's answer for a request that named +// no protected resource on a deployment that decides the audience from one. +// +// It is worth telling apart from every other refusal because it is the one +// caused by the context document rather than by the deployment: the identity +// did not say which product this deployment binds access to, and the fix is a +// line in a document rather than a change to a registration. +func (r issuerRefusal) requiresResourceIndicator() bool { + return r.status == http.StatusBadRequest && r.code == "invalid_target" +} + // rejectedClient reports the deployment declining the credentials the request // identified its client with. func (r issuerRefusal) rejectedClient() bool { From 27a723bb289e0e96d87063c60d7c1e9585b42bd5 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 19:54:32 +0530 Subject: [PATCH 3/8] test(smoke): describe a Thunder deployment, and name its secret in code A live run against Thunder differs from the two deployments already described in two ways, and both follow from Thunder deciding an access token's audience per request: the audience is the resource server's identifier and must be an absolute URI, and the run has to say which identity provider it is describing or the login is refused with invalid_target before a session exists. The provider is validated when the environment is read rather than left to the document, so a deployment described with a name this shell does not read is reported before a run reaches a browser and wastes a person's attention. The non-interactive run needs a client secret. Its variable is named in code, beside the secure-store reference that is already fixed there, and deliberately not in the deployment description: that file is one people copy and keep, and naming the variable in it invites the value to be pasted beside the name. The run reads the variable from the process environment exactly as the shell reads the variable a context names. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- test/smoke/config.go | 59 ++++++++++++++++++- test/smoke/env.example | 37 ++++++++++++ test/smoke/thunder_config_test.go | 97 +++++++++++++++++++++++++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 test/smoke/thunder_config_test.go diff --git a/test/smoke/config.go b/test/smoke/config.go index 2545c46..93481fa 100644 --- a/test/smoke/config.go +++ b/test/smoke/config.go @@ -66,8 +66,13 @@ const ( // slice calls it, so it exists only to keep the document honest. EndpointVar = "WSO2_SMOKE_ENDPOINT" // IdentityTypeVar is "cloud" for Asgardeo or "onprem" for an Identity - // Server deployment. It is optional and defaults to cloud. + // Server or Thunder deployment. It is optional and defaults to cloud. IdentityTypeVar = "WSO2_SMOKE_IDENTITY_TYPE" + // ProviderVar names the identity provider behind the issuer, which decides + // how the shell derives a module's access from the login. It is optional: + // left unset, the run describes a deployment that binds audiences from the + // application's registration, which is Asgardeo and Identity Server. + ProviderVar = "WSO2_SMOKE_PROVIDER" // UnregisteredPortVar is the loopback port the any-port experiment binds: // one deliberately outside the registered 10425-10428 range. UnregisteredPortVar = "WSO2_SMOKE_UNREGISTERED_PORT" @@ -97,6 +102,23 @@ const ( // a smoke run cannot overwrite a real session, and a cleanup that deletes // it cannot delete one. CredentialRef = "wso2-cli-smoke" + // SecretVariable names the environment variable the non-interactive run + // reads its client secret from. + // + // It is fixed here rather than described alongside the deployment, and that + // is the point. A deployment description is a file people copy, share, and + // keep; naming the secret's variable in one invites the value to be pasted + // beside the name. The run reads the variable from the process environment, + // exactly as the shell reads the variable a context names, so the secret + // lives in the shell that exported it and nowhere else. + SecretVariable = "WSO2_SMOKE_CLIENT_SECRET" + // CIClientIDVar names the confidential client the non-interactive run + // presents. It is a name, not a credential. + CIClientIDVar = "WSO2_SMOKE_CI_CLIENT_ID" + // CIIdentityName is the non-interactive identity's name. + CIIdentityName = "smoke-ci-identity" + // CIContextName is the non-interactive context's name. + CIContextName = "smoke-ci" ) // Defaults for the knobs a run rarely sets. @@ -128,6 +150,12 @@ type Config struct { Endpoint string // IdentityType is "cloud" or "onprem". IdentityType string + // Provider names the identity provider behind the issuer, or is empty when + // the deployment binds audiences from the application's registration. + Provider string + // CIClientID is the confidential client the non-interactive run presents, + // or empty when no such run is configured. + CIClientID string // UnregisteredPort is the loopback port the any-port experiment binds. UnregisteredPort int // Deadline bounds a run that is waiting on a human. @@ -206,6 +234,19 @@ func Load(lookup func(string) (string, bool)) (Config, error) { config.IdentityType = declared } + // The provider is checked here rather than left to the document, so a + // deployment described with a name this shell does not read is reported + // before a run reaches a browser. + if declared := read(ProviderVar); declared != "" { + if !slices.Contains(readableProviders(), declared) { + return Config{}, fmt.Errorf( + "%s: %q is not an identity provider this shell reads; use one of %s", + ProviderVar, declared, strings.Join(readableProviders(), ", ")) + } + config.Provider = declared + } + config.CIClientID = read(CIClientIDVar) + config.UnregisteredPort = defaultUnregisteredPort if declared := read(UnregisteredPortVar); declared != "" { port, err := strconv.Atoi(declared) @@ -242,6 +283,17 @@ func Empirical(lookup func(string) (string, bool)) bool { // ones oauthflow binds by default. func RegisteredPorts() []int { return []int{10425, 10426, 10427, 10428} } +// readableProviders are the identity providers a deployment description may +// name. It is the shell's own list, so a run cannot describe a deployment the +// context document would then refuse. +func readableProviders() []string { + return []string{ + contexts.ProviderAsgardeo, + contexts.ProviderIdentityServer, + contexts.ProviderThunder, + } +} + // Document is the schema version 2 context document a live run installs. // // It is built rather than hand-written so that a run cannot drift from the @@ -260,6 +312,11 @@ func (c Config) Document() contexts.Document { ClientID: c.ClientID, Tenant: c.Tenant, CredentialRef: CredentialRef, + // Naming the provider is what makes the run derive the way the + // deployment requires. Left empty it is simply absent from the + // document, which is the open-world case and what every run + // against Asgardeo or Identity Server describes. + Provider: c.Provider, }, Products: map[string]contexts.Product{ Namespace: { diff --git a/test/smoke/env.example b/test/smoke/env.example index c4cfbca..7f825df 100644 --- a/test/smoke/env.example +++ b/test/smoke/env.example @@ -74,6 +74,43 @@ export WSO2_SMOKE_AUDIENCE="$WSO2_SMOKE_CLIENT_ID" # export WSO2_SMOKE_SCOPE='reference:status:read reference:status:write' # export WSO2_SMOKE_IDENTITY_TYPE=onprem +# --- ThunderID, locally -------------------------------------------------- +# +# Delete the blocks above and uncomment this one. Two things here are unlike +# either deployment above, and both are Thunder deciding the audience per +# request rather than from the application's registration. +# +# AUDIENCE is an absolute URI, and it is the resource server's identifier. +# Thunder refuses an identifier that is not a URI, so the bare API resource name +# that works on Identity Server fails here before a browser opens. +# +# PROVIDER is what makes the run derive the way Thunder requires. Thunder wants +# an RFC 8707 resource indicator on the authorization request and accepts only +# one; without the provider named, the login is refused with invalid_target and +# no session is established. Naming it also binds this run to one product, which +# is all one Thunder login can reach. +# +# The issuer is the bare origin, not a path under it. Confirm it rather than +# assuming, and use the `issuer` value verbatim: +# +# curl -sk https://localhost:8490/.well-known/openid-configuration +# +# This deployment serves a self-signed certificate and has to be trusted by the +# operating system before login can reach it. See the Thunder walkthrough. + +# export WSO2_SMOKE_ISSUER='https://localhost:8490' +# export WSO2_SMOKE_CLIENT_ID='wso2-cli' +# export WSO2_SMOKE_AUDIENCE='https://localhost:8490/reference-status' +# export WSO2_SMOKE_SCOPE='read write' +# export WSO2_SMOKE_PROVIDER=thunder +# export WSO2_SMOKE_IDENTITY_TYPE=onprem + +# The non-interactive run also needs a confidential client, and that client's +# secret. The client's name belongs here; the secret does not, and neither does +# the name of the variable holding it. RUNNING.md says which variable to export +# and why it is named there rather than here. +# export WSO2_SMOKE_CI_CLIENT_ID='wso2-cli-ci' + # --- Optional ------------------------------------------------------------ # The identity's home organization. Left unset, the smoke context names none. diff --git a/test/smoke/thunder_config_test.go b/test/smoke/thunder_config_test.go new file mode 100644 index 0000000..fe5a8a2 --- /dev/null +++ b/test/smoke/thunder_config_test.go @@ -0,0 +1,97 @@ +// 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 smoke_test + +import ( + "testing" + + "github.com/wso2/wso2-cli/internal/contexts" + "github.com/wso2/wso2-cli/test/smoke" +) + +// thunderEnvironment is a Thunder deployment as the environment describes one. +// The audience is an absolute URI because Thunder refuses a resource identifier +// that is not. +func thunderEnvironment() map[string]string { + return map[string]string{ + smoke.IssuerVar: "https://localhost:8490", + smoke.ClientIDVar: "wso2-cli", + smoke.AudienceVar: "https://localhost:8490/reference-status", + smoke.ScopeVar: "read write", + smoke.ProviderVar: contexts.ProviderThunder, + smoke.IdentityTypeVar: "onprem", + } +} + +// A run against a deployment that binds access to a named resource has to say +// so, or the document it installs derives the way every other deployment does +// and the login is refused before a browser opens. +func TestADeploymentMayDeclareItsIdentityProvider(t *testing.T) { + config, err := smoke.Load(environment(thunderEnvironment())) + if err != nil { + t.Fatalf("load: %v", err) + } + identity := config.Document().Identities[0] + if identity.Auth.Provider != contexts.ProviderThunder { + t.Fatalf("the document names the provider %q, want %q", + identity.Auth.Provider, contexts.ProviderThunder) + } + if got := identity.Auth.Derivation(); got != contexts.DerivationTokenResource { + t.Fatalf("a Thunder deployment derives by %q, want %q", + got, contexts.DerivationTokenResource) + } +} + +// The document a live run installs is read by the shell before any browser +// opens, so a Thunder run cannot fail on a document defect in front of a +// waiting human. +func TestAThunderDocumentIsReadableByTheShell(t *testing.T) { + config, err := smoke.Load(environment(thunderEnvironment())) + if err != nil { + t.Fatalf("load: %v", err) + } + encoded, err := config.Document().Encode() + if err != nil { + t.Fatalf("encode: %v", err) + } + if _, err := contexts.Decode(encoded); err != nil { + t.Fatalf("the shell refused the document a Thunder run installs: %v", err) + } +} + +// A provider this shell does not read is a description of a deployment it +// cannot act on, and a run that reached a browser first would waste the sign-in. +func TestAnUnknownProviderIsRefusedBeforeAnyRun(t *testing.T) { + values := thunderEnvironment() + values[smoke.ProviderVar] = "acme-idp" + if _, err := smoke.Load(environment(values)); err == nil { + t.Fatal("an unreadable identity provider was accepted") + } +} + +// A deployment description names no secret and no secret's variable. The +// non-interactive run needs one, so the name it reads is fixed here instead, +// where nobody is invited to paste a value beside it. +func TestTheNonInteractiveSecretVariableIsNamedInCodeOnly(t *testing.T) { + if smoke.SecretVariable == "" { + t.Fatal("the non-interactive run names no environment variable for its client secret") + } + if smoke.SecretVariable == smoke.ClientIDVar || smoke.SecretVariable == smoke.IssuerVar { + t.Fatalf("the client secret variable collides with a deployment variable: %q", + smoke.SecretVariable) + } +} From 9fe44a0411bdd983b9e4d94529c0d3f6780801b6 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 20:04:35 +0530 Subject: [PATCH 4/8] docs: record what Thunder measures, and how to register against it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walkthrough is its own guide rather than a third numbered section of the login guide. Thunder is pre-1.0 and will change faster than the other two products, and churn is cheapest in a file nothing cross-references; the login guide was also already long enough that a third inline walkthrough would have pushed it past thirteen hundred lines. The asymmetry is a compromise and is filed as its own issue rather than left as a wart. Every step in it was run against the deployment it describes, including the console navigation: the resource hierarchy panel, the OAuth2 configuration under Advanced Settings, and the four loopback callbacks on the General tab. The version it was written against is named, because a console layout is the fastest thing in an alpha product to go stale and a reader needs to know what the steps described. The research document gains Thunder's verdict cells beside Asgardeo's and Identity Server's, each with the date and the deployment that produced it. It also gains a row the other two never raise — where the audience is decided — because that is the difference everything else follows from. Two findings are recorded that a reader would otherwise measure differently and conclude the other was wrong: a default resource server makes the resource indicator optional and binds every token to the same audience, and a resource handle cannot contain the delimiter that joins permission names. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- docs/guides/login-thunder.md | 368 ++++++++++++++++++ docs/guides/login.md | 6 + ...gardeo-redirect-uri-and-scope-narrowing.md | 73 ++++ 3 files changed, 447 insertions(+) create mode 100644 docs/guides/login-thunder.md diff --git a/docs/guides/login-thunder.md b/docs/guides/login-thunder.md new file mode 100644 index 0000000..612c0c3 --- /dev/null +++ b/docs/guides/login-thunder.md @@ -0,0 +1,368 @@ +# Logging in with the WSO2 CLI: ThunderID + +This is the registration walkthrough for **ThunderID**, the third deployment +`wso2 login` supports. Asgardeo and WSO2 Identity Server are covered by +[the main login guide](login.md), sections 2 and 3, and everything after +registration — writing the context document, logging in, CI, troubleshooting — +is the same document for all three products. Read this one for the registration, +then return to [section 4 of the login guide](login.md#4-write-the-context-document). + +It is a separate file for two reasons. Thunder is pre-1.0 and will change faster +than the other two, and churn is cheapest in a file nothing cross-references. +And the login guide is already long enough that a third walkthrough inside it +would push it past thirteen hundred lines. Splitting all three product +walkthroughs symmetrically is [tracked separately](https://github.com/wso2/wso2-cli/issues/45). + +**Written against ThunderID `v1.0.0-beta`.** Console layouts move in an alpha +and beta product; if a control named here is not where this says, the version +is the first thing to check. The container recipe below pins that exact version +so the two cannot drift apart. + +--- + +## 1. What is different about Thunder + +Everything in this section has a consequence further down, so it is worth +reading before registering anything. + +**Thunder decides an access token's audience per request.** Asgardeo and +Identity Server decide it from the application's registration; Thunder reads an +[RFC 8707](https://www.rfc-editor.org/rfc/rfc8707) *resource indicator* on the +authorization request and mints the token for exactly that resource server. The +shell sends the indicator when the context document names Thunder as the +identity provider, and not otherwise. + +Three things follow: + +- **The audience is a URI.** A resource server's identifier must be an absolute + URI, so `products..audience` is a URI here. The bare API resource + identifier that works on Identity Server is refused. +- **One login reaches one product.** Thunder accepts a single resource indicator + per authorization — *"Only a single resource parameter is supported"*. A + session is therefore bound to one resource server, and the context document + refuses an identity that names Thunder and declares more than one product. + Lifting that is [tracked separately](https://github.com/wso2/wso2-cli/issues/43). +- **The audience check means what it says.** On Asgardeo an access token's `aud` + is the client ID and cannot distinguish one product from another. On Thunder it + is the resource server identifier and nothing else, which is the strongest + audience guarantee of the three products. + +**Thunder has no device authorization grant.** Its discovery document advertises +no `device_authorization_endpoint` and its grant handlers register none. +`wso2 login --device-code` cannot work against a Thunder-backed deployment; +browser login is the interactive path. + +--- + +## 2. Run a deployment + +```sh +docker run -d --name thunderid -p 8090:8090 \ + ghcr.io/thunder-id/thunderid:1.0.0-beta \ + bash -c './setup.sh --admin-username admin --admin-password "Admin@123" && ./start.sh' +``` + +`setup.sh` generates the deployment's keys and certificates and seeds the +default resources; `start.sh` serves. It answers in well under a minute. Nothing +is persisted outside the container, so `docker rm -f thunderid` returns the +machine to where it started — which is the reason to prefer it while you are +learning the console, where a half-registered application from a previous +attempt is hard to tell from a correct one. + +Thunder runs standalone on embedded storage. It needs no database and no cache +beside it, whatever a compose file you may have seen alongside it suggests. + +**Pin the version.** `latest` has carried an older alpha than the newest release +during this walkthrough's lifetime, so a recipe that uses it describes whatever +was pushed last rather than what is written here. + +**If port 8090 is taken**, publishing on a different host port is not enough on +its own. Thunder advertises its own public URL in its discovery document, and +the shell checks that document against the issuer it was fetched from, so the +advertised URL and the URL you reach it on have to agree. Change the advertised +one to match: + +```sh +docker run -d --name thunderid -p 8490:8090 \ + ghcr.io/thunder-id/thunderid:1.0.0-beta \ + bash -c 'sed -i "s|public_url: \"https://localhost:8090\"|public_url: \"https://localhost:8490\"|" deployment.yaml \ + && ./setup.sh --admin-username admin --admin-password "Admin@123" && ./start.sh' +``` + +Confirm what it advertises rather than assuming it: + +```sh +curl -sk https://localhost:8090/.well-known/openid-configuration +``` + +Note the issuer is the **bare origin** — `https://localhost:8090` — not a path +under it. Identity Server's issuer is `https://localhost:9443/oauth2/token`; the +shape is not the same and using one product's shape against the other fails at +discovery. + +The console is at `https://localhost:8090/console`, with the administrator +credentials the recipe set. + +--- + +## 3. Trust the deployment's certificate + +Thunder serves TLS with a minimum version of 1.3 and, on a fresh deployment, a +self-signed certificate. The shell uses the process's ordinary HTTP client and +has no flag anywhere for a custom certificate authority, so until that +certificate is trusted, login cannot reach discovery at all: + +``` +tls: failed to verify certificate: x509: certificate signed by unknown authority +``` + +On macOS, note that Go **ignores `SSL_CERT_FILE`** — `crypto/x509` honors it on +every Unix except Darwin — so the keychain is the only way in. Take the +certificate from the port: + +```sh +openssl s_client -connect localhost:8090 -servername localhost /dev/null \ + | openssl x509 -outform pem > thunder-localhost.pem + +security add-trusted-cert -r trustRoot -p ssl \ + -k ~/Library/Keychains/login.keychain-db thunder-localhost.pem +``` + +**This trade is narrower than the equivalent one for Identity Server**, and the +difference is worth knowing. Identity Server ships a `CA:TRUE` certificate whose +private key is inside every download and every copy of the public container +image, behind a published password; trusting it means trusting a signing key +anyone can obtain, for any hostname. Thunder's is generated by `setup.sh` on +the deployment that serves it, so trusting it trusts that deployment and +nothing else. `-p ssl` confines it to TLS and the login keychain confines it to +your user. + +Remove it when you are done: + +```sh +security delete-certificate -c localhost ~/Library/Keychains/login.keychain-db +``` + +--- + +## 4. Register the API as a resource server + +This is the step with no equivalent name on the other two products. Asgardeo and +Identity Server call it an API resource; Thunder calls it a **resource server**, +and its identifier is what lands in an access token's `aud`. + +1. **Resource Servers → Add resource server**. +2. **Name**: `Reference Status`. +3. **Identifier**: `https://localhost:8090/reference-status`. + + It must be an **absolute URI**. Thunder refuses anything else with + `invalid_target: Invalid resource parameter: must be an absolute URI`, and + the refusal arrives at login rather than at registration, so a bare name here + costs a browser sign-in to discover. + +Then add the permissions the reference module asks for. On the resource +server's **Resources** tab there is a **Resource Hierarchy** panel: + +4. Use the **+** on the panel header to add a top-level resource. Give it a + **Name** and a **Handle**; the **Permission** shown beside it is what a + token's `scope` will carry, and the handle is immutable once created. +5. Use the **+** on a resource's own row to add a child beneath it. + +**Handles cannot contain the resource server's delimiter**, which is `:` by +default. A handle of `reference:status:read` is refused with +`Delimiter conflict in handle`. A permission of that shape is built as a +hierarchy instead — `reference`, with a `status` child, with `read` and `write` +children under that — and Thunder joins the handles with the delimiter to +produce the permission. + +For a first run, two flat resources are enough and simpler to verify: handles +`read` and `write`, giving permissions `read` and `write`. + +### Do not set a default resource server + +The resource server page carries a **Set as default** action, whose confirmation +says *"Requests without a resource parameter will fall back to it."* It does +exactly that, and it is worth understanding rather than using. + +With a default configured, Thunder issues tokens for requests that name no +resource — and binds every one of them to the default, whichever product asked. +That is precisely the weakness the shell's audience check exists to avoid, and +it is the one thing that would make Thunder's audience guarantee no better than +Asgardeo's. Leave it unset; the shell names the resource on every request and +does not need the fallback. + +--- + +## 5. Register the CLI as a public client + +1. **Applications → Add Application → Custom**. + + Custom is the type that exposes the whole OAuth configuration. The + technology-named types (React, Node.js, and so on) preset choices that do not + describe a command-line tool. + +2. **Name** it `WSO2 CLI`, then **Finish**. The OAuth settings are configured + after creation, on the application's own page. + +3. On the **General** tab, under **Authorized redirect URIs**, add all four + loopback callbacks with **Add URI**: + + ``` + http://127.0.0.1:10425/callback + http://127.0.0.1:10426/callback + http://127.0.0.1:10427/callback + http://127.0.0.1:10428/callback + ``` + + These are the ports the shell binds, in order, taking the first that is free. + Registering all four is what lets a login succeed when something else on the + machine already holds one of them. + +4. On the **Advanced Settings** tab, under **OAuth2 Configuration**: + + - **Grant Types**: `authorization_code` and `refresh_token`, and nothing else. + The refresh grant is what every later per-module acquisition narrows from; + without it a login succeeds and no module can be granted anything. + - **Response Types**: `code`. + - **Public Client**: on. **Client Authentication Method** then locks itself to + `none`, which is correct — the shell is a public client and holds no secret. + - **PKCE Required**: locked on, labelled *"Always required for public + clients."* Nothing to do; Thunder does not let a public client skip it. + +5. Leave **Default Audience** empty. It applies only to tokens that target no + resource server, and the shell always targets one. + +--- + +## 6. Create a user, and grant it the permissions + +1. **Users → add a user**, with a username and password you will sign in with. +2. **Roles → add a role**, for example `Reference Status Caller`. +3. On the role, add **permissions**, choosing the `Reference Status` resource + server and the permissions you created under it. +4. Assign the role to the user. + +A user with no such role signs in successfully and receives a token stating no +permissions, which the shell then refuses — see the troubleshooting note below. + +--- + +## 7. A confidential client for CI, if you need one + +For the non-interactive path, register a second application: + +1. **Applications → Add Application → Custom**, named `WSO2 CLI CI`. +2. **Advanced Settings → OAuth2 Configuration**: **Grant Types** + `client_credentials`; **Public Client** off; **Client Authentication Method** + `client_secret_basic`. +3. Record the client ID and secret. + +The client-credentials grant has no earlier authorization to inherit a resource +binding from, so the shell sends the resource indicator on that request too. A +Thunder deployment refuses the grant outright without it. + +--- + +## 8. Record what you need + +- **Client ID** of the `WSO2 CLI` application. +- **Issuer**, the bare origin, taken verbatim from the `issuer` value in + `https://localhost:8090/.well-known/openid-configuration`. +- **Audience**, which is the resource server's **identifier** — the absolute URI + from section 4, not its name. +- **Scopes**, the permissions from section 4. +- **Whether this machine trusts the deployment's certificate** (section 3). + +--- + +## 9. Write the context document + +Everything from here is [the main login guide](login.md), from section 4. Two +members are Thunder-specific: + +```json +{ + "name": "thunder-local", + "type": "onprem", + "auth": { + "kind": "oauth-browser", + "provider": "thunder", + "issuer": "https://localhost:8090", + "clientId": "wso2-cli", + "credentialRef": "thunder-local-login" + }, + "products": { + "reference": { + "endpoint": "https://localhost:8090", + "audience": "https://localhost:8090/reference-status", + "scopes": ["read", "write"] + } + } +} +``` + +`provider` is what makes the shell send the resource indicator. Without it the +login is refused with `invalid_target` and no session is established. + +You may also write `narrowing` explicitly — `scoped-refresh` or +`token-resource` — for a deployment that does not behave the way its product +ordinarily does. An explicit `narrowing` wins over what `provider` implies. A +Thunder deployment with a default resource server configured is the case this +exists for. + +--- + +## 10. Troubleshooting + +### `auth.narrowing_unavailable`, mentioning a protected resource + +> the deployment will not issue access for the "reference" module without being +> told which protected resource it is for + +The identity does not name Thunder as its provider, so the shell asked in a +shape this deployment does not accept. Add `"provider": "thunder"` to the +identity's `auth` block. + +### `contexts.document_malformed`, about one login and one product + +The identity derives access by resource and declares more than one product. +Thunder accepts one resource indicator per authorization, so one session cannot +reach two products. Split them across two identities, each with its own +`credentialRef`. + +### `contexts.document_malformed`, about a product without an audience + +A resource-bound derivation has to name the resource it binds to, and it takes +that name from the product's `audience`. Add it. + +### `auth.narrowing_unavailable`, about permissions the deployment did not state + +Thunder issues a token stating no permissions when the client or user holds +none, rather than refusing the request. The shell cannot prove such a token +carries what was asked for, so it refuses. Check that the signed-in user holds a +role granting the permissions on the right resource server (section 6). + +### `auth.discovery_failed` at login + +Either the certificate is not trusted (section 3), or the issuer in the context +document is not what the deployment advertises. Compare it against the `issuer` +value in the discovery document, and remember it is the bare origin on Thunder. + +### The login page asks for credentials every time + +Expected on a default deployment, and measured: a second authorization request +minutes after a completed sign-in presented the sign-in form again. Whether a +single sign-on session can be configured is not established here. + +--- + +## 11. Proving it against this deployment + +The live runs in `test/smoke/` work against Thunder exactly as they do against +the other two products. `test/smoke/env.example` carries a Thunder block; copy +it, fill in what section 8 told you to record, and see +[`test/smoke/RUNNING.md`](../../test/smoke/RUNNING.md). + +The measured behaviour behind everything above is recorded in +[`docs/research/asgardeo-redirect-uri-and-scope-narrowing.md`](../research/asgardeo-redirect-uri-and-scope-narrowing.md) +§3.2, with the date and the deployment each verdict came from. diff --git a/docs/guides/login.md b/docs/guides/login.md index 086fb0d..01b9a87 100644 --- a/docs/guides/login.md +++ b/docs/guides/login.md @@ -16,6 +16,12 @@ Two audiences, one path. Sections 2 and 3 are alternatives — register the application in Asgardeo **or** in Identity Server 7.x, whichever you are targeting — and everything after them is the same for both. +Registering against **ThunderID** is a third alternative, and it lives in +[its own walkthrough](login-thunder.md) because Thunder decides an access +token's audience differently enough to change what you register and what you +write down. Read that instead of sections 2 and 3, then rejoin this guide at +section 4. + --- ## 1. What the shell needs from a deployment diff --git a/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md b/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md index c346b3b..e0e2840 100644 --- a/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md +++ b/docs/research/asgardeo-redirect-uri-and-scope-narrowing.md @@ -305,6 +305,79 @@ the client ID on Asgardeo and the API resource identifier on Identity Server, and carrying one product's value to the other costs a browser sign-in and ends in `auth.narrowing_unavailable`. +### 3.2 The same questions against ThunderID 1.0.0-beta + +Measured 2026-08-06 against `https://localhost:8490` — the +`ghcr.io/thunder-id/thunderid:1.0.0-beta` container, registered as +[the Thunder walkthrough](../guides/login-thunder.md) describes. A third +deployment, not a re-reading of either above. The host port is 8490 rather than +the default 8090 only because another container held 8090 on the machine that +ran this; the deployment's advertised public URL was changed to match, which is +what the walkthrough's port-offset recipe does. + +**This product answers a question the other two never raise**, so the table has +a row they do not: where the audience is decided. On Asgardeo and Identity +Server it is decided by the application's registration. On Thunder it is decided +per request, by an RFC 8707 resource indicator. + +| Question | Verdict | +|---|---| +| Refresh-grant scope narrowing | **Honoured.** A session established for `read write` was refreshed asking for `read` alone and received exactly that — the plain verdict, so the protocol scopes were dropped too. Same as both other products. | +| Access token `aud` | **The resource server's identifier, exactly and alone.** No client ID beside it. Stronger than Asgardeo, where `aud` is the client ID and cannot distinguish products, and cleaner than Identity Server 7.3.0, where it is the client ID plus the registered audiences. | +| Resource indicator on the **authorization** request | **Required.** Without one the flow is refused and the browser returns to the callback carrying `error=invalid_target`, `"No resource parameter supplied and no default resource server is configured"`. No session is established. | +| Resource indicator on the **refresh** grant | **Not required.** The refresh token inherits the binding established at authorization; a refresh carrying no indicator returned a token still bound to the original resource server. | +| Resource indicator on **client credentials** | **Required.** There is no earlier authorization to inherit from, and the grant is refused with `invalid_target` without one. | +| Multiple resource indicators | **Rejected** — *"Only a single resource parameter is supported."* | +| Resource server identifier format | **Must be an absolute URI.** A bare `reference-status` is refused with `"Invalid resource parameter: must be an absolute URI"`. | +| Unauthorised scopes on client credentials | **Silently dropped.** The grant succeeded and issued a token stating no scope at all, rather than refusing. The broker then refuses, because it cannot prove the token carries what was asked for. | +| Device authorization grant | **Absent.** No `device_authorization_endpoint` in the discovery document, confirming from a running deployment what the landscape research inferred from source. | + +**The two "required" verdicts have an exception, and it matters.** Thunder's own +refusal names it: *no default resource server is configured*. A resource server +carries a **Set as default** action, whose confirmation states that requests +without a resource parameter will fall back to it. Measured on the same +deployment: with the default set, a client-credentials grant carrying no +indicator succeeded, and the issued token's `aud` was the default's identifier. +The same request had been refused minutes earlier. + +So the indicator is required *unless* a default resource server is configured. +Both halves belong here, because a reader who has set a default and one who has +not would otherwise measure different things and each conclude the other was +wrong. The walkthrough tells a reader not to set one, and why: a default binds +every token to the same audience whichever product asked, which is exactly the +Asgardeo weakness recorded in §3 and issue #37, reintroduced on the one product +whose audience model can avoid it. + +**What this changed in the shell.** Unlike §3 and §3.1, this measurement forced +production code. The scoped refresh the broker implements cannot establish a +session on a Thunder deployment at all, because the refusal happens at +authorization, before any session exists. An identity may now name its identity +provider, and the shell sends the indicator where it does — on the authorization +request, and on the client-credentials grant. The refresh grant was left alone, +because the binding is inherited. + +**A consequence worth stating separately.** One resource indicator per +authorization means one Thunder session reaches one product. The context schema +refuses an identity that derives this way and declares more than one product, +rather than letting the contradiction surface at the end of a browser sign-in. +Lifting that needs per-product sessions, which is +[its own issue](https://github.com/wso2/wso2-cli/issues/43) and was already +recorded as a required gap in +[product-authentication-compatibility.md](product-authentication-compatibility.md) +§1.5. + +**One question this document asks of the other products is not answered here.** +Any-port loopback was not measured against Thunder. The walkthrough registers +all four callback ports explicitly, as it does for the other two, so nothing in +the shell depends on the answer; it is simply not evidence anyone has gathered. + +**And one incidental finding.** A resource server's permissions cannot contain +its own delimiter, which is `:` by default: a handle of `reference:status:read` +is refused with `Delimiter conflict in handle`. Hierarchical permissions are +built as a tree of resources whose handles Thunder joins with the delimiter. The +reference module's permission names are therefore expressible, but not as flat +strings the way they are on the other two products. + ## 4. Producing and recording the verdicts **Added 2026-08-05.** The experiments described in §1.2 and §2 are implemented From 65b7b71ca9ef5183ab0600ea253d554cfe8a8348 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 20:15:39 +0530 Subject: [PATCH 5/8] test(smoke): add the Thunder experiments and the first live CI run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The experiments are the ported half of the spike: the throwaway that answered the design question is gone, and what remains is the part that backs a recorded verdict. They ask Thunder the two questions sections 3 and 3.1 ask of Asgardeo and Identity Server, so its column is comparable rather than a separate story, plus the one those products never raise — whether the audience can be chosen at all. They refuse to run against a deployment that is not a Thunder one, because a resource experiment against a product that takes no resource indicator would print a verdict about nothing. The CI run is the first live coverage of the non-interactive path against any deployment. Until now that source was proven only against the in-process fake issuer, on every product, so the guarantee it makes to a module rested on a fixture. It asks for one permission out of the configured set rather than for all of them, because a run that asks for everything cannot tell narrowing from a deployment that ignored the request — the defect #35 fixed in the browser run, written out of this one from the start. It needs no browser and no human, so it can run unattended, and it is the one live target that can. Its client secret comes from the environment and from no file, and the document it installs holds a secret's variable and no secure-store reference, which is what the schema requires of a kind that never logs in. That document is built in the untagged half of the package so this package's own tests prove the shell will read it before a live run depends on it. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- Makefile | 22 ++- internal/auth/oauthflow/resource_test.go | 2 +- test/smoke/RUNNING.md | 27 +++- test/smoke/ci_smoke_test.go | 97 ++++++++++++ test/smoke/config.go | 44 ++++++ test/smoke/thunder_config_test.go | 56 +++++++ test/smoke/thunder_empirical_test.go | 190 +++++++++++++++++++++++ 7 files changed, 435 insertions(+), 3 deletions(-) create mode 100644 test/smoke/ci_smoke_test.go create mode 100644 test/smoke/thunder_empirical_test.go diff --git a/Makefile b/Makefile index 2da0c21..46e63f3 100644 --- a/Makefile +++ b/Makefile @@ -83,9 +83,11 @@ help: @echo ' make acceptance Run the full architecture-proof acceptance gate.' @echo ' make smoke-build Compile the live runs without executing them.' @echo '' - @echo 'Against a real deployment (Asgardeo or a local Identity Server 7.x):' + @echo 'Against a real deployment (Asgardeo, Identity Server 7.x, or ThunderID):' @echo ' make smoke-login Log in and broker one acquisition. Opens a browser.' + @echo ' make smoke-ci Broker one acquisition the way CI does. No browser.' @echo ' make empirical-asgardeo Run the two one-time experiments and print their verdicts.' + @echo ' make empirical-thunder The same questions against a Thunder deployment.' @echo '' @echo 'Both live targets skip cleanly when no deployment is configured.' @echo 'They read $(SMOKE_ENV) when it exists; name another with' @@ -134,3 +136,21 @@ smoke-login: empirical-asgardeo: @$(smoke_env) WSO2_EMPIRICAL=1 \ $(GO) test $(SMOKE_FLAGS) $(SMOKE_PACKAGE) -run TestAsgardeoEmpirical + +# Answers the questions that decided how the shell derives access on a +# deployment which binds tokens to a named resource, and prints one verdict line +# each for recording. Skips unless the configured deployment says it is a +# Thunder one, because the experiments are meaningless against a product that +# takes no resource indicator. +.PHONY: empirical-thunder +empirical-thunder: + @$(smoke_env) WSO2_EMPIRICAL=1 \ + $(GO) test $(SMOKE_FLAGS) $(SMOKE_PACKAGE) -run TestThunderEmpirical + +# Brokers one acquisition the way a CI job does: inline, from a client secret +# already in this shell, with no login and no browser. Needs no human, so unlike +# smoke-login it can run unattended. The secret comes from the environment and +# from no file; RUNNING.md says which variable. +.PHONY: smoke-ci +smoke-ci: + @$(smoke_env) $(GO) test $(SMOKE_FLAGS) $(SMOKE_PACKAGE) -run TestCISmoke diff --git a/internal/auth/oauthflow/resource_test.go b/internal/auth/oauthflow/resource_test.go index 2739dce..c521882 100644 --- a/internal/auth/oauthflow/resource_test.go +++ b/internal/auth/oauthflow/resource_test.go @@ -76,7 +76,7 @@ func TestALoginWithoutAResourceIsRefusedByADeploymentThatRequiresOne(t *testing. if err == nil { t.Fatal("a login carrying no resource indicator completed against a deployment that requires one") } - requireProblem(t, err, "auth.credential_unavailable") + _ = requireProblem(t, err, "auth.credential_unavailable") } // A deployment that binds no audience at authorization time must be unaffected, diff --git a/test/smoke/RUNNING.md b/test/smoke/RUNNING.md index 663fad8..9de4a1c 100644 --- a/test/smoke/RUNNING.md +++ b/test/smoke/RUNNING.md @@ -61,7 +61,32 @@ worth avoiding on purpose. | `WSO2_SMOKE_IDENTITY_TYPE` | no | `cloud` (default) or `onprem`. | | `WSO2_SMOKE_UNREGISTERED_PORT` | no | The loopback port the any-port experiment binds. Defaults to `16000`. Must be outside 10425-10428. | | `WSO2_SMOKE_DEADLINE` | no | How long an **experiment** waits at the browser. Defaults to `3m`. It does not reach `make smoke-login`, which signs in through `wso2 login` and carries the shell's own five-minute deadline. | -| `WSO2_EMPIRICAL` | experiments only | Set to `1` to opt into the experiments. `make empirical-asgardeo` sets it for you. | +| `WSO2_SMOKE_PROVIDER` | no | The identity provider behind the issuer: `asgardeo`, `identity-server`, or `thunder`. Left unset, the run describes a deployment that binds audiences from the application's registration, which is the first two. **Required for ThunderID**, which binds them per request and refuses a login that names no resource. A name this shell does not read fails rather than skipping. | +| `WSO2_SMOKE_CI_CLIENT_ID` | CI run only | The confidential client `make smoke-ci` presents. Its **secret** is not named here; see below. | +| `WSO2_EMPIRICAL` | experiments only | Set to `1` to opt into the experiments. `make empirical-asgardeo` and `make empirical-thunder` set it for you. | + +### The client secret, and why it is not in the table above + +`make smoke-ci` authenticates as a confidential client, so it needs that +client's secret. Export it in the shell you run from: + +```sh +export WSO2_SMOKE_CLIENT_SECRET='' +make smoke-ci +``` + +The variable's name is fixed in `config.go`, beside the secure-store reference +that is already fixed there, and it is deliberately absent from +`test/smoke/env.example`. A deployment description is a file people copy, keep, +and send to each other; naming the secret's variable in one is an invitation to +paste the value beside the name, and `*.env` being git-ignored is a weaker +protection than the value never being written down at all. + +This is also the production contract, exercised as shipped: a context names an +environment variable, the broker reads it into process memory for one grant, and +it never reaches shell state, the secure store, or the module's environment. + +Without it, `make smoke-ci` skips and says so. With none of them set, both targets skip and say which variables they wanted. That is the expected result on a machine with no deployment: diff --git a/test/smoke/ci_smoke_test.go b/test/smoke/ci_smoke_test.go new file mode 100644 index 0000000..cdd5789 --- /dev/null +++ b/test/smoke/ci_smoke_test.go @@ -0,0 +1,97 @@ +// 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. + +//go:build smoke + +package smoke_test + +import ( + "os" + "strings" + "testing" + "time" + + "github.com/wso2/wso2-cli/internal/auth" + "github.com/wso2/wso2-cli/test/smoke" +) + +// TestCISmoke brokers one acquisition the way a CI job does: inline, from a +// client secret already on the machine, with no login and no browser. +// +// It is the first live coverage of that path against any deployment. Until now +// the non-interactive source was proven only against the in-process fake +// issuer, on every product — so the guarantee it makes to a module rested on a +// fixture rather than on a deployment. The guarantee is the same one the +// browser path makes, and the reason it is worth proving live is that a module +// cannot tell which kind of context invoked it and must not need to. +// +// It needs no browser and no human, so unlike TestLoginSmoke it can run +// unattended. What it does need is a confidential client and its secret, and +// the secret comes from the environment rather than from any file — see +// RUNNING.md. +func TestCISmoke(t *testing.T) { + config := requireDeployment(t) + if config.CIClientID == "" { + t.Skipf("no confidential client is configured: set %s (see test/smoke/RUNNING.md)", + smoke.CIClientIDVar) + } + if strings.TrimSpace(os.Getenv(smoke.SecretVariable)) == "" { + t.Skipf("no client secret is exported: set %s in this shell "+ + "(see test/smoke/RUNNING.md; it deliberately belongs in no file)", + smoke.SecretVariable) + } + + selection, err := config.CIDocument().Select("") + if err != nil { + t.Fatalf("the non-interactive document selects no context: %v", err) + } + broker := &auth.Broker{ + Namespace: smoke.Namespace, + Capabilities: config.Capabilities(), + Selection: selection, + InvocationID: "smoke-ci", + StateRoot: t.TempDir(), + } + + target, err := config.NarrowTarget() + if err != nil { + t.Skipf("%v", err) + } + // One permission out of the configured set, so the run proves narrowing + // rather than proving that asking for everything returns everything. #35 + // fixed exactly that defect in the browser run; this one is written not to + // have it. + request := auth.Request{Audience: config.Audience, Scopes: []string{target}} + + grant, err := broker.Acquire(request) + if err != nil { + t.Fatalf("the broker refused a CI acquisition: %v", err) + } + if grant.Token == "" { + t.Fatal("the broker granted an empty token") + } + if !grant.ExpiresAt.After(time.Now().UTC()) { + t.Fatalf("the grant expires at %s, which is not in the future", grant.ExpiresAt) + } + + // The broker proved the narrowing before returning: it verifies that the + // issued token carries exactly the permissions asked for and is bound to + // the audience asked for, and refuses when it cannot. Reaching here is the + // assertion, and decoding the token again would only restate it somewhere + // that could drift from the check that matters. + t.Logf("CI acquisition granted for %q against %q, expiring %s", + target, config.Audience, grant.ExpiresAt.Format(time.RFC3339)) +} diff --git a/test/smoke/config.go b/test/smoke/config.go index 93481fa..5a2afbc 100644 --- a/test/smoke/config.go +++ b/test/smoke/config.go @@ -337,6 +337,50 @@ func (c Config) Document() contexts.Document { } } +// CIDocument is the schema version 2 document the non-interactive run installs. +// +// It is a second document rather than a second identity inside the first, +// because the two runs must not be able to reach each other's material. The +// interactive identity holds a secure-store reference and no secret; this one +// holds a secret's variable and no reference, which is what the schema requires +// of a kind that never logs in. Building it here, rather than in the tagged +// half of the package, is what lets this package's own tests prove the shell +// will read it before a live run depends on it. +func (c Config) CIDocument() contexts.Document { + return contexts.Document{ + SchemaVersion: contexts.SchemaVersion, + DefaultContext: CIContextName, + Identities: []contexts.Identity{{ + Name: CIIdentityName, + Type: c.IdentityType, + Auth: contexts.IdentityAuth{ + Kind: contexts.KindClientCredentials, + Issuer: c.Issuer, + ClientID: c.CIClientID, + Tenant: c.Tenant, + // The deployment is the same deployment, so it derives the same + // way. A confidential client on a deployment that binds by + // resource needs the indicator too, and has no earlier + // authorization to inherit one from. + Provider: c.Provider, + ClientSecretVariable: SecretVariable, + }, + Products: map[string]contexts.Product{ + Namespace: { + Endpoint: c.Endpoint, + Audience: c.Audience, + Scopes: slices.Clone(c.Scopes), + }, + }, + }}, + Contexts: []contexts.Context{{ + Name: CIContextName, + Identity: CIIdentityName, + Organization: c.Tenant, + }}, + } +} + // Capabilities are the receipt a module installed for this run would carry. // // The broker checks a request against the receipt before it checks anything diff --git a/test/smoke/thunder_config_test.go b/test/smoke/thunder_config_test.go index fe5a8a2..55d2df7 100644 --- a/test/smoke/thunder_config_test.go +++ b/test/smoke/thunder_config_test.go @@ -83,6 +83,62 @@ func TestAnUnknownProviderIsRefusedBeforeAnyRun(t *testing.T) { } } +// The non-interactive run installs its own document, because the identity it +// authenticates as is a different one: a confidential client with a secret, +// holding no secure-store reference and never logging in. +func TestTheNonInteractiveDocumentIsReadableByTheShell(t *testing.T) { + values := thunderEnvironment() + values[smoke.CIClientIDVar] = "wso2-cli-ci" + config, err := smoke.Load(environment(values)) + if err != nil { + t.Fatalf("load: %v", err) + } + document := config.CIDocument() + encoded, err := document.Encode() + if err != nil { + t.Fatalf("encode: %v", err) + } + if _, err := contexts.Decode(encoded); err != nil { + t.Fatalf("the shell refused the document a CI run installs: %v", err) + } + + identity := document.Identities[0] + if identity.Auth.Kind != contexts.KindClientCredentials { + t.Fatalf("the CI identity is of kind %q, want %q", + identity.Auth.Kind, contexts.KindClientCredentials) + } + if identity.Auth.ClientID != "wso2-cli-ci" { + t.Fatalf("the CI identity presents %q, want the configured confidential client", + identity.Auth.ClientID) + } + if identity.Auth.ClientSecretVariable != smoke.SecretVariable { + t.Fatalf("the CI identity reads its secret from %q, want %q", + identity.Auth.ClientSecretVariable, smoke.SecretVariable) + } + // A non-interactive identity never logs in, so it must hold no reference to + // a stored session. The schema refuses one; so must the document this run + // builds, or a live run fails on a defect this package could have caught. + if identity.Auth.CredentialRef != "" { + t.Fatalf("the CI identity names a secure-store reference %q; it never logs in", + identity.Auth.CredentialRef) + } +} + +// The CI document derives the same way the interactive one does, because the +// deployment is the same deployment. A confidential client on Thunder needs the +// resource indicator too — it has no earlier authorization to inherit one from. +func TestTheNonInteractiveDocumentDerivesLikeTheDeployment(t *testing.T) { + values := thunderEnvironment() + values[smoke.CIClientIDVar] = "wso2-cli-ci" + config, err := smoke.Load(environment(values)) + if err != nil { + t.Fatalf("load: %v", err) + } + if got := config.CIDocument().Identities[0].Auth.Derivation(); got != contexts.DerivationTokenResource { + t.Fatalf("the CI identity derives by %q, want %q", got, contexts.DerivationTokenResource) + } +} + // A deployment description names no secret and no secret's variable. The // non-interactive run needs one, so the name it reads is fixed here instead, // where nobody is invited to paste a value beside it. diff --git a/test/smoke/thunder_empirical_test.go b/test/smoke/thunder_empirical_test.go new file mode 100644 index 0000000..ed0cd8e --- /dev/null +++ b/test/smoke/thunder_empirical_test.go @@ -0,0 +1,190 @@ +// 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. + +//go:build smoke + +package smoke_test + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/wso2/wso2-cli/internal/auth" + "github.com/wso2/wso2-cli/internal/auth/oauthflow" + "github.com/wso2/wso2-cli/internal/auth/session" + "github.com/wso2/wso2-cli/internal/contexts" + "github.com/wso2/wso2-cli/test/smoke" +) + +// TestThunderEmpirical answers the questions that decided how the shell derives +// access on a deployment which binds tokens to a named resource. +// +// They are the same two questions sections 3 and 3.1 of the research document +// ask of Asgardeo and Identity Server — does the refresh grant honour a +// narrower scope, and what lands in aud — plus the one those products never +// raise, which is whether the audience can be chosen at all. Keeping the first +// two makes Thunder's column comparable with the other two rather than a +// separate story. +// +// Each experiment prints one greppable verdict line for a human to copy into +// section 3.2. The test passes whatever the deployment answers: a refusal is a +// finding, and a run that failed on one would be reporting the deployment's +// behaviour as the shell's defect. +// +// These run against a deployment the Thunder walkthrough describes. Against +// Asgardeo or Identity Server the resource experiments are meaningless — those +// products take no resource indicator — so the run refuses to start unless the +// deployment says it is a Thunder one. +func TestThunderEmpirical(t *testing.T) { + config := requireDeployment(t) + if !smoke.Empirical(os.LookupEnv) { + t.Skipf("set %s=1 to run the one-time empirical experiments "+ + "(see test/smoke/RUNNING.md)", smoke.EmpiricalVar) + } + if config.Provider != contexts.ProviderThunder { + t.Skipf("these experiments describe a deployment that binds access to a named resource; "+ + "set %s=%s to run them", smoke.ProviderVar, contexts.ProviderThunder) + } + + t.Setenv("WSO2_CONTEXT", "") + t.Setenv("WSO2_NON_INTERACTIVE", "") + + t.Run("indicator-required", func(t *testing.T) { experimentIndicatorRequired(t, config) }) + t.Run("resource-narrowing", func(t *testing.T) { experimentResourceNarrowing(t, config) }) +} + +// experimentIndicatorRequired asks whether the deployment will establish a +// session at all without being told which protected resource it is for. +// +// This is the question that decided the derivation. If the answer is that it +// will, the shell's existing scoped refresh would have served Thunder unchanged +// and this slice would have been documentation. It will not, unless a default +// resource server has been configured — which is why the verdict distinguishes +// the two rather than reporting a bare yes or no. +func experimentIndicatorRequired(t *testing.T, config smoke.Config) { + t.Logf("signing in for %v WITHOUT a resource indicator", config.Scopes) + ctx, cancel := context.WithTimeout(context.Background(), config.Deadline) + defer cancel() + + result, err := oauthflow.Login{ + Issuer: config.Issuer, + ClientID: config.ClientID, + Scopes: config.Scopes, + Out: os.Stderr, + // Deliberately no Resource. This is the experiment. + }.Run(ctx) + + var verdict string + switch { + case err != nil: + verdict = "required (the deployment refused a login carrying no indicator)" + t.Logf("the login ended with %s: %s", refusalCode(err), refusalMessage(err)) + case result.Token.RefreshToken == "": + verdict = "inconclusive (the login completed but issued no refresh token)" + default: + verdict = "optional (a default resource server is configured; every token it " + + "issues carries that default's audience whichever product asked)" + } + reportThunder(t, "THUNDER RESOURCE INDICATOR AT AUTHORIZATION", verdict, config) +} + +// experimentResourceNarrowing asks the two questions the other products are +// asked, on a session established the way this product requires. +// +// It signs in for every configured permission, naming the product's resource, +// stores the session exactly as `wso2 login` would, and then asks the broker +// for one permission out of the set. The broker's own verification decides the +// verdict: it proves the issued token carries exactly the permissions asked for +// and is bound to the audience asked for, and refuses when it cannot. So a +// grant here is a statement about both narrowing and audience binding at once. +func experimentResourceNarrowing(t *testing.T, config smoke.Config) { + target, err := config.NarrowTarget() + if err != nil { + t.Skipf("%v", err) + } + + stateRoot := filepath.Join(t.TempDir(), "state") + forgetSmokeSession(t) + + t.Logf("signing in for %v against %q, then asking for %q alone", + config.Scopes, config.Audience, target) + ctx, cancel := context.WithTimeout(context.Background(), config.Deadline) + defer cancel() + + result, err := oauthflow.Login{ + Issuer: config.Issuer, + ClientID: config.ClientID, + Scopes: config.Scopes, + Resource: config.Audience, + Out: os.Stderr, + }.Run(ctx) + if err != nil { + t.Fatalf("the experiment could not establish a session to narrow: %v", err) + } + if result.Token.RefreshToken == "" { + t.Fatalf("the deployment issued no refresh token, so there is no session to narrow; " + + "check that the application is allowed the refresh token grant") + } + + store := session.Store{StateRoot: stateRoot} + err = store.WithLock(smoke.CredentialRef, func() error { + return store.Save(smoke.CredentialRef, session.Session{ + Issuer: config.Issuer, + RefreshToken: result.Token.RefreshToken, + AccessToken: result.Token.AccessToken, + ExpiresAt: result.Token.Expiry.UTC(), + }) + }) + if err != nil { + t.Fatalf("the experiment could not store the session it just established: %v", err) + } + + selection, selectErr := config.Document().Select("") + if selectErr != nil { + t.Fatalf("the smoke document selects no context: %v", selectErr) + } + broker := &auth.Broker{ + Namespace: smoke.Namespace, + Capabilities: config.Capabilities(), + Selection: selection, + InvocationID: "smoke-empirical", + StateRoot: stateRoot, + } + + _, err = broker.Acquire(auth.Request{Audience: config.Audience, Scopes: []string{target}}) + verdict := smoke.NarrowingVerdict(refusalCode(err), refusalMessage(err), []string{target}) + if err != nil { + t.Logf("the broker reported %s: %s", refusalCode(err), refusalMessage(err)) + } + reportThunder(t, "THUNDER REFRESH NARROWING AND AUDIENCE BINDING", verdict, config) +} + +// reportThunder prints one verdict line, naming the section it belongs in. +// +// It is separate from the Asgardeo experiments' reporter for one reason: the +// section a verdict is recorded in is part of the verdict. A Thunder finding +// pasted into the Asgardeo table would be indistinguishable from a measurement +// of the wrong deployment. +func reportThunder(t *testing.T, question, verdict string, config smoke.Config) { + t.Helper() + _, _ = fmt.Fprintf(os.Stdout, "\n%s: %s\n deployment: %s\n recorded in: %s\n\n", + question, verdict, config.Issuer, + "docs/research/asgardeo-redirect-uri-and-scope-narrowing.md section 3.2") + t.Logf("%s: %s", question, verdict) +} From b0827415126dcf2ea741d64d808d48a94905433a Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Thu, 6 Aug 2026 20:25:26 +0530 Subject: [PATCH 6/8] fix(smoke): do not report an unreachable deployment as a finding The resource-indicator experiment classified every error as the deployment refusing a login that named no resource. A deployment the shell cannot reach at all produces an error too, so an unreachable host was recorded as the strongest possible finding about a question that was never asked. This is not hypothetical. The first run of this experiment against a rebuilt container reported "required" when what had actually happened was that the container had regenerated its certificate and discovery failed. The verdict would have gone into the research document as evidence. auth.discovery_failed now reads as inconclusive and says so, matching how the Asgardeo experiments already treat the same code. A verdict that cannot tell a refusal from an unreachable host is not evidence, and the whole reason these experiments are committed rather than run by hand is that a recorded finding has to be reproducible by someone who was not there. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- test/smoke/thunder_empirical_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/smoke/thunder_empirical_test.go b/test/smoke/thunder_empirical_test.go index ed0cd8e..c71d475 100644 --- a/test/smoke/thunder_empirical_test.go +++ b/test/smoke/thunder_empirical_test.go @@ -92,6 +92,14 @@ func experimentIndicatorRequired(t *testing.T, config smoke.Config) { var verdict string switch { + case refusalCode(err) == codeDiscoveryFailed: + // The shell never reached the deployment, so it never asked the + // question. Reporting that as "required" would record the strongest + // possible finding on the strength of an unreachable host — which is + // exactly what this experiment did the first time it was run against a + // deployment whose certificate had been regenerated. + verdict = "inconclusive (" + codeDiscoveryFailed + "; the shell could not reach the deployment)" + t.Logf("the login ended with %s: %s", refusalCode(err), refusalMessage(err)) case err != nil: verdict = "required (the deployment refused a login carrying no indicator)" t.Logf("the login ended with %s: %s", refusalCode(err), refusalMessage(err)) From cc5b77a4683e705828911238cb83fa71d444e1e3 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Fri, 7 Aug 2026 10:11:19 +0530 Subject: [PATCH 7/8] fix(contexts): refuse a resource-bound audience that is not an absolute URI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that a resource-bound identity was validated only for a non-empty audience, so a document naming the bare identifier the other two products use loaded successfully and then failed at the deployment with invalid_target. That is the failure this validation exists to prevent: the walkthrough already warns that a bare name here costs a browser sign-in to discover, and the schema was letting it through anyway. The rule is RFC 8707 section 2's — an absolute URI carrying no fragment — and stops there. Requiring a particular scheme, or a host, would refuse identifiers the specification permits; a URN names a resource server perfectly well, and this shell never dereferences the value. What the check cannot claim is written down beside it. "localhost:8490/x" is an absolute URI whose scheme happens to be "localhost", almost certainly a missing "https://", and it is accepted — refusing it would mean holding an opinion about plausibility that nothing here is entitled to hold. It fails at the deployment, which knows which resource servers it registered. The provider list is now exported and read by the live-run harness rather than copied into it. A copy would drift the moment a product is added: the harness would refuse a provider the shell accepts, and report a correct description as a deployment that could not be run. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- internal/app/login_resource_test.go | 14 ++++- internal/contexts/derivation_test.go | 93 +++++++++++++++++++++++++++- internal/contexts/identity.go | 31 +++++++++- test/smoke/config.go | 16 +++-- 4 files changed, 138 insertions(+), 16 deletions(-) diff --git a/internal/app/login_resource_test.go b/internal/app/login_resource_test.go index 20f0820..f3214c2 100644 --- a/internal/app/login_resource_test.go +++ b/internal/app/login_resource_test.go @@ -29,11 +29,19 @@ import ( "github.com/zalando/go-keyring" ) -// thunderDoc is browserDoc against a deployment that decides the audience at -// authorization time. +// theResource is the protected resource a deployment that decides the audience +// at authorization time mints access for. It is an absolute URI because RFC 8707 +// requires the indicator to be one, and because the context schema now refuses +// anything else on an identity that derives this way. +const theResource = "https://deployment.example.test/reference-status" + +// thunderDoc is browserDoc against such a deployment. func thunderDoc(issuerURL string) contexts.Document { document := browserDoc(issuerURL) document.Identities[0].Auth.Provider = contexts.ProviderThunder + product := document.Identities[0].Products["reference"] + product.Audience = theResource + document.Identities[0].Products["reference"] = product return document } @@ -66,7 +74,7 @@ func TestLoginBindsTheSessionToTheResourceTheProductNames(t *testing.T) { if stored.RefreshToken == "" { t.Fatal("the stored session holds no refresh token") } - if !strings.Contains(errOut.String(), "resource="+url.QueryEscape("reference-status")) { + if !strings.Contains(errOut.String(), "resource="+url.QueryEscape(theResource)) { t.Fatalf("the authorization URL carried no resource indicator:\n%s", errOut) } } diff --git a/internal/contexts/derivation_test.go b/internal/contexts/derivation_test.go index 539c8f1..5f3643d 100644 --- a/internal/contexts/derivation_test.go +++ b/internal/contexts/derivation_test.go @@ -17,6 +17,7 @@ package contexts_test import ( + "slices" "strings" "testing" @@ -117,16 +118,102 @@ func TestSeveralProductsStayLegalWithoutThunder(t *testing.T) { // Deriving access bound to a resource means naming the resource, and a product // with no audience names none. func TestAThunderIdentityWhoseProductNamesNoAudienceIsRefused(t *testing.T) { - document := withProvider(contexts.ProviderThunder) - document = strings.Replace(document, `"audience": "reference-status",`, ``, 1) + document := withAudience(contexts.ProviderThunder, "") + document = strings.Replace(document, `"audience": "",`, ``, 1) _, err := contexts.Decode([]byte(document)) assertProblemCode(t, err, "contexts.document_malformed") } +// A resource indicator is an absolute URI by RFC 8707 section 2, so a document +// that derives by resource and names a bare identifier describes a request the +// deployment will refuse. Catching it here is the difference between a document +// that cannot be loaded and a browser sign-in that ends in invalid_target. +func TestAResourceBoundAudienceMustBeAnAbsoluteURI(t *testing.T) { + for name, audience := range map[string]string{ + "a bare identifier": "reference-status", + "a path": "/reference-status", + } { + t.Run(name, func(t *testing.T) { + _, err := contexts.Decode([]byte(withAudience(contexts.ProviderThunder, audience))) + assertProblemCode(t, err, "contexts.document_malformed") + }) + } +} + +// A fragment is the one thing RFC 8707 rules out beyond absoluteness, and it is +// worth refusing for the same reason: the deployment will. +func TestAResourceBoundAudienceMayNotCarryAFragment(t *testing.T) { + _, err := contexts.Decode([]byte(withAudience(contexts.ProviderThunder, + "https://localhost:8490/reference-status#read"))) + assertProblemCode(t, err, "contexts.document_malformed") +} + +// The rule is RFC 8707's, not a guess about which schemes a deployment serves. +// A URN is an absolute URI and is refused by nothing in that specification, so +// this shell does not refuse it either. +func TestAResourceBoundAudienceAcceptsAnyAbsoluteURI(t *testing.T) { + // The last case is the limit of what this check can claim. "localhost:8490/x" + // is an absolute URI whose scheme happens to be "localhost", almost + // certainly a missing "https://" — but the specification accepts it, this + // shell never dereferences the value, and refusing it would mean inventing a + // rule about plausibility that nothing here is entitled to hold. It fails at + // the deployment, which knows which resource servers it registered. + for name, audience := range map[string]string{ + "an https URL": "https://localhost:8490/reference-status", + "a urn": "urn:wso2:reference-status", + "a scheme that is likely a typo": "localhost:8490/reference-status", + } { + t.Run(name, func(t *testing.T) { + if _, err := contexts.Decode([]byte(withAudience(contexts.ProviderThunder, audience))); err != nil { + t.Fatalf("an absolute URI audience was refused: %v", err) + } + }) + } +} + +// The rule belongs to the derivation, not to the schema. A deployment that +// binds audiences from the application's registration takes whatever identifier +// it registered, and Identity Server's is a bare name. +func TestABareAudienceStaysLegalWithoutAResourceBoundDerivation(t *testing.T) { + if _, err := contexts.Decode([]byte(validV2())); err != nil { + t.Fatalf("a bare audience was refused on a scoped-refresh identity: %v", err) + } + if _, err := contexts.Decode([]byte(withAudience(contexts.ProviderIdentityServer, "reference-status"))); err != nil { + t.Fatalf("a bare audience was refused on an Identity Server identity: %v", err) + } +} + +// Every provider this shell reads is one the smoke harness may describe, so the +// list has one home rather than a copy per caller. +func TestEveryReadableProviderIsNamed(t *testing.T) { + named := contexts.Providers() + for _, provider := range []string{ + contexts.ProviderAsgardeo, contexts.ProviderIdentityServer, contexts.ProviderThunder, + } { + if !slices.Contains(named, provider) { + t.Fatalf("Providers() omits %q: %v", provider, named) + } + } + if !slices.IsSorted(named) { + t.Fatalf("Providers() is unsorted, so a refusal naming them would vary between runs: %v", named) + } +} + +// withProvider names the identity provider, and gives the product an audience +// that provider would accept. Thunder derives by resource and therefore needs an +// absolute URI; using one for every provider keeps the helper honest without +// making the caller think about it. func withProvider(provider string) string { - return strings.Replace(validV2(), + return withAudience(provider, "https://issuer.example.test/reference-status") +} + +func withAudience(provider, audience string) string { + document := strings.Replace(validV2(), `"kind": "oauth-browser",`, `"kind": "oauth-browser", "provider": "`+provider+`",`, 1) + return strings.Replace(document, + `"audience": "reference-status",`, + `"audience": "`+audience+`",`, 1) } func withDerivation(provider, derivation string) string { diff --git a/internal/contexts/identity.go b/internal/contexts/identity.go index 09c8d11..190f672 100644 --- a/internal/contexts/identity.go +++ b/internal/contexts/identity.go @@ -93,6 +93,16 @@ var legalDerivations = map[string]bool{ DerivationScopedRefresh: true, DerivationTokenResource: true, } +// Providers are the identity providers a document may name, in a stable order. +// +// It is exported because the list has readers outside this package — the live +// runs describe a deployment before building a document from it, and a harness +// that accepted a name the shell then refused would report a configuration +// mistake as a failed deployment. One list, one place to add the next product. +func Providers() []string { + return slices.Sorted(maps.Keys(providerDerivation)) +} + // refPattern constrains a credential reference to one readable word, exactly // as context names are constrained. A credential value pasted where a // reference belongs — a JWT, anything with dots, equals signs, or upper-case @@ -227,11 +237,30 @@ func (i Identity) validateDerivation() error { "and gives it %d products", i.Name, len(i.Products))) } for _, namespace := range slices.Sorted(maps.Keys(i.Products)) { - if i.Products[namespace].Audience == "" { + audience := i.Products[namespace].Audience + if audience == "" { return malformed(fmt.Sprintf( "declares the %q product on the identity %q without the audience its deployment "+ "binds access to", namespace, i.Name)) } + // The audience travels as an RFC 8707 resource indicator, which section + // 2 of that specification requires to be an absolute URI carrying no + // fragment. A bare identifier is the shape the other two products use + // and is accepted by neither the specification nor a deployment reading + // it, so it is refused here rather than at the end of a browser sign-in + // that ends in invalid_target. + // + // The rule is the specification's and stops there. Requiring a + // particular scheme, or a host, would refuse identifiers RFC 8707 + // permits — a URN names a resource server perfectly well — and this + // shell never dereferences the value, so it has no reason to hold an + // opinion the specification does not. + parsed, err := url.Parse(audience) + if err != nil || parsed.Scheme == "" || parsed.Fragment != "" { + return malformed(fmt.Sprintf( + "declares the %q product on the identity %q with an audience that is not an "+ + "absolute URI, which is what its deployment binds access by", namespace, i.Name)) + } } return nil } diff --git a/test/smoke/config.go b/test/smoke/config.go index 5a2afbc..5ec7b71 100644 --- a/test/smoke/config.go +++ b/test/smoke/config.go @@ -284,15 +284,13 @@ func Empirical(lookup func(string) (string, bool)) bool { func RegisteredPorts() []int { return []int{10425, 10426, 10427, 10428} } // readableProviders are the identity providers a deployment description may -// name. It is the shell's own list, so a run cannot describe a deployment the -// context document would then refuse. -func readableProviders() []string { - return []string{ - contexts.ProviderAsgardeo, - contexts.ProviderIdentityServer, - contexts.ProviderThunder, - } -} +// name. +// +// It defers to the shell's own list rather than repeating it. A copy here would +// drift the moment a product is added: this package would refuse a provider the +// shell accepts, and report a description that was correct as a deployment that +// could not be run. +func readableProviders() []string { return contexts.Providers() } // Document is the schema version 2 context document a live run installs. // From 1567098a694d030a2184a6ba1b2b84cd9772b8a4 Mon Sep 17 00:00:00 2001 From: Kanushka Gayan Date: Fri, 7 Aug 2026 10:36:32 +0530 Subject: [PATCH 8/8] fix(auth): keep a session's resource binding, and tell the two invalid_targets apart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from review, all valid. The fake issuer bound only the initial access token to its resource. Refresh state carried the granted permissions and not the resource, so a renewal fell back to the registration's audience — and the whole reason the refresh grant was left untouched is that a deployment which binds by resource carries the binding forward. The fixture disagreed with the deployment on exactly the point the design rests on, and no test could have noticed: the audience it fell back to happened to equal the resource in every fixture. Giving the registration a different audience is what makes the new test able to fail at all. invalid_target covered two opposite causes and always reported one of them. A request that named no resource and a request that named one the deployment does not know arrive as the same OAuth error, and the shell told both to go and name an identity provider — advice the second has already followed. Which happened is a fact about the request, so the classification says only that the target was rejected and the caller, which made the request, decides what to say. A resource-bound identity with no products passed validation, and login then sent no indicator. The rule is exactly one product, not at most one: an identity with none has nothing to bind to. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod --- docs/guides/login-thunder.md | 11 ++++- internal/auth/fakeissuer/fakeissuer.go | 65 ++++++++++++++++++++----- internal/auth/narrowing.go | 17 ++++++- internal/auth/resource_test.go | 66 ++++++++++++++++++++++++++ internal/auth/source_browser_test.go | 7 +++ internal/auth/source_clientcred.go | 16 +++++-- internal/auth/tokenrequest.go | 15 +++--- internal/contexts/derivation_test.go | 26 ++++++++++ internal/contexts/identity.go | 8 +++- 9 files changed, 203 insertions(+), 28 deletions(-) diff --git a/docs/guides/login-thunder.md b/docs/guides/login-thunder.md index 612c0c3..0b8df49 100644 --- a/docs/guides/login-thunder.md +++ b/docs/guides/login-thunder.md @@ -89,6 +89,13 @@ docker run -d --name thunderid -p 8490:8090 \ && ./setup.sh --admin-username admin --admin-password "Admin@123" && ./start.sh' ``` +**Everything below this point writes `8090`.** If you took the offset recipe, +substitute the host port you chose — in the discovery URL, the console URL, the +certificate you trust, the resource server's identifier, and the issuer and +audience you write into the context document. The port is part of the issuer's +identity here, not a detail of how you reach it, so a value that is close but +not exact fails at discovery. + Confirm what it advertises rather than assuming it: ```sh @@ -112,7 +119,7 @@ self-signed certificate. The shell uses the process's ordinary HTTP client and has no flag anywhere for a custom certificate authority, so until that certificate is trusted, login cannot reach discovery at all: -``` +```text tls: failed to verify certificate: x509: certificate signed by unknown authority ``` @@ -207,7 +214,7 @@ does not need the fallback. 3. On the **General** tab, under **Authorized redirect URIs**, add all four loopback callbacks with **Add URI**: - ``` + ```text http://127.0.0.1:10425/callback http://127.0.0.1:10426/callback http://127.0.0.1:10427/callback diff --git a/internal/auth/fakeissuer/fakeissuer.go b/internal/auth/fakeissuer/fakeissuer.go index 7167010..976ffee 100644 --- a/internal/auth/fakeissuer/fakeissuer.go +++ b/internal/auth/fakeissuer/fakeissuer.go @@ -86,6 +86,11 @@ type Options struct { // worth modeling is not the refusal but what follows from it: a session // established this way reaches exactly one protected resource. RequireResource bool + // RegisteredResource is the only protected resource this deployment knows, + // when it is set. A request naming any other is refused with invalid_target, + // modeling a resource server that was never registered — the same OAuth + // error as a request that named none, arriving for the opposite reason. + RegisteredResource string // RotateRefreshTokens issues a new refresh token on every refresh, // invalidating the one presented. RotateRefreshTokens bool @@ -130,8 +135,8 @@ type Issuer struct { mutex sync.Mutex codes map[string]codeGrant - refreshTokens map[string][]string // refresh token -> granted scopes - accessTokens map[string]tokenRecord // access token -> introspectable facts + refreshTokens map[string]refreshRecord // refresh token -> what it may renew + accessTokens map[string]tokenRecord // access token -> introspectable facts } type codeGrant struct { @@ -143,6 +148,18 @@ type codeGrant struct { resource string } +// refreshRecord is what a refresh token may renew: the permissions it was +// granted, and the protected resource the authorization bound it to. +// +// The resource travels with the token because that is what the deployments +// requiring one do — the binding is established once, at authorization, and +// every renewal inherits it. A fixture that dropped it would let the shell's +// refresh look correct while a real deployment returned access bound elsewhere. +type refreshRecord struct { + scopes []string + resource string +} + type tokenRecord struct { scopes []string audience string @@ -167,7 +184,7 @@ func New(t *testing.T, opts Options) *Issuer { key: key, keyID: randomToken("key"), codes: map[string]codeGrant{}, - refreshTokens: map[string][]string{}, + refreshTokens: map[string]refreshRecord{}, accessTokens: map[string]tokenRecord{}, } if opts.NegativeSerialCertificate { @@ -197,7 +214,25 @@ func (i *Issuer) SeedSession(scopes []string) string { i.mutex.Lock() defer i.mutex.Unlock() seeded := randomToken("rt") - i.refreshTokens[seeded] = append([]string(nil), scopes...) + i.refreshTokens[seeded] = refreshRecord{scopes: append([]string(nil), scopes...)} + return seeded +} + +// SeedSessionFor stores a session established against one protected resource, +// as a login carrying a resource indicator leaves behind. +// +// It is separate from SeedSession because the binding is the point: a test that +// seeds without one and then asserts the audience would be asserting the +// registration's audience, and would pass whether or not the renewal carried +// anything forward. +func (i *Issuer) SeedSessionFor(scopes []string, resource string) string { + i.mutex.Lock() + defer i.mutex.Unlock() + seeded := randomToken("rt") + i.refreshTokens[seeded] = refreshRecord{ + scopes: append([]string(nil), scopes...), + resource: resource, + } return seeded } @@ -411,7 +446,7 @@ func (i *Issuer) exchangeCode(w http.ResponseWriter, r *http.Request) { if !i.opts.OmitRefreshToken { refreshToken := randomToken("rt") i.mutex.Lock() - i.refreshTokens[refreshToken] = grant.scopes + i.refreshTokens[refreshToken] = refreshRecord{scopes: grant.scopes, resource: grant.resource} i.mutex.Unlock() response["refresh_token"] = refreshToken } @@ -447,7 +482,8 @@ func (i *Issuer) refreshGrant(w http.ResponseWriter, r *http.Request) { requested := splitScopes(r.PostForm.Get("scope")) i.mutex.Lock() - original, found := i.refreshTokens[presented] + record, found := i.refreshTokens[presented] + original := record.scopes issued := original rejected := false if found && len(requested) > 0 { @@ -471,7 +507,10 @@ func (i *Issuer) refreshGrant(w http.ResponseWriter, r *http.Request) { if found && !rejected && i.opts.RotateRefreshTokens { rotated = randomToken("rt") delete(i.refreshTokens, presented) - i.refreshTokens[rotated] = original + // The replacement inherits the resource as well as the permissions. A + // rotation that dropped the binding would leave the session renewable + // but bound to nothing, which no deployment does. + i.refreshTokens[rotated] = record } i.mutex.Unlock() @@ -483,8 +522,10 @@ func (i *Issuer) refreshGrant(w http.ResponseWriter, r *http.Request) { oauthError(w, http.StatusBadRequest, "invalid_scope") return } + // The renewal carries the binding the authorization established, which is + // what makes a resource indicator unnecessary on this grant. response := map[string]any{ - "access_token": i.mintAccessToken("user-1", issued), + "access_token": i.mintAccessTokenFor("user-1", issued, record.resource), "token_type": "Bearer", "expires_in": 300, } @@ -512,6 +553,10 @@ func (i *Issuer) clientCredentialsGrant(w http.ResponseWriter, r *http.Request) oauthError(w, http.StatusBadRequest, "invalid_target") return } + if i.opts.RegisteredResource != "" && resource != i.opts.RegisteredResource { + oauthError(w, http.StatusBadRequest, "invalid_target") + return + } requested := splitScopes(r.PostForm.Get("scope")) issued := requested switch i.opts.ClientScopeMode { @@ -593,10 +638,6 @@ func (i *Issuer) handleIntrospect(w http.ResponseWriter, r *http.Request) { // mintAccessToken signs a real RS256 access token and records it for // introspection. -func (i *Issuer) mintAccessToken(subject string, scopes []string) string { - return i.mintAccessTokenFor(subject, scopes, "") -} - // mintAccessTokenFor mints access bound to one named resource, falling back to // the registration's audience when the request named none. A deployment that // takes a resource indicator binds the token to it and to nothing else, which diff --git a/internal/auth/narrowing.go b/internal/auth/narrowing.go index 30a0a62..a99bdb5 100644 --- a/internal/auth/narrowing.go +++ b/internal/auth/narrowing.go @@ -21,6 +21,8 @@ import ( "slices" "strings" "time" + + "github.com/wso2/wso2-cli/internal/contexts" ) // tokenResponse is what a token endpoint answers a refresh grant with. @@ -54,8 +56,19 @@ const narrowingRecovery = "Check the deployment's API resource registration and // kind of deployment this is, so the shell asked in a shape this one does not // accept. const indicatorRecovery = "This deployment binds access to one named resource and will not issue " + - "any without being told which. Name the identity provider on this identity in the context " + - "document, then retry." + "any without being told which. Name the deployment's identity provider on this identity in " + + "the context document, or set its derivation to " + contexts.DerivationTokenResource + + " explicitly, then retry." + +// unknownResourceRecovery is the way back from a deployment that was told which +// protected resource, and does not know the one it was told. +// +// It is the opposite failure to indicatorRecovery arriving as the same OAuth +// error. The identity already says how this deployment derives access; what is +// wrong is the name it derives against, which is a registration on the +// deployment or a value in the document, and never the derivation itself. +const unknownResourceRecovery = "Register that resource server on the deployment, or correct the " + + "audience on this identity's product entry to one it knows, then retry." // verify proves an issued token is exactly what the module asked for. // diff --git a/internal/auth/resource_test.go b/internal/auth/resource_test.go index 0595e80..00d57b1 100644 --- a/internal/auth/resource_test.go +++ b/internal/auth/resource_test.go @@ -17,8 +17,10 @@ package auth_test import ( + "strings" "testing" + "github.com/wso2/wso2-cli/internal/auth" "github.com/wso2/wso2-cli/internal/auth/fakeissuer" "github.com/wso2/wso2-cli/internal/contexts" ) @@ -44,6 +46,70 @@ func TestAnInlineIdentityBindsAccessToTheResourceItsProductNames(t *testing.T) { } } +// The whole reason the refresh grant was left alone is that a deployment which +// binds by resource carries the binding forward from the authorization that +// established the session. If that were not so, every module's access would +// come back bound to nothing and the broker would refuse it. +// +// Measured on ThunderID v1.0.0-beta: a refresh carrying no indicator returned a +// token still bound to the resource the login named. This proves the shell +// against that behaviour rather than against a transcript of it. +func TestABrowserSessionKeepsItsResourceBindingAcrossARefresh(t *testing.T) { + // The registration's own audience is deliberately not the resource the + // session was established for. If the refresh lost the binding and fell back + // to the registration, the token would carry this instead — which is the + // only way this test can tell a retained binding from a coincidence. + deployment := seedBrowserSession(t, fakeissuer.Options{ + RequireResource: true, + Audience: "registration-audience-not-the-resource", + RotateRefreshTokens: true, + }) + broker := deployment.broker(t) + broker.Selection.Identity.Auth.Provider = contexts.ProviderThunder + + grant, err := broker.Acquire(auth.Request{Audience: audience, Scopes: []string{readScope}}) + if err != nil { + t.Fatalf("acquire: %v", err) + } + _, scopes, audiences := deployment.issuer.Introspect(t, grant.Token) + if len(audiences) != 1 || audiences[0] != audience { + t.Fatalf("the refresh lost the session's resource binding: audience %v", audiences) + } + if len(scopes) != 1 || scopes[0] != readScope { + t.Fatalf("the refresh issued %v, want exactly [%s]", scopes, readScope) + } +} + +// invalid_target says the deployment would not issue for the target it was +// given. That is two different situations, and they send the user to two +// different places: an identity that named no provider asked in a shape this +// deployment does not accept, and one that did name a resource named a resource +// the deployment does not recognise. Telling the second to go and name a +// provider it has already named would be advice it cannot act on. +func TestAResourceTheDeploymentRejectsIsNotReportedAsAMissingOne(t *testing.T) { + deployment := deployInline(t, fakeissuer.Options{ + RequireResource: true, + RegisteredResource: "https://deployment.example.test/some-other-api", + }) + broker := deployment.broker(t) + broker.Selection.Identity.Auth.Provider = contexts.ProviderThunder + + refusal := denied(t, broker, declaredRequest()) + + if refusal.Problem.Code != "auth.narrowing_unavailable" { + t.Fatalf("code = %q, want auth.narrowing_unavailable", refusal.Problem.Code) + } + // The resource the module asked for is not a secret, and naming it is the + // difference between a refusal and a registration someone can go and fix. + if !strings.Contains(refusal.Problem.Message, audience) { + t.Fatalf("the refusal does not name the resource it sent: %q", refusal.Problem.Message) + } + if strings.Contains(refusal.Reported().Recovery, "identity provider") { + t.Fatalf("the refusal told the user to name a provider the identity already names: %q", + refusal.Reported().Recovery) + } +} + // The indicator is sent because the identity says this deployment reads one. // An identity that says nothing must keep asking exactly as it did before, or // every deployment already working would start receiving a parameter it never diff --git a/internal/auth/source_browser_test.go b/internal/auth/source_browser_test.go index 4d5ab19..a5f9e2f 100644 --- a/internal/auth/source_browser_test.go +++ b/internal/auth/source_browser_test.go @@ -63,7 +63,14 @@ func seedBrowserSession(t *testing.T, options fakeissuer.Options) browserDeploym } issuer := fakeissuer.New(t, options) root := t.TempDir() + // A session against a deployment that decides the audience at authorization + // time could only have been established by naming a resource, so the seeded + // one names the product's. Seeding without it would model a session no such + // deployment can issue. seeded := issuer.SeedSession([]string{readScope, writeScope}) + if options.RequireResource { + seeded = issuer.SeedSessionFor([]string{readScope, writeScope}, audience) + } store := session.Store{StateRoot: root} if err := store.Save(sessionRef, session.Session{Issuer: issuer.URL, RefreshToken: seeded}); err != nil { t.Fatalf("seeding the stored session: %v", err) diff --git a/internal/auth/source_clientcred.go b/internal/auth/source_clientcred.go index f6205da..d28fb7d 100644 --- a/internal/auth/source_clientcred.go +++ b/internal/auth/source_clientcred.go @@ -89,7 +89,7 @@ func (s clientCredentialsSource) mint(request Request, now time.Time) (Grant, er issued, err := requestToken(ctx, s.client, endpoint, form, clientAuth{id: s.identity.Auth.ClientID, secret: s.secret}) if err != nil { - return Grant{}, s.refusedGrant(err) + return Grant{}, s.refusedGrant(err, form.Get("resource")) } facts, err := issued.verify(request, s.namespace) if err != nil { @@ -104,10 +104,20 @@ func (s clientCredentialsSource) mint(request Request, now time.Time) (Grant, er // Three answers are worth telling apart, because they send the user to three // different places: a registration the deployment owns, a secret the job owns, // and an issuer this shell cannot speak for. -func (s clientCredentialsSource) refusedGrant(err error) error { +// sent is the resource indicator this request carried, empty when it carried +// none. It is what tells the two halves of invalid_target apart. +func (s clientCredentialsSource) refusedGrant(err error, sent string) error { var refusal issuerRefusal switch { - case errors.As(err, &refusal) && refusal.requiresResourceIndicator(): + case errors.As(err, &refusal) && refusal.rejectedTarget() && sent != "": + // The deployment was told which resource and does not know it. The + // resource is a name the user chose and is not a secret, so the refusal + // says which one was rejected rather than leaving them to guess. + return denial("auth.narrowing_unavailable", + fmt.Sprintf("the deployment does not recognize %q as a protected resource for the %q module", + sent, s.namespace), + unknownResourceRecovery) + case errors.As(err, &refusal) && refusal.rejectedTarget(): return denial("auth.narrowing_unavailable", fmt.Sprintf("the deployment will not issue access for the %q module without being told "+ "which protected resource it is for", s.namespace), diff --git a/internal/auth/tokenrequest.go b/internal/auth/tokenrequest.go index 23bcef1..c1d3bb7 100644 --- a/internal/auth/tokenrequest.go +++ b/internal/auth/tokenrequest.go @@ -80,14 +80,15 @@ func (r issuerRefusal) refusedToNarrow() bool { return r.status == http.StatusBadRequest && r.code == "invalid_scope" } -// requiresResourceIndicator reports RFC 8707's answer for a request that named -// no protected resource on a deployment that decides the audience from one. +// rejectedTarget reports RFC 8707's answer for a request whose protected +// resource the deployment would not issue for. // -// It is worth telling apart from every other refusal because it is the one -// caused by the context document rather than by the deployment: the identity -// did not say which product this deployment binds access to, and the fix is a -// line in a document rather than a change to a registration. -func (r issuerRefusal) requiresResourceIndicator() bool { +// It says only that, and deliberately not why. One error covers two opposite +// causes — a request that named no resource on a deployment that requires one, +// and a request that named one the deployment does not know — and which of them +// happened is a fact about the request, not about the answer. The caller made +// the request and is the only one that can tell. +func (r issuerRefusal) rejectedTarget() bool { return r.status == http.StatusBadRequest && r.code == "invalid_target" } diff --git a/internal/contexts/derivation_test.go b/internal/contexts/derivation_test.go index 5f3643d..c33faee 100644 --- a/internal/contexts/derivation_test.go +++ b/internal/contexts/derivation_test.go @@ -124,6 +124,23 @@ func TestAThunderIdentityWhoseProductNamesNoAudienceIsRefused(t *testing.T) { assertProblemCode(t, err, "contexts.document_malformed") } +// An identity that binds access to a resource and names no product has no +// resource to bind to. Login would then send no indicator and be refused by the +// deployment, which is the failure this validation exists to move earlier. +func TestAResourceBoundIdentityWithoutAProductIsRefused(t *testing.T) { + _, err := contexts.Decode([]byte(withoutProducts(withProvider(contexts.ProviderThunder)))) + assertProblemCode(t, err, "contexts.document_malformed") +} + +// An identity that derives the way every deployment did before may still +// declare no product; the refusal above is about the derivation, not about +// identities in general. +func TestAnIdentityWithoutAProductStaysLegalOtherwise(t *testing.T) { + if _, err := contexts.Decode([]byte(withoutProducts(validV2()))); err != nil { + t.Fatalf("an identity with no products should validate: %v", err) + } +} + // A resource indicator is an absolute URI by RFC 8707 section 2, so a document // that derives by resource and names a bare identifier describes a request the // deployment will refuse. Catching it here is the difference between a document @@ -222,6 +239,15 @@ func withDerivation(provider, derivation string) string { `"kind": "oauth-browser", "narrowing": "`+derivation+`",`, 1) } +func withoutProducts(document string) string { + opening := strings.Index(document, `"products": {`) + // The identity's own closing brace is the first one at this indentation; + // the product's is nested deeper, so anchoring on the newline is what keeps + // this from cutting the document in the wrong place. + closing := strings.Index(document[opening:], "\n }") + opening + len("\n }") + return document[:opening] + `"products": {}` + document[closing:] +} + func withSecondProduct(document string) string { return strings.Replace(document, `"reference": {`, diff --git a/internal/contexts/identity.go b/internal/contexts/identity.go index 190f672..903208a 100644 --- a/internal/contexts/identity.go +++ b/internal/contexts/identity.go @@ -231,10 +231,14 @@ func (i Identity) validateDerivation() error { if i.Auth.Derivation() != DerivationTokenResource { return nil } - if len(i.Products) > 1 { + // Exactly one, not at most one. A deployment that binds by resource takes + // the resource from the identity's product, so an identity with none has + // nothing to name: login would send no indicator and be refused, which is + // the failure this whole validation exists to move earlier. + if len(i.Products) != 1 { return malformed(fmt.Sprintf( "declares the identity %q against a deployment that binds one login to one product, "+ - "and gives it %d products", i.Name, len(i.Products))) + "and gives it %d", i.Name, len(i.Products))) } for _, namespace := range slices.Sorted(maps.Keys(i.Products)) { audience := i.Products[namespace].Audience