From 04dc8fee1b7f942770dcd23a1dfd94ab7d37306e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sun, 12 Apr 2026 19:01:24 +0100 Subject: [PATCH 1/4] Extract SubjectTokenValidator interface for multi-issuer support Introduce a SubjectTokenValidator interface with a single Validate method, preparing for multi-issuer token validation where external OIDC tokens (e.g., Keycloak) need different JWKS resolution than self-issued tokens. Rename the existing concrete struct to SelfIssuedTokenValidator to clarify its role. The handler now depends on the interface, not the concrete type, following Go's "accept interfaces, return structs" pattern. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../server/tokenexchange/factory.go | 2 +- .../server/tokenexchange/handler.go | 2 +- .../server/tokenexchange/handler_test.go | 4 +-- pkg/authserver/server/tokenexchange/types.go | 11 ++++++++ .../server/tokenexchange/validator.go | 17 ++++++----- .../server/tokenexchange/validator_test.go | 28 +++++++++---------- 6 files changed, 39 insertions(+), 25 deletions(-) create mode 100644 pkg/authserver/server/tokenexchange/types.go diff --git a/pkg/authserver/server/tokenexchange/factory.go b/pkg/authserver/server/tokenexchange/factory.go index 3fe0324145..30cccbd6dc 100644 --- a/pkg/authserver/server/tokenexchange/factory.go +++ b/pkg/authserver/server/tokenexchange/factory.go @@ -25,7 +25,7 @@ func Factory(delegationLifespan time.Duration) (server.Factory, error) { time.Duration(0), server.MaxAccessTokenLifespan, delegationLifespan) } return func(config *server.AuthorizationServerConfig, storage fosite.Storage, strategy any) (any, error) { - validator, err := NewSubjectTokenValidator(config.PublicJWKS(), config.GetAccessTokenIssuer(), config.AllowedAudiences) + validator, err := NewSelfIssuedTokenValidator(config.PublicJWKS(), config.GetAccessTokenIssuer(), config.AllowedAudiences) if err != nil { return nil, fmt.Errorf("tokenexchange: failed to create subject token validator: %w", err) } diff --git a/pkg/authserver/server/tokenexchange/handler.go b/pkg/authserver/server/tokenexchange/handler.go index 51b2f43c82..22369b9629 100644 --- a/pkg/authserver/server/tokenexchange/handler.go +++ b/pkg/authserver/server/tokenexchange/handler.go @@ -46,7 +46,7 @@ const maxDelegationDepth = 10 // token effort. type Handler struct { *oauth2.HandleHelper - validator *SubjectTokenValidator + validator SubjectTokenValidator delegationLifespan time.Duration config tokenExchangeConfig allowedAudiences []string diff --git a/pkg/authserver/server/tokenexchange/handler_test.go b/pkg/authserver/server/tokenexchange/handler_test.go index 3cffb049c9..d60cffc8a0 100644 --- a/pkg/authserver/server/tokenexchange/handler_test.go +++ b/pkg/authserver/server/tokenexchange/handler_test.go @@ -30,7 +30,7 @@ const testAgentClientID = "devops-agent" func newTestHandler(t *testing.T, tj *testJWKS, delegationLifespan time.Duration) *Handler { t.Helper() - validator, err := NewSubjectTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) require.NoError(t, err) return &Handler{ @@ -963,7 +963,7 @@ func newTestHandlerWithHelper(t *testing.T, tj *testJWKS, accessLifespan time.Du const delegationLifespan = 15 * time.Minute - validator, err := NewSubjectTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) require.NoError(t, err) cfg := &mockConfig{ diff --git a/pkg/authserver/server/tokenexchange/types.go b/pkg/authserver/server/tokenexchange/types.go new file mode 100644 index 0000000000..d5f7d9846e --- /dev/null +++ b/pkg/authserver/server/tokenexchange/types.go @@ -0,0 +1,11 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package tokenexchange + +import "context" + +// SubjectTokenValidator validates subject tokens presented during RFC 8693 token exchange. +type SubjectTokenValidator interface { + Validate(ctx context.Context, rawToken string) (*ValidatedClaims, error) +} diff --git a/pkg/authserver/server/tokenexchange/validator.go b/pkg/authserver/server/tokenexchange/validator.go index 02bf21686d..07a0822043 100644 --- a/pkg/authserver/server/tokenexchange/validator.go +++ b/pkg/authserver/server/tokenexchange/validator.go @@ -66,16 +66,19 @@ type MayActClaim struct { Sub string `json:"sub"` } -// SubjectTokenValidator validates subject tokens presented during RFC 8693 token exchange. +// Compile-time check that SelfIssuedTokenValidator implements SubjectTokenValidator. +var _ SubjectTokenValidator = (*SelfIssuedTokenValidator)(nil) + +// SelfIssuedTokenValidator validates subject tokens presented during RFC 8693 token exchange. // It verifies that the token was issued by this authorization server by checking // the signature against the server's own JWKS, and validates standard JWT claims. -type SubjectTokenValidator struct { +type SelfIssuedTokenValidator struct { publicJWKS *jose.JSONWebKeySet issuer string allowedAudiences []string } -// NewSubjectTokenValidator creates a new validator for subject tokens. +// NewSelfIssuedTokenValidator creates a new validator for subject tokens. // The jwks parameter must be non-nil and contain only the authorization server's // public signing keys (e.g. AuthorizationServerConfig.PublicJWKS) — the validator // only ever verifies signatures, so it must not be handed private key material. @@ -83,16 +86,16 @@ type SubjectTokenValidator struct { // set of audiences this server accepts in a subject token's "aud" claim; per the // same secure default as AuthorizationServerConfig.AllowedAudiences, an empty // allowedAudiences rejects every subject token rather than skipping the check. -func NewSubjectTokenValidator( +func NewSelfIssuedTokenValidator( jwks *jose.JSONWebKeySet, issuer string, allowedAudiences []string, -) (*SubjectTokenValidator, error) { +) (*SelfIssuedTokenValidator, error) { if jwks == nil { return nil, fmt.Errorf("JWKS must not be nil") } if issuer == "" { return nil, fmt.Errorf("issuer must not be empty") } - return &SubjectTokenValidator{ + return &SelfIssuedTokenValidator{ publicJWKS: jwks, issuer: issuer, allowedAudiences: allowedAudiences, @@ -113,7 +116,7 @@ func NewSubjectTokenValidator( // against the client's registered audiences. // // Returns the validated claims on success, or a descriptive error on failure. -func (v *SubjectTokenValidator) Validate(_ context.Context, rawToken string) (*ValidatedClaims, error) { +func (v *SelfIssuedTokenValidator) Validate(_ context.Context, rawToken string) (*ValidatedClaims, error) { parsedToken, err := jwt.ParseSigned(rawToken, allowedSignatureAlgorithms) if err != nil { return nil, fmt.Errorf("subject token is not a valid JWT: %w", err) diff --git a/pkg/authserver/server/tokenexchange/validator_test.go b/pkg/authserver/server/tokenexchange/validator_test.go index 03ae858ad7..783203b0a2 100644 --- a/pkg/authserver/server/tokenexchange/validator_test.go +++ b/pkg/authserver/server/tokenexchange/validator_test.go @@ -99,34 +99,34 @@ func validExtraClaims() map[string]any { } } -func TestSubjectTokenValidator_NewValidation(t *testing.T) { +func TestSelfIssuedTokenValidator_NewValidation(t *testing.T) { t.Parallel() tj := newTestJWKS(t) t.Run("nil JWKS returns error", func(t *testing.T) { t.Parallel() - _, err := NewSubjectTokenValidator(nil, testIssuer, []string{testIssuer}) + _, err := NewSelfIssuedTokenValidator(nil, testIssuer, []string{testIssuer}) require.Error(t, err) assert.Contains(t, err.Error(), "JWKS must not be nil") }) t.Run("empty issuer returns error", func(t *testing.T) { t.Parallel() - _, err := NewSubjectTokenValidator(tj.publicJWKS(), "", []string{testIssuer}) + _, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), "", []string{testIssuer}) require.Error(t, err) assert.Contains(t, err.Error(), "issuer must not be empty") }) t.Run("valid params succeed", func(t *testing.T) { t.Parallel() - v, err := NewSubjectTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) + v, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) require.NoError(t, err) assert.NotNil(t, v) }) } -func TestSubjectTokenValidator_Validate(t *testing.T) { +func TestSelfIssuedTokenValidator_Validate(t *testing.T) { t.Parallel() tj := newTestJWKS(t) @@ -302,7 +302,7 @@ func TestSubjectTokenValidator_Validate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validator, err := NewSubjectTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), testIssuer, []string{testIssuer}) require.NoError(t, err) rawToken := tt.token(t) @@ -324,7 +324,7 @@ func TestSubjectTokenValidator_Validate(t *testing.T) { } } -func TestSubjectTokenValidator_AudienceValidation(t *testing.T) { +func TestSelfIssuedTokenValidator_AudienceValidation(t *testing.T) { t.Parallel() tj := newTestJWKS(t) @@ -359,7 +359,7 @@ func TestSubjectTokenValidator_AudienceValidation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validator, err := NewSubjectTokenValidator(tj.publicJWKS(), testIssuer, tt.allowedAudiences) + validator, err := NewSelfIssuedTokenValidator(tj.publicJWKS(), testIssuer, tt.allowedAudiences) require.NoError(t, err) claims := validClaims() @@ -417,7 +417,7 @@ func signWithJWK(t *testing.T, signingKey jose.JSONWebKey, alg jose.SignatureAlg return raw } -func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { +func TestSelfIssuedTokenValidator_MultiKeyJWKS(t *testing.T) { t.Parallel() t.Run("token verified with kid-matched key", func(t *testing.T) { @@ -427,7 +427,7 @@ func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { jwk2 := newECDSAJWK(t, "test-key-2") jwks := publicJWKSOf(jwk1, jwk2) - validator, err := NewSubjectTokenValidator(jwks, testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) rawToken := signWithJWK(t, jwk2, jose.ES256, validClaims()) @@ -445,7 +445,7 @@ func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { jwk2 := newECDSAJWK(t, "test-key-2") jwks := publicJWKSOf(jwk1, jwk2) - validator, err := NewSubjectTokenValidator(jwks, testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) // Sign with jwk1 (whose public half is in the JWKS) but omit the kid @@ -467,7 +467,7 @@ func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { jwk1 := newECDSAJWK(t, "test-key-1") jwks := publicJWKSOf(jwk1) - validator, err := NewSubjectTokenValidator(jwks, testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) // Key B is not in the JWKS. @@ -487,7 +487,7 @@ func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { jwk2 := newECDSAJWK(t, "test-key-2") jwks := publicJWKSOf(jwk1, jwk2) - validator, err := NewSubjectTokenValidator(jwks, testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) // Sign with jwk2's key material but claim jwk1's kid in the header. @@ -517,7 +517,7 @@ func TestSubjectTokenValidator_MultiKeyJWKS(t *testing.T) { } jwks := publicJWKSOf(rsaJWK) - validator, err := NewSubjectTokenValidator(jwks, testIssuer, []string{testIssuer}) + validator, err := NewSelfIssuedTokenValidator(jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) rawToken := signWithJWK(t, rsaJWK, jose.RS256, validClaims()) From 75b82d69f8ce19a425b3824ce9acd6a246e833f5 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sun, 12 Apr 2026 19:15:38 +0100 Subject: [PATCH 2/4] Add multi-issuer token validator for external OIDC token exchange Implement MultiIssuerTokenValidator that routes JWT validation based on the token's issuer claim. Self-issued tokens delegate to the existing SelfIssuedTokenValidator. External tokens (e.g., Keycloak) are validated against the issuer's JWKS, fetched via OIDC discovery with caching. Security: JWKS URLs from OIDC discovery are validated to require HTTPS and reject private/loopback addresses (SSRF prevention). Discovered URLs are re-resolved when the JWKS cache expires. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../tokenexchange/multi_issuer_validator.go | 326 ++++++++++++++ .../multi_issuer_validator_test.go | 397 ++++++++++++++++++ 2 files changed, 723 insertions(+) create mode 100644 pkg/authserver/server/tokenexchange/multi_issuer_validator.go create mode 100644 pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go new file mode 100644 index 0000000000..9a256da46f --- /dev/null +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go @@ -0,0 +1,326 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package tokenexchange + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/url" + "sync" + "time" + + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" + + "github.com/stacklok/toolhive/pkg/oauthproto" +) + +const ( + // jwksCacheTTL is the time-to-live for cached JWKS fetched from external issuers. + jwksCacheTTL = 5 * time.Minute + + // httpTimeout is the timeout for HTTP requests to external OIDC endpoints. + httpTimeout = 10 * time.Second + + // maxResponseBodySize is the maximum size of HTTP response bodies read from + // external OIDC endpoints (1 MiB). This prevents resource exhaustion from + // unexpectedly large responses. + maxResponseBodySize = 1 << 20 +) + +// Compile-time check that MultiIssuerTokenValidator implements SubjectTokenValidator. +var _ SubjectTokenValidator = (*MultiIssuerTokenValidator)(nil) + +// TrustedIssuer configures an external OIDC issuer whose tokens are +// accepted as subject tokens during token exchange. +type TrustedIssuer struct { + // IssuerURL is the expected "iss" claim value (exact match). + IssuerURL string + // ExpectedAudience is the expected "aud" claim value that must appear + // in the token's audience list. + ExpectedAudience string + // JWKSURL is the URL to fetch the issuer's JSON Web Key Set from. + // If empty, it is resolved via OIDC discovery at {IssuerURL}/.well-known/openid-configuration. + JWKSURL string +} + +// MultiIssuerTokenValidator validates subject tokens from the authorization +// server itself or from configured external OIDC issuers. +// +// For self-issued tokens (where the "iss" claim matches selfIssuer), validation +// is delegated to the SelfIssuedTokenValidator. For tokens from trusted external +// issuers, the validator resolves the issuer's JWKS (via OIDC discovery if needed), +// verifies the JWT signature, and validates standard claims. +type MultiIssuerTokenValidator struct { + selfIssuer string + selfValidator *SelfIssuedTokenValidator + issuers map[string]*externalIssuerConfig + httpClient *http.Client + + // insecureSkipJWKSURLValidation disables HTTPS enforcement on discovered + // JWKS URLs. This MUST only be set for testing with httptest servers. + insecureSkipJWKSURLValidation bool +} + +// externalIssuerConfig holds the configuration and cached state for an external +// OIDC issuer. The mutex protects lazy JWKS URL discovery and JWKS caching. +type externalIssuerConfig struct { + TrustedIssuer + + mu sync.Mutex + jwksURL string // resolved from OIDC discovery or preconfigured + jwks *jose.JSONWebKeySet // cached JWKS + jwksExp time.Time // when the cached JWKS expires +} + +// NewMultiIssuerTokenValidator creates a validator that accepts tokens from the +// authorization server itself and from the provided list of trusted external issuers. +func NewMultiIssuerTokenValidator( + selfValidator *SelfIssuedTokenValidator, + selfIssuer string, + trustedIssuers []TrustedIssuer, +) *MultiIssuerTokenValidator { + issuers := make(map[string]*externalIssuerConfig, len(trustedIssuers)) + for _, ti := range trustedIssuers { + issuers[ti.IssuerURL] = &externalIssuerConfig{ + TrustedIssuer: ti, + jwksURL: ti.JWKSURL, + } + } + + return &MultiIssuerTokenValidator{ + selfIssuer: selfIssuer, + selfValidator: selfValidator, + issuers: issuers, + httpClient: &http.Client{ + Timeout: httpTimeout, + }, + } +} + +// Validate parses the raw JWT to extract the issuer claim, then routes validation +// to either the self-issued validator or the appropriate external issuer validator. +// Returns an error if the issuer is not trusted. +func (v *MultiIssuerTokenValidator) Validate(ctx context.Context, rawToken string) (*ValidatedClaims, error) { + // Parse the JWT without verification to peek at the issuer claim. + // This is safe because we verify the signature in a subsequent step. + issuer, err := peekIssuer(rawToken) + if err != nil { + return nil, fmt.Errorf("failed to determine token issuer: %w", err) + } + + // Self-issued tokens are delegated to the existing validator. + if issuer == v.selfIssuer { + return v.selfValidator.Validate(ctx, rawToken) + } + + // Look up the external issuer configuration. + issuerConfig, ok := v.issuers[issuer] + if !ok { + return nil, fmt.Errorf("untrusted issuer: %q", issuer) + } + + return v.validateExternalToken(ctx, rawToken, issuerConfig) +} + +// validateExternalToken verifies a JWT from a trusted external issuer by +// fetching the issuer's JWKS (with caching) and validating the signature and claims. +func (v *MultiIssuerTokenValidator) validateExternalToken( + ctx context.Context, + rawToken string, + issuerConfig *externalIssuerConfig, +) (*ValidatedClaims, error) { + parsedToken, err := jwt.ParseSigned(rawToken, allowedSignatureAlgorithms) + if err != nil { + return nil, fmt.Errorf("subject token is not a valid JWT: %w", err) + } + + jwks, err := v.resolveJWKS(ctx, issuerConfig) + if err != nil { + return nil, fmt.Errorf("failed to fetch JWKS for issuer %s: %w", issuerConfig.IssuerURL, err) + } + + standardClaims, extraClaims, err := verifySignature(parsedToken, jwks) + if err != nil { + return nil, err + } + + // Validate standard claims: issuer must match, audience must contain the expected value, + // and the token must not be expired. + expected := jwt.Expected{ + Issuer: issuerConfig.IssuerURL, + AnyAudience: jwt.Audience{issuerConfig.ExpectedAudience}, + } + if err := standardClaims.ValidateWithLeeway(expected, 0); err != nil { + return nil, fmt.Errorf("subject token claims validation failed: %w", err) + } + + // Subject is required for delegation. + if standardClaims.Subject == "" { + return nil, fmt.Errorf("subject token is missing required 'sub' claim") + } + + return buildValidatedClaims(standardClaims, extraClaims), nil +} + +// resolveJWKS returns the cached JWKS for an external issuer, fetching it if +// the cache is empty or expired. If the JWKS URL is not configured, it is first +// resolved via OIDC discovery. +func (v *MultiIssuerTokenValidator) resolveJWKS( + ctx context.Context, + issuerConfig *externalIssuerConfig, +) (*jose.JSONWebKeySet, error) { + issuerConfig.mu.Lock() + defer issuerConfig.mu.Unlock() + + // Return cached JWKS if still valid. + if issuerConfig.jwks != nil && time.Now().Before(issuerConfig.jwksExp) { + return issuerConfig.jwks, nil + } + + // Cache expired — clear the discovered URL so we re-discover on next fetch. + // This handles the (rare) case where an issuer rotates its JWKS endpoint URL. + // The explicitly configured JWKSURL (from TrustedIssuer) is preserved. + if issuerConfig.TrustedIssuer.JWKSURL == "" { + issuerConfig.jwksURL = "" + } + + // Discover the JWKS URL if not yet resolved. + if issuerConfig.jwksURL == "" { + jwksURL, err := v.discoverJWKSURL(ctx, issuerConfig.IssuerURL) + if err != nil { + return nil, fmt.Errorf("OIDC discovery failed for %s: %w", issuerConfig.IssuerURL, err) + } + issuerConfig.jwksURL = jwksURL + } + + // Fetch and cache the JWKS. + jwks, err := v.fetchJWKS(ctx, issuerConfig.jwksURL) + if err != nil { + return nil, err + } + + issuerConfig.jwks = jwks + issuerConfig.jwksExp = time.Now().Add(jwksCacheTTL) + + return jwks, nil +} + +// peekIssuer parses a JWT without signature verification to extract the "iss" claim. +func peekIssuer(rawToken string) (string, error) { + token, err := jwt.ParseSigned(rawToken, allowedSignatureAlgorithms) + if err != nil { + return "", fmt.Errorf("subject token is not a valid JWT: %w", err) + } + + var claims jwt.Claims + if err := token.UnsafeClaimsWithoutVerification(&claims); err != nil { + return "", fmt.Errorf("failed to extract claims from subject token: %w", err) + } + + if claims.Issuer == "" { + return "", fmt.Errorf("subject token is missing 'iss' claim") + } + + return claims.Issuer, nil +} + +// discoverJWKSURL performs OIDC discovery to resolve the JWKS URL for an issuer. +// It fetches the OpenID Connect discovery document at {issuerURL}/.well-known/openid-configuration +// and extracts the jwks_uri field. +func (v *MultiIssuerTokenValidator) discoverJWKSURL(ctx context.Context, issuerURL string) (string, error) { + discoveryURL := issuerURL + "/.well-known/openid-configuration" + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create discovery request: %w", err) + } + + resp, err := v.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("discovery request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("discovery endpoint returned status %d", resp.StatusCode) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize)) + if err != nil { + return "", fmt.Errorf("failed to read discovery response: %w", err) + } + + var doc oauthproto.OIDCDiscoveryDocument + if err := json.Unmarshal(body, &doc); err != nil { + return "", fmt.Errorf("failed to parse discovery document: %w", err) + } + + if doc.JWKSURI == "" { + return "", fmt.Errorf("discovery document missing 'jwks_uri'") + } + + if !v.insecureSkipJWKSURLValidation { + if err := validateJWKSURL(doc.JWKSURI); err != nil { + return "", fmt.Errorf("discovered jwks_uri is invalid: %w", err) + } + } + + return doc.JWKSURI, nil +} + +// validateJWKSURL checks that the JWKS URL uses HTTPS and is not a private/loopback address. +// This prevents SSRF attacks where a compromised discovery document points to internal services. +func validateJWKSURL(jwksURL string) error { + u, err := url.Parse(jwksURL) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + + if u.Scheme != "https" { + return fmt.Errorf("must use HTTPS, got %q", u.Scheme) + } + + host := u.Hostname() + ip := net.ParseIP(host) + if ip != nil && (ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()) { + return fmt.Errorf("must not point to a private or loopback address") + } + + return nil +} + +// fetchJWKS fetches a JSON Web Key Set from the given URL. +func (v *MultiIssuerTokenValidator) fetchJWKS(ctx context.Context, jwksURL string) (*jose.JSONWebKeySet, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, jwksURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create JWKS request: %w", err) + } + + resp, err := v.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("JWKS request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("JWKS endpoint returned status %d", resp.StatusCode) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize)) + if err != nil { + return nil, fmt.Errorf("failed to read JWKS response: %w", err) + } + + var jwks jose.JSONWebKeySet + if err := json.Unmarshal(body, &jwks); err != nil { + return nil, fmt.Errorf("failed to parse JWKS: %w", err) + } + + return &jwks, nil +} diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go new file mode 100644 index 0000000000..e28089b179 --- /dev/null +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go @@ -0,0 +1,397 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package tokenexchange + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/go-jose/go-jose/v4/jwt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testExternalIssuer = "https://keycloak.example.com/realms/test" + testExternalAudience = "toolhive-authserver" +) + +// newExternalTestJWKS creates a separate JWKS for simulating an external issuer. +// It reuses newTestJWKS but conceptually represents a different signing authority. +func newExternalTestJWKS(t *testing.T) *testJWKS { + t.Helper() + return newTestJWKS(t) +} + +// startJWKSServer creates a test HTTP server that serves a JWKS endpoint. +// The returned server must be closed by the caller. +func startJWKSServer(t *testing.T, tj *testJWKS) *httptest.Server { + t.Helper() + + mux := http.NewServeMux() + mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { + // Serve only the public keys. + publicJWKS := publicKeysFrom(t, tj) + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(publicJWKS) + require.NoError(t, err) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// publicKeysFrom extracts the public portion of the test JWKS keys. +func publicKeysFrom(t *testing.T, tj *testJWKS) map[string]interface{} { + t.Helper() + keys := make([]map[string]interface{}, 0, len(tj.jwks.Keys)) + for _, key := range tj.jwks.Keys { + pub := key.Public() + raw, err := pub.MarshalJSON() + require.NoError(t, err, "failed to marshal public key") + var m map[string]interface{} + require.NoError(t, json.Unmarshal(raw, &m), "failed to unmarshal public key") + keys = append(keys, m) + } + return map[string]interface{}{"keys": keys} +} + +// startDiscoveryServer creates a test HTTP server that serves both OIDC discovery +// and JWKS endpoints, simulating an external OIDC provider. +func startDiscoveryServer(t *testing.T, tj *testJWKS) *httptest.Server { + t.Helper() + + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + // The jwks_uri must use the test server's own base URL, which we + // don't know until the server starts. We use the Host header to + // construct it. + scheme := "http" + jwksURI := scheme + "://" + r.Host + "/jwks" + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{ + "issuer": testExternalIssuer, + "jwks_uri": jwksURI, + }) + }) + mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { + publicJWKS := publicKeysFrom(t, tj) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(publicJWKS) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// newMultiValidator creates a MultiIssuerTokenValidator configured for testing. +// The external JWKS URL is pre-resolved (no discovery needed) unless jwksURL is empty. +func newMultiValidator( + t *testing.T, + selfJWKS *testJWKS, + trustedIssuers []TrustedIssuer, +) *MultiIssuerTokenValidator { + t.Helper() + + selfValidator, err := NewSelfIssuedTokenValidator(selfJWKS.jwks, testIssuer, []string{testIssuer}) + require.NoError(t, err) + + v := NewMultiIssuerTokenValidator(selfValidator, testIssuer, trustedIssuers) + v.insecureSkipJWKSURLValidation = true // Allow HTTP test servers + return v +} + +// externalClaims returns standard JWT claims for a token issued by the external issuer. +func externalClaims() jwt.Claims { + now := time.Now() + return jwt.Claims{ + Subject: "ext-user-456", + Issuer: testExternalIssuer, + Audience: jwt.Audience{testExternalAudience}, + Expiry: jwt.NewNumericDate(now.Add(time.Hour)), + IssuedAt: jwt.NewNumericDate(now), + NotBefore: jwt.NewNumericDate(now.Add(-time.Minute)), + ID: "jti-ext-789", + } +} + +func TestMultiIssuerTokenValidator_Validate(t *testing.T) { + t.Parallel() + + selfJWKS := newTestJWKS(t) + externalJWKS := newExternalTestJWKS(t) + jwksServer := startJWKSServer(t, externalJWKS) + + tests := []struct { + name string + trustedIssuers []TrustedIssuer + token func(t *testing.T) string + wantErr bool + errContains string + check func(t *testing.T, vc *ValidatedClaims) + }{ + { + name: "self-issued token routes to self validator", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + return selfJWKS.signToken(t, validClaims(), validExtraClaims()) + }, + check: func(t *testing.T, vc *ValidatedClaims) { + t.Helper() + assert.Equal(t, "user-123", vc.Subject) + assert.Equal(t, testIssuer, vc.Issuer) + assert.Equal(t, []string{testIssuer}, vc.Audience) + assert.Equal(t, "Test User", vc.Name) + assert.Equal(t, "test@example.com", vc.Email) + }, + }, + { + name: "external token accepted", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + return externalJWKS.signToken(t, externalClaims(), map[string]interface{}{ + "name": "External User", + "email": "ext@keycloak.example.com", + }) + }, + check: func(t *testing.T, vc *ValidatedClaims) { + t.Helper() + assert.Equal(t, "ext-user-456", vc.Subject) + assert.Equal(t, testExternalIssuer, vc.Issuer) + assert.Equal(t, []string{testExternalAudience}, vc.Audience) + assert.Equal(t, "jti-ext-789", vc.JWTID) + assert.Equal(t, "External User", vc.Name) + assert.Equal(t, "ext@keycloak.example.com", vc.Email) + assert.False(t, vc.Expiry.IsZero()) + assert.False(t, vc.IssuedAt.IsZero()) + }, + }, + { + name: "external token wrong audience", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + claims := externalClaims() + claims.Audience = jwt.Audience{"wrong-audience"} + return externalJWKS.signToken(t, claims, nil) + }, + wantErr: true, + errContains: "claims validation failed", + }, + { + name: "unknown issuer rejected", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + claims := externalClaims() + claims.Issuer = "https://evil.example.com" + return externalJWKS.signToken(t, claims, nil) + }, + wantErr: true, + errContains: "untrusted issuer", + }, + { + name: "external token bad signature", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + // Sign with a different key than the one the JWKS server serves. + wrongJWKS := newTestJWKS(t) + return wrongJWKS.signToken(t, externalClaims(), nil) + }, + wantErr: true, + errContains: "signature verification failed", + }, + { + name: "self-issued token signed by external key fails", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + // Token claims say iss=self, but signed by the external key. + // Routes to self validator, which rejects the signature. + return externalJWKS.signToken(t, validClaims(), nil) + }, + wantErr: true, + errContains: "signature verification failed", + }, + { + name: "external token missing subject", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + claims := externalClaims() + claims.Subject = "" + return externalJWKS.signToken(t, claims, nil) + }, + wantErr: true, + errContains: "missing required 'sub' claim", + }, + { + name: "external token expired", + trustedIssuers: []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }}, + token: func(t *testing.T) string { + t.Helper() + claims := externalClaims() + claims.Expiry = jwt.NewNumericDate(time.Now().Add(-time.Hour)) + claims.IssuedAt = jwt.NewNumericDate(time.Now().Add(-2 * time.Hour)) + return externalJWKS.signToken(t, claims, nil) + }, + wantErr: true, + errContains: "claims validation failed", + }, + { + name: "malformed token", + trustedIssuers: nil, + token: func(_ *testing.T) string { + return "not-a-jwt" + }, + wantErr: true, + errContains: "failed to determine token issuer", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + validator := newMultiValidator(t, selfJWKS, tt.trustedIssuers) + rawToken := tt.token(t) + + result, err := validator.Validate(context.Background(), rawToken) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + assert.Nil(t, result) + return + } + + require.NoError(t, err) + require.NotNil(t, result) + if tt.check != nil { + tt.check(t, result) + } + }) + } +} + +func TestMultiIssuerTokenValidator_OIDCDiscovery(t *testing.T) { + t.Parallel() + + selfJWKS := newTestJWKS(t) + externalJWKS := newExternalTestJWKS(t) + discoveryServer := startDiscoveryServer(t, externalJWKS) + + // Configure the trusted issuer WITHOUT a JWKS URL, forcing OIDC discovery. + // The discovery server's URL is used as the issuer URL so that the + // /.well-known/openid-configuration endpoint is reachable. + trustedIssuers := []TrustedIssuer{{ + IssuerURL: discoveryServer.URL, + ExpectedAudience: testExternalAudience, + // JWKSURL intentionally left empty to trigger discovery. + }} + + validator := newMultiValidator(t, selfJWKS, trustedIssuers) + + // Sign a token with the external key, using the discovery server's URL as issuer. + claims := jwt.Claims{ + Subject: "discovered-user", + Issuer: discoveryServer.URL, + Audience: jwt.Audience{testExternalAudience}, + Expiry: jwt.NewNumericDate(time.Now().Add(time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now()), + NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Minute)), + ID: "jti-disc-001", + } + rawToken := externalJWKS.signToken(t, claims, nil) + + result, err := validator.Validate(context.Background(), rawToken) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "discovered-user", result.Subject) + assert.Equal(t, discoveryServer.URL, result.Issuer) +} + +func TestMultiIssuerTokenValidator_JWKSCaching(t *testing.T) { + t.Parallel() + + selfJWKS := newTestJWKS(t) + externalJWKS := newExternalTestJWKS(t) + + // Track how many times the JWKS endpoint is hit. + var fetchCount atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { + fetchCount.Add(1) + publicJWKS := publicKeysFrom(t, externalJWKS) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(publicJWKS) + }) + jwksServer := httptest.NewServer(mux) + t.Cleanup(jwksServer.Close) + + trustedIssuers := []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }} + + validator := newMultiValidator(t, selfJWKS, trustedIssuers) + + // Validate two tokens — the JWKS should be fetched only once (cached). + for i := range 2 { + claims := externalClaims() + claims.ID = fmt.Sprintf("jti-cache-%d", i) + rawToken := externalJWKS.signToken(t, claims, nil) + + result, err := validator.Validate(context.Background(), rawToken) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "ext-user-456", result.Subject) + } + + assert.Equal(t, int32(1), fetchCount.Load(), "JWKS should be fetched only once due to caching") +} From 51b80eb0d41e8cc3d305dbe274084c482f845630 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 14 Jul 2026 23:42:37 +0100 Subject: [PATCH 3/4] Fail loudly on empty ExpectedAudience, fix subject-token error code Address panel review feedback on the multi-issuer validator: NewMultiIssuerTokenValidator silently accepted a TrustedIssuer with no ExpectedAudience instead of rejecting it, and subject-token validation failures returned invalid_grant instead of invalid_request. Also drain response bodies before closing on non-200 discovery/JWKS fetches. Co-Authored-By: Claude Sonnet 5 --- pkg/authserver/server/tokenexchange/handler.go | 2 +- .../server/tokenexchange/handler_test.go | 2 +- .../tokenexchange/multi_issuer_validator.go | 16 +++++++++++++--- .../tokenexchange/multi_issuer_validator_test.go | 3 ++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/authserver/server/tokenexchange/handler.go b/pkg/authserver/server/tokenexchange/handler.go index 22369b9629..b346ef334c 100644 --- a/pkg/authserver/server/tokenexchange/handler.go +++ b/pkg/authserver/server/tokenexchange/handler.go @@ -115,7 +115,7 @@ func (h *Handler) HandleTokenEndpointRequest(ctx context.Context, requester fosi "error", err, "actor", actorID, ) - return errorsx.WithStack(fosite.ErrInvalidGrant.WithHint( + return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint( "The subject token is invalid or could not be verified.")) } diff --git a/pkg/authserver/server/tokenexchange/handler_test.go b/pkg/authserver/server/tokenexchange/handler_test.go index d60cffc8a0..d5bfb45e03 100644 --- a/pkg/authserver/server/tokenexchange/handler_test.go +++ b/pkg/authserver/server/tokenexchange/handler_test.go @@ -258,7 +258,7 @@ func TestTokenExchangeHandler_HandleTokenEndpointRequest(t *testing.T) { }, lifespan: 15 * time.Minute, wantErr: true, - wantFositeIs: fosite.ErrInvalidGrant, + wantFositeIs: fosite.ErrInvalidRequest, hintContains: "subject token is invalid", }, { diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go index 9a256da46f..15cf594df1 100644 --- a/pkg/authserver/server/tokenexchange/multi_issuer_validator.go +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go @@ -42,7 +42,8 @@ type TrustedIssuer struct { // IssuerURL is the expected "iss" claim value (exact match). IssuerURL string // ExpectedAudience is the expected "aud" claim value that must appear - // in the token's audience list. + // in the token's audience list. Required; NewMultiIssuerTokenValidator + // rejects any TrustedIssuer with an empty ExpectedAudience. ExpectedAudience string // JWKSURL is the URL to fetch the issuer's JSON Web Key Set from. // If empty, it is resolved via OIDC discovery at {IssuerURL}/.well-known/openid-configuration. @@ -80,11 +81,18 @@ type externalIssuerConfig struct { // NewMultiIssuerTokenValidator creates a validator that accepts tokens from the // authorization server itself and from the provided list of trusted external issuers. +// Returns an error if any TrustedIssuer has an empty ExpectedAudience. func NewMultiIssuerTokenValidator( selfValidator *SelfIssuedTokenValidator, selfIssuer string, trustedIssuers []TrustedIssuer, -) *MultiIssuerTokenValidator { +) (*MultiIssuerTokenValidator, error) { + for _, issuer := range trustedIssuers { + if issuer.ExpectedAudience == "" { + return nil, fmt.Errorf("trusted issuer %q: ExpectedAudience is required", issuer.IssuerURL) + } + } + issuers := make(map[string]*externalIssuerConfig, len(trustedIssuers)) for _, ti := range trustedIssuers { issuers[ti.IssuerURL] = &externalIssuerConfig{ @@ -100,7 +108,7 @@ func NewMultiIssuerTokenValidator( httpClient: &http.Client{ Timeout: httpTimeout, }, - } + }, nil } // Validate parses the raw JWT to extract the issuer claim, then routes validation @@ -248,6 +256,7 @@ func (v *MultiIssuerTokenValidator) discoverJWKSURL(ctx context.Context, issuerU defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { + _, _ = io.Copy(io.Discard, resp.Body) return "", fmt.Errorf("discovery endpoint returned status %d", resp.StatusCode) } @@ -309,6 +318,7 @@ func (v *MultiIssuerTokenValidator) fetchJWKS(ctx context.Context, jwksURL strin defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { + _, _ = io.Copy(io.Discard, resp.Body) return nil, fmt.Errorf("JWKS endpoint returned status %d", resp.StatusCode) } diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go index e28089b179..7ffeca2dd8 100644 --- a/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go @@ -105,7 +105,8 @@ func newMultiValidator( selfValidator, err := NewSelfIssuedTokenValidator(selfJWKS.jwks, testIssuer, []string{testIssuer}) require.NoError(t, err) - v := NewMultiIssuerTokenValidator(selfValidator, testIssuer, trustedIssuers) + v, err := NewMultiIssuerTokenValidator(selfValidator, testIssuer, trustedIssuers) + require.NoError(t, err) v.insecureSkipJWKSURLValidation = true // Allow HTTP test servers return v } From 6f06d5432f02bf781c947129b8fd72651ed02388 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 20:36:06 +0200 Subject: [PATCH 4/4] Harden multi-issuer validator, fix review findings Address code-review findings on the multi-issuer subject-token validator. Fix the self-issued routing test, which handed the private JWKS to the validator instead of the public set. Require an "exp" claim on external tokens so the delegated token can be bounded. Validate that the OIDC discovery document's issuer matches the expected issuer. Harden JWKS fetching against SSRF: validate redirects and resolved IPs at dial time (blocking loopback, private, link-local and unspecified addresses), not just the pre-request URL string. Reject empty key sets and cap the accepted key count. Drop dead test helpers and route JWKS handlers through the existing public-JWKS helper. Add tests for the empty-audience constructor error, discovery failures, and kid mismatch. External-token delegation consent, error-code mapping and clock-skew leeway are deferred to #5989 and flagged with TODOs, since the validator is not yet wired into Factory. --- .../server/tokenexchange/handler.go | 6 + .../tokenexchange/multi_issuer_validator.go | 92 +++++++++- .../multi_issuer_validator_test.go | 158 +++++++++++++----- 3 files changed, 208 insertions(+), 48 deletions(-) diff --git a/pkg/authserver/server/tokenexchange/handler.go b/pkg/authserver/server/tokenexchange/handler.go index b346ef334c..a7c0b614ce 100644 --- a/pkg/authserver/server/tokenexchange/handler.go +++ b/pkg/authserver/server/tokenexchange/handler.go @@ -115,6 +115,12 @@ func (h *Handler) HandleTokenEndpointRequest(ctx context.Context, requester fosi "error", err, "actor", actorID, ) + // TODO(#5989): this maps every validation failure to invalid_request, + // which is correct for a malformed/unverifiable subject token. Once the + // multi-issuer validator is wired in, grant-level failures reachable only + // on the external path (untrusted issuer, wrong audience, expired) should + // map to invalid_grant per RFC 6749 §5.2; that needs typed validator + // errors the handler can distinguish. return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint( "The subject token is invalid or could not be verified.")) } diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go index 15cf594df1..da970861fd 100644 --- a/pkg/authserver/server/tokenexchange/multi_issuer_validator.go +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator.go @@ -6,6 +6,7 @@ package tokenexchange import ( "context" "encoding/json" + "errors" "fmt" "io" "net" @@ -31,6 +32,13 @@ const ( // external OIDC endpoints (1 MiB). This prevents resource exhaustion from // unexpectedly large responses. maxResponseBodySize = 1 << 20 + + // maxJWKSKeys caps the number of keys accepted from an external JWKS to + // prevent CPU amplification from a hostile endpoint serving many keys. + maxJWKSKeys = 100 + + // maxRedirects caps redirects followed when fetching external OIDC metadata. + maxRedirects = 5 ) // Compile-time check that MultiIssuerTokenValidator implements SubjectTokenValidator. @@ -57,6 +65,13 @@ type TrustedIssuer struct { // is delegated to the SelfIssuedTokenValidator. For tokens from trusted external // issuers, the validator resolves the issuer's JWKS (via OIDC discovery if needed), // verifies the JWT signature, and validates standard claims. +// +// TODO(#5989): this validator is not yet wired into Factory (which still +// constructs a SelfIssuedTokenValidator), so external subject tokens are not +// reachable in production. External tokens carry no client_id claim, so the +// handler's checkDelegationConsent fails them closed. The external-token +// delegation-consent policy MUST land in the same change that wires this +// validator into Factory — do not enable external issuers without it. type MultiIssuerTokenValidator struct { selfIssuer string selfValidator *SelfIssuedTokenValidator @@ -64,7 +79,9 @@ type MultiIssuerTokenValidator struct { httpClient *http.Client // insecureSkipJWKSURLValidation disables HTTPS enforcement on discovered - // JWKS URLs. This MUST only be set for testing with httptest servers. + // JWKS URLs and relaxes the dial-time IP/scheme checks (so httptest servers + // on loopback over HTTP are reachable). This MUST only be set for testing + // with httptest servers. insecureSkipJWKSURLValidation bool } @@ -101,14 +118,47 @@ func NewMultiIssuerTokenValidator( } } - return &MultiIssuerTokenValidator{ + v := &MultiIssuerTokenValidator{ selfIssuer: selfIssuer, selfValidator: selfValidator, issuers: issuers, - httpClient: &http.Client{ - Timeout: httpTimeout, + } + + v.httpClient = &http.Client{ + Timeout: httpTimeout, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return errors.New("too many redirects") + } + // Re-validate the scheme of each redirect hop; the resolved IP + // is checked in DialContext below. + if !v.insecureSkipJWKSURLValidation && req.URL.Scheme != "https" { + return fmt.Errorf("redirect to non-HTTPS URL: %q", req.URL.Scheme) + } + return nil }, - }, nil + Transport: &http.Transport{ + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, err + } + ips, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return nil, err + } + for _, ipa := range ips { + if !v.insecureSkipJWKSURLValidation && isDisallowedIP(ipa.IP) { + return nil, fmt.Errorf("refusing to connect to disallowed address %s", ipa.IP) + } + } + return (&net.Dialer{Timeout: 5 * time.Second}).DialContext( + ctx, network, net.JoinHostPort(ips[0].String(), port)) + }, + }, + } + + return v, nil } // Validate parses the raw JWT to extract the issuer claim, then routes validation @@ -173,6 +223,12 @@ func (v *MultiIssuerTokenValidator) validateExternalToken( return nil, fmt.Errorf("subject token is missing required 'sub' claim") } + // Expiry is required so the delegated token can be bounded by the subject + // token's remaining lifetime. + if standardClaims.Expiry == nil { + return nil, errors.New("subject token is missing required 'exp' claim") + } + return buildValidatedClaims(standardClaims, extraClaims), nil } @@ -185,6 +241,8 @@ func (v *MultiIssuerTokenValidator) resolveJWKS( ) (*jose.JSONWebKeySet, error) { issuerConfig.mu.Lock() defer issuerConfig.mu.Unlock() + // The lock is intentionally held across the network fetch so concurrent + // validations of the same issuer don't trigger duplicate JWKS fetches. // Return cached JWKS if still valid. if issuerConfig.jwks != nil && time.Now().Before(issuerConfig.jwksExp) { @@ -194,7 +252,7 @@ func (v *MultiIssuerTokenValidator) resolveJWKS( // Cache expired — clear the discovered URL so we re-discover on next fetch. // This handles the (rare) case where an issuer rotates its JWKS endpoint URL. // The explicitly configured JWKSURL (from TrustedIssuer) is preserved. - if issuerConfig.TrustedIssuer.JWKSURL == "" { + if issuerConfig.JWKSURL == "" { issuerConfig.jwksURL = "" } @@ -270,6 +328,10 @@ func (v *MultiIssuerTokenValidator) discoverJWKSURL(ctx context.Context, issuerU return "", fmt.Errorf("failed to parse discovery document: %w", err) } + if doc.Issuer != issuerURL { + return "", fmt.Errorf("discovery document issuer %q does not match expected issuer %q", doc.Issuer, issuerURL) + } + if doc.JWKSURI == "" { return "", fmt.Errorf("discovery document missing 'jwks_uri'") } @@ -297,13 +359,20 @@ func validateJWKSURL(jwksURL string) error { host := u.Hostname() ip := net.ParseIP(host) - if ip != nil && (ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()) { - return fmt.Errorf("must not point to a private or loopback address") + if ip != nil && isDisallowedIP(ip) { + return errors.New("must not point to a private or loopback address") } return nil } +// isDisallowedIP reports whether an IP must not be dialed when fetching +// external OIDC metadata, blocking SSRF to internal/metadata addresses. +func isDisallowedIP(ip net.IP) bool { + return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || ip.IsUnspecified() +} + // fetchJWKS fetches a JSON Web Key Set from the given URL. func (v *MultiIssuerTokenValidator) fetchJWKS(ctx context.Context, jwksURL string) (*jose.JSONWebKeySet, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, jwksURL, nil) @@ -332,5 +401,12 @@ func (v *MultiIssuerTokenValidator) fetchJWKS(ctx context.Context, jwksURL strin return nil, fmt.Errorf("failed to parse JWKS: %w", err) } + if len(jwks.Keys) == 0 { + return nil, errors.New("JWKS contains no keys") + } + if len(jwks.Keys) > maxJWKSKeys { + return nil, fmt.Errorf("JWKS contains too many keys: %d (max %d)", len(jwks.Keys), maxJWKSKeys) + } + return &jwks, nil } diff --git a/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go index 7ffeca2dd8..c357de4105 100644 --- a/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go +++ b/pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/go-jose/go-jose/v4" "github.com/go-jose/go-jose/v4/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,13 +24,6 @@ const ( testExternalAudience = "toolhive-authserver" ) -// newExternalTestJWKS creates a separate JWKS for simulating an external issuer. -// It reuses newTestJWKS but conceptually represents a different signing authority. -func newExternalTestJWKS(t *testing.T) *testJWKS { - t.Helper() - return newTestJWKS(t) -} - // startJWKSServer creates a test HTTP server that serves a JWKS endpoint. // The returned server must be closed by the caller. func startJWKSServer(t *testing.T, tj *testJWKS) *httptest.Server { @@ -38,10 +32,8 @@ func startJWKSServer(t *testing.T, tj *testJWKS) *httptest.Server { mux := http.NewServeMux() mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { // Serve only the public keys. - publicJWKS := publicKeysFrom(t, tj) w.Header().Set("Content-Type", "application/json") - err := json.NewEncoder(w).Encode(publicJWKS) - require.NoError(t, err) + _ = json.NewEncoder(w).Encode(tj.publicJWKS()) }) srv := httptest.NewServer(mux) @@ -49,21 +41,6 @@ func startJWKSServer(t *testing.T, tj *testJWKS) *httptest.Server { return srv } -// publicKeysFrom extracts the public portion of the test JWKS keys. -func publicKeysFrom(t *testing.T, tj *testJWKS) map[string]interface{} { - t.Helper() - keys := make([]map[string]interface{}, 0, len(tj.jwks.Keys)) - for _, key := range tj.jwks.Keys { - pub := key.Public() - raw, err := pub.MarshalJSON() - require.NoError(t, err, "failed to marshal public key") - var m map[string]interface{} - require.NoError(t, json.Unmarshal(raw, &m), "failed to unmarshal public key") - keys = append(keys, m) - } - return map[string]interface{}{"keys": keys} -} - // startDiscoveryServer creates a test HTTP server that serves both OIDC discovery // and JWKS endpoints, simulating an external OIDC provider. func startDiscoveryServer(t *testing.T, tj *testJWKS) *httptest.Server { @@ -71,21 +48,20 @@ func startDiscoveryServer(t *testing.T, tj *testJWKS) *httptest.Server { mux := http.NewServeMux() mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { - // The jwks_uri must use the test server's own base URL, which we - // don't know until the server starts. We use the Host header to - // construct it. - scheme := "http" - jwksURI := scheme + "://" + r.Host + "/jwks" + // The issuer and jwks_uri must use the test server's own base URL, + // which we don't know until the server starts. We use the Host header + // to construct them. The issuer must match the requested issuer URL + // (which the test configures as the discovery server's own URL). + base := "http://" + r.Host w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]string{ - "issuer": testExternalIssuer, - "jwks_uri": jwksURI, + "issuer": base, + "jwks_uri": base + "/jwks", }) }) mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { - publicJWKS := publicKeysFrom(t, tj) w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(publicJWKS) + _ = json.NewEncoder(w).Encode(tj.publicJWKS()) }) srv := httptest.NewServer(mux) @@ -102,7 +78,7 @@ func newMultiValidator( ) *MultiIssuerTokenValidator { t.Helper() - selfValidator, err := NewSelfIssuedTokenValidator(selfJWKS.jwks, testIssuer, []string{testIssuer}) + selfValidator, err := NewSelfIssuedTokenValidator(selfJWKS.publicJWKS(), testIssuer, []string{testIssuer}) require.NoError(t, err) v, err := NewMultiIssuerTokenValidator(selfValidator, testIssuer, trustedIssuers) @@ -129,7 +105,7 @@ func TestMultiIssuerTokenValidator_Validate(t *testing.T) { t.Parallel() selfJWKS := newTestJWKS(t) - externalJWKS := newExternalTestJWKS(t) + externalJWKS := newTestJWKS(t) jwksServer := startJWKSServer(t, externalJWKS) tests := []struct { @@ -323,7 +299,7 @@ func TestMultiIssuerTokenValidator_OIDCDiscovery(t *testing.T) { t.Parallel() selfJWKS := newTestJWKS(t) - externalJWKS := newExternalTestJWKS(t) + externalJWKS := newTestJWKS(t) discoveryServer := startDiscoveryServer(t, externalJWKS) // Configure the trusted issuer WITHOUT a JWKS URL, forcing OIDC discovery. @@ -360,16 +336,15 @@ func TestMultiIssuerTokenValidator_JWKSCaching(t *testing.T) { t.Parallel() selfJWKS := newTestJWKS(t) - externalJWKS := newExternalTestJWKS(t) + externalJWKS := newTestJWKS(t) // Track how many times the JWKS endpoint is hit. var fetchCount atomic.Int32 mux := http.NewServeMux() mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { fetchCount.Add(1) - publicJWKS := publicKeysFrom(t, externalJWKS) w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(publicJWKS) + _ = json.NewEncoder(w).Encode(externalJWKS.publicJWKS()) }) jwksServer := httptest.NewServer(mux) t.Cleanup(jwksServer.Close) @@ -396,3 +371,106 @@ func TestMultiIssuerTokenValidator_JWKSCaching(t *testing.T) { assert.Equal(t, int32(1), fetchCount.Load(), "JWKS should be fetched only once due to caching") } + +func TestNewMultiIssuerTokenValidator_EmptyAudience(t *testing.T) { + t.Parallel() + selfJWKS := newTestJWKS(t) + selfValidator, err := NewSelfIssuedTokenValidator(selfJWKS.publicJWKS(), testIssuer, []string{testIssuer}) + require.NoError(t, err) + _, err = NewMultiIssuerTokenValidator(selfValidator, testIssuer, []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: "", + }}) + require.Error(t, err) + assert.Contains(t, err.Error(), "ExpectedAudience is required") +} + +func TestMultiIssuerTokenValidator_DiscoveryFailures(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + handler func(w http.ResponseWriter, r *http.Request) + }{ + { + name: "non-200", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }, + }, + { + name: "malformed doc", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{not-json")) + }, + }, + { + name: "missing jwks_uri", + handler: func(w http.ResponseWriter, r *http.Request) { + // Issuer must match so discovery reaches the jwks_uri check. + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{ + "issuer": "http://" + r.Host, + }) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + selfJWKS := newTestJWKS(t) + externalJWKS := newTestJWKS(t) + + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", tt.handler) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + trustedIssuers := []TrustedIssuer{{ + IssuerURL: server.URL, + ExpectedAudience: testExternalAudience, + // JWKSURL left empty to force discovery. + }} + validator := newMultiValidator(t, selfJWKS, trustedIssuers) + + claims := externalClaims() + claims.Issuer = server.URL + rawToken := externalJWKS.signToken(t, claims, nil) + + result, err := validator.Validate(context.Background(), rawToken) + require.Error(t, err) + assert.Contains(t, err.Error(), "OIDC discovery failed") + assert.Nil(t, result) + }) + } +} + +func TestMultiIssuerTokenValidator_KidMismatch(t *testing.T) { + t.Parallel() + + selfJWKS := newTestJWKS(t) + externalJWKS := newTestJWKS(t) + jwksServer := startJWKSServer(t, externalJWKS) + + trustedIssuers := []TrustedIssuer{{ + IssuerURL: testExternalIssuer, + ExpectedAudience: testExternalAudience, + JWKSURL: jwksServer.URL + "/jwks", + }} + validator := newMultiValidator(t, selfJWKS, trustedIssuers) + + // Sign with a key whose public half is NOT in the served JWKS and whose + // kid does not match any served key. The kid lookup misses, so the + // validator falls back to trying every served key and fails verification. + unknownKey := newECDSAJWK(t, "unknown-kid") + claims := externalClaims() + rawToken := signWithJWK(t, unknownKey, jose.ES256, claims) + + result, err := validator.Validate(context.Background(), rawToken) + require.Error(t, err) + assert.Contains(t, err.Error(), "signature verification failed") + assert.Nil(t, result) +}