From 72cfce076443d7df5f05a6a40513060b2a227d38 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 22 Apr 2026 23:26:23 +0100 Subject: [PATCH] Wire token exchange handler into fosite Switch createProvider() from compose.Compose to NewAuthorizationServer with factory adapters so the custom RFC 8693 token exchange handler can be registered alongside the standard OAuth grant types. - Add wrapComposeFactory adapter to bridge compose.Factory -> server.Factory - Register tokenexchange.Factory in the extra-factories list - Add DelegationTokenLifespan to Config (default 15m, max 24h) and RunConfig, parsed in runner/embeddedauthserver.go - Add integration and config-validation test coverage Token exchange is intentionally not advertised in discovery metadata yet: it requires a confidential client, and no confidential-client registration path exists on this branch, so advertising it would describe a grant no issuable client can use. Co-Authored-By: Claude Opus 4.8 --- docs/server/docs.go | 4 + docs/server/swagger.json | 4 + docs/server/swagger.yaml | 6 + pkg/authserver/config.go | 54 +++++- pkg/authserver/config_test.go | 76 ++++++++ pkg/authserver/integration_test.go | 204 ++++++++++++++++++++ pkg/authserver/runner/embeddedauthserver.go | 14 +- pkg/authserver/server_impl.go | 113 +++++++---- 8 files changed, 436 insertions(+), 39 deletions(-) diff --git a/docs/server/docs.go b/docs/server/docs.go index 6b4d01e5bc..c330727ed8 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -559,6 +559,10 @@ const docTemplate = `{ "cimd": { "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver.CIMDRunConfig" }, + "delegation_token_lifespan": { + "description": "DelegationTokenLifespan is the maximum lifetime for delegated tokens issued\nvia RFC 8693 token exchange. Specified as a Go duration string (e.g., \"15m\").\nIf empty, defaults to 15 minutes.", + "type": "string" + }, "disable_upstream_token_injection": { "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients, but instead of\ninjecting upstream IdP tokens the proxy strips the client's credential headers\n(Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the\nbackend receives an unauthenticated request. Incompatible with token exchange\nand AWS STS, which would re-add credentials after the strip.", "type": "boolean" diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 2d756d8d62..559e78205c 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -552,6 +552,10 @@ "cimd": { "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver.CIMDRunConfig" }, + "delegation_token_lifespan": { + "description": "DelegationTokenLifespan is the maximum lifetime for delegated tokens issued\nvia RFC 8693 token exchange. Specified as a Go duration string (e.g., \"15m\").\nIf empty, defaults to 15 minutes.", + "type": "string" + }, "disable_upstream_token_injection": { "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients, but instead of\ninjecting upstream IdP tokens the proxy strips the client's credential headers\n(Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the\nbackend receives an unauthenticated request. Incompatible with token exchange\nand AWS STS, which would re-add credentials after the strip.", "type": "boolean" diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index ae4b482f81..7528e01aa4 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -638,6 +638,12 @@ components: uniqueItems: false cimd: $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_authserver.CIMDRunConfig' + delegation_token_lifespan: + description: |- + DelegationTokenLifespan is the maximum lifetime for delegated tokens issued + via RFC 8693 token exchange. Specified as a Go duration string (e.g., "15m"). + If empty, defaults to 15 minutes. + type: string disable_upstream_token_injection: description: |- DisableUpstreamTokenInjection prevents the upstream swap middleware from being added. diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 7a2d073575..72d9cdb67b 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -13,6 +13,7 @@ import ( "strings" "time" + oauthserver "github.com/stacklok/toolhive/pkg/authserver/server" servercrypto "github.com/stacklok/toolhive/pkg/authserver/server/crypto" "github.com/stacklok/toolhive/pkg/authserver/server/handlers" "github.com/stacklok/toolhive/pkg/authserver/server/keys" @@ -63,6 +64,11 @@ type RunConfig struct { // If nil, defaults are applied (access: 1h, refresh: 7d, authCode: 10m). TokenLifespans *TokenLifespanRunConfig `json:"token_lifespans,omitempty" yaml:"token_lifespans,omitempty"` + // DelegationTokenLifespan is the maximum lifetime for delegated tokens issued + // via RFC 8693 token exchange. Specified as a Go duration string (e.g., "15m"). + // If empty, defaults to 15 minutes. + DelegationTokenLifespan string `json:"delegation_token_lifespan,omitempty" yaml:"delegation_token_lifespan,omitempty"` + // Upstreams configures connections to upstream Identity Providers. // At least one upstream is required - the server delegates authentication to these providers. // Multiple upstreams are supported for sequential authorization chains. @@ -616,6 +622,11 @@ type Config struct { // If zero, defaults to 10 minutes. AuthCodeLifespan time.Duration + // DelegationTokenLifespan is the maximum lifetime for delegated tokens issued + // via RFC 8693 token exchange. The actual lifetime is the minimum of this value + // and the subject token's remaining lifetime. If zero, defaults to 15 minutes. + DelegationTokenLifespan time.Duration + // Upstreams contains configurations for connecting to upstream IDPs. // At least one upstream is required - the server delegates authentication to the upstream IDP. // Multiple upstreams form a sequential authorization chain. @@ -730,11 +741,12 @@ func (c *Config) Validate() error { } } - if c.CIMDEnabled && c.CIMDCacheMaxSize < 1 { - return fmt.Errorf("cimd.cache_max_size must be >= 1 when CIMD is enabled") + if err := c.validateCIMDBounds(); err != nil { + return err } - if c.CIMDEnabled && c.CIMDCacheFallbackTTL < 0 { - return fmt.Errorf("cimd.cache_fallback_ttl must be non-negative when CIMD is enabled") + + if err := c.validateDelegationTokenLifespan(); err != nil { + return err } slog.Debug("authserver config validation passed", @@ -744,6 +756,36 @@ func (c *Config) Validate() error { return nil } +// validateCIMDBounds rejects invalid CIMD cache bounds when CIMD is enabled. +// When CIMD is disabled the cache fields are ignored. +func (c *Config) validateCIMDBounds() error { + if !c.CIMDEnabled { + return nil + } + if c.CIMDCacheMaxSize < 1 { + return fmt.Errorf("cimd.cache_max_size must be >= 1 when CIMD is enabled") + } + if c.CIMDCacheFallbackTTL < 0 { + return fmt.Errorf("cimd.cache_fallback_ttl must be non-negative when CIMD is enabled") + } + return nil +} + +// validateDelegationTokenLifespan rejects negative or excessively long delegation +// token lifespans. Capped at oauthserver.MaxAccessTokenLifespan (the same ceiling +// the token-exchange Factory enforces) so validation and construction agree on a +// single source of truth — delegated tokens should be short-lived. Zero is +// accepted; applyDefaults substitutes the default. +func (c *Config) validateDelegationTokenLifespan() error { + if c.DelegationTokenLifespan < 0 { + return fmt.Errorf("delegation token lifespan must not be negative") + } + if c.DelegationTokenLifespan > oauthserver.MaxAccessTokenLifespan { + return fmt.Errorf("delegation token lifespan must not exceed %v", oauthserver.MaxAccessTokenLifespan) + } + return nil +} + // Validate checks that the OAuth2UpstreamRunConfig is internally consistent. // It enforces the mutual exclusivity of ClientID and DCRConfig: exactly one must // be set. A ClientID is required for pre-provisioned clients; a DCRConfig is @@ -961,6 +1003,10 @@ func (c *Config) applyDefaults() error { c.AuthCodeLifespan = 10 * time.Minute slog.Debug("applied default auth code lifespan", "duration", c.AuthCodeLifespan) } + if c.DelegationTokenLifespan == 0 { + c.DelegationTokenLifespan = 15 * time.Minute + slog.Debug("applied default delegation token lifespan", "duration", c.DelegationTokenLifespan) + } if c.HMACSecrets == nil { secret := make([]byte, servercrypto.MinSecretLength) if _, err := rand.Read(secret); err != nil { diff --git a/pkg/authserver/config_test.go b/pkg/authserver/config_test.go index 850a7d0347..7487b759fb 100644 --- a/pkg/authserver/config_test.go +++ b/pkg/authserver/config_test.go @@ -669,3 +669,79 @@ func TestConfigApplyDefaults_CIMD(t *testing.T) { }) } } + +// TestConfigValidate_DelegationTokenLifespan covers the RFC 8693 delegation +// token lifespan bounds added to Config.Validate: zero is accepted (it is +// defaulted later by applyDefaults), values in (0, 24h] are accepted, and +// negative or over-24h values are rejected. +func TestConfigValidate_DelegationTokenLifespan(t *testing.T) { + t.Parallel() + + // base returns a minimally-valid Config so each case isolates the + // DelegationTokenLifespan check from unrelated validation failures. + base := func() Config { + return Config{ + Issuer: "https://example.com", + KeyProvider: keys.NewGeneratingProvider(keys.DefaultAlgorithm), + HMACSecrets: &servercrypto.HMACSecrets{Current: make([]byte, 32)}, + Upstreams: []UpstreamConfig{{ + Name: "default", + Type: UpstreamProviderTypeOAuth2, + OAuth2Config: &upstream.OAuth2Config{ + CommonOAuthConfig: upstream.CommonOAuthConfig{ClientID: "c", RedirectURI: "https://example.com/cb"}, + AuthorizationEndpoint: "https://idp.example.com/authorize", + TokenEndpoint: "https://idp.example.com/token", + }, + }}, + AllowedAudiences: []string{"https://mcp.example.com"}, + } + } + + tests := []struct { + name string + lifespan time.Duration + wantErr bool + errMsg string + }{ + {name: "zero accepted (defaulted later)", lifespan: 0}, + {name: "valid 15m", lifespan: 15 * time.Minute}, + {name: "valid 1h", lifespan: time.Hour}, + {name: "valid 24h boundary", lifespan: 24 * time.Hour}, + {name: "negative rejected", lifespan: -time.Second, wantErr: true, errMsg: "delegation token lifespan must not be negative"}, + {name: "over 24h rejected", lifespan: 24*time.Hour + time.Second, wantErr: true, errMsg: "delegation token lifespan must not exceed 24h"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := base() + cfg.DelegationTokenLifespan = tt.lifespan + assertError(t, cfg.Validate(), tt.wantErr, tt.errMsg) + }) + } +} + +// TestConfigApplyDefaults_DelegationTokenLifespan verifies that applyDefaults +// fills a zero DelegationTokenLifespan with the 15-minute default and preserves +// a caller-supplied value. +func TestConfigApplyDefaults_DelegationTokenLifespan(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input time.Duration + want time.Duration + }{ + {name: "zero gets 15m default", input: 0, want: 15 * time.Minute}, + {name: "custom value preserved", input: 5 * time.Minute, want: 5 * time.Minute}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := Config{Issuer: "https://example.com", DelegationTokenLifespan: tt.input} + require.NoError(t, cfg.applyDefaults()) + require.Equal(t, tt.want, cfg.DelegationTokenLifespan) + }) + } +} diff --git a/pkg/authserver/integration_test.go b/pkg/authserver/integration_test.go index 73c2bdbc6c..0c5f20e214 100644 --- a/pkg/authserver/integration_test.go +++ b/pkg/authserver/integration_test.go @@ -38,6 +38,7 @@ import ( "github.com/stacklok/toolhive/pkg/authserver/server/session" "github.com/stacklok/toolhive/pkg/authserver/storage" "github.com/stacklok/toolhive/pkg/authserver/upstream" + "github.com/stacklok/toolhive/pkg/oauthproto" ) const ( @@ -95,6 +96,11 @@ type testServerOptions struct { // which delegates to setupTestServer. setupTestServerWithRTProxy does not // accept testServerOption at all, so this field does not apply to it. upstreamFilter handlers.UpstreamFilter + // extraClients, when non-empty, are registered in storage in addition to the + // default public PKCE test client. Used to install a confidential client for + // flows the default public client cannot exercise (e.g. RFC 8693 token + // exchange, which requires a confidential acting client). + extraClients []fosite.Client } // testServerOption is a functional option for test server setup. @@ -130,6 +136,14 @@ func withUpstreamFilter(f handlers.UpstreamFilter) testServerOption { } } +// withExtraClient registers an additional client in storage alongside the +// default public PKCE test client. +func withExtraClient(c fosite.Client) testServerOption { + return func(opts *testServerOptions) { + opts.extraClients = append(opts.extraClients, c) + } +} + // withRedisBackedStorage swaps the default in-memory storage for a // miniredis-backed *RedisStorage. This exercises the same Lua scripts and // Redis-shape key layout used in production, while remaining hermetic and @@ -226,6 +240,11 @@ func setupTestServer(t *testing.T, opts ...testServerOption) *testServer { }) require.NoError(t, err) + // Register any extra clients (e.g. a confidential client for token exchange). + for _, c := range options.extraClients { + require.NoError(t, stor.RegisterClient(ctx, c)) + } + // 5. Build upstream config for newServer // When no upstream is provided, use a dummy config that satisfies validation // Note: Uses HTTPS to pass config validation @@ -585,6 +604,191 @@ func TestIntegration_TokenEndpoint_RefreshToken(t *testing.T) { require.GreaterOrEqual(t, replayResp.StatusCode, 400, "old refresh token must be rejected after rotation") } +// ============================================================================ +// RFC 8693 Token Exchange Wiring Tests +// ============================================================================ + +// TestIntegration_TokenExchange_PublicClientRejected proves that public clients +// are barred from the RFC 8693 token-exchange grant (only confidential clients +// may act on a user's behalf), and that the grant is wired into the fosite +// provider and reachable at the token endpoint. +// +// It does not assert a full delegated-token issuance: the handler requires a +// confidential client (RFC 8693 §2.1) and the shared test harness only +// registers a public client. Instead it relies on a decisive dispatch +// discriminator observable at the token endpoint: +// +// - With the token-exchange factory registered, a token-exchange request is +// routed to the handler, whose first guard rejects the public client with +// error=invalid_grant and a "token-exchange"-specific hint. +// - Without the factory, no handler claims grant_type=token-exchange and +// fosite returns error=invalid_request (see fosite NewAccessRequest: an +// unmatched grant yields ErrInvalidRequest). +// +// So invalid_grant with a token-exchange hint proves the handler is registered +// and executed — exactly the regression a dropped factory would introduce. +func TestIntegration_TokenExchange_PublicClientRejected(t *testing.T) { + t.Parallel() + + m := startMockOIDC(t) + ts := setupTestServerWithMockOIDC(t, m) + + verifier := servercrypto.GeneratePKCEVerifier() + challenge := servercrypto.ComputePKCEChallenge(verifier) + + // Mint a genuine server-issued access token to use as the subject_token, so + // the request is a well-formed RFC 8693 exchange up to the client-type gate. + authCode, _ := completeAuthorizationFlow(t, ts.Server.URL, authorizationParams{ + ClientID: testClientID, + RedirectURI: testRedirectURI, + State: "token-exchange-dispatch", + Challenge: challenge, + Scope: "openid profile", + ResponseType: "code", + }) + tokenData := exchangeCodeForTokens(t, ts.Server.URL, authCode, verifier, testAudience) + subjectToken, ok := tokenData["access_token"].(string) + require.True(t, ok, "access_token should be a string") + require.NotEmpty(t, subjectToken) + + resp := makeTokenRequest(t, ts.Server.URL, url.Values{ + "grant_type": {oauthproto.GrantTypeTokenExchange}, + "subject_token": {subjectToken}, + "subject_token_type": {oauthproto.TokenTypeAccessToken}, + "client_id": {testClientID}, + }) + defer resp.Body.Close() + + body := parseTokenResponse(t, resp) + errCode, _ := body["error"].(string) + errDesc, _ := body["error_description"].(string) + + require.Equal(t, http.StatusBadRequest, resp.StatusCode, + "token-exchange from a public client should be a 400, got %d (body: %v)", resp.StatusCode, body) + // The decisive assertion: the token-exchange handler ran and rejected the + // public client (invalid_grant), rather than the request falling through + // unhandled (invalid_request), which is what a missing factory would produce. + require.Equal(t, "invalid_grant", errCode, + "expected invalid_grant from the token-exchange handler; invalid_request would mean "+ + "the token-exchange factory is not wired into the provider") + assert.Contains(t, errDesc, "token-exchange", + "the rejection must originate from the token-exchange handler specifically") +} + +// TestIntegration_TokenExchange_ConfidentialClientHappyPath drives a full RFC 8693 +// delegation exchange over HTTP through the real fosite provider: a confidential +// acting client authenticates with client_secret_post, presents a server-signed +// subject token, and receives a delegated access token. +// +// Unlike the unit tests in the tokenexchange package (which call the handler +// directly with mock strategy/storage), this exercises the complete glued path — +// fosite NewAccessRequest -> client authentication -> handler dispatch -> +// PopulateTokenEndpointResponse JSON issuance — proving the token exchange is not +// just registered but functional end-to-end. +func TestIntegration_TokenExchange_ConfidentialClientHappyPath(t *testing.T) { + t.Parallel() + + const ( + agentClientID = "test-agent-client" + agentClientSecret = "test-agent-secret" + delegatedUserSub = "delegated-user-sub" + ) + + // A confidential client registered for the token-exchange grant is the acting + // agent. The handler rejects public clients, so this must be confidential. + agentClient, err := registration.New(registration.Config{ + ID: agentClientID, + Secret: agentClientSecret, + Public: false, + GrantTypes: []string{oauthproto.GrantTypeTokenExchange}, + Scopes: registration.DefaultScopes, + Audience: []string{testAudience}, + }) + require.NoError(t, err) + + m := startMockOIDC(t) + ts := setupTestServerWithMockOIDC(t, m, withExtraClient(agentClient)) + + // Mint a subject token signed by the server's own key (the validator verifies + // against the server's JWKS). client_id must equal the acting client so the + // RFC 8693 §4.1 delegation-consent check (client_id binding) passes. + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: ts.PrivateKey}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", "test-key"), + ) + require.NoError(t, err) + + now := time.Now() + subjectToken, err := jwt.Signed(signer). + Claims(jwt.Claims{ + Issuer: testIssuer, + Subject: delegatedUserSub, + Audience: jwt.Audience{testAudience}, + Expiry: jwt.NewNumericDate(now.Add(30 * time.Minute)), + IssuedAt: jwt.NewNumericDate(now), + }). + Claims(map[string]any{ + "client_id": agentClientID, + "name": "Delegated User", + "email": "deleg@example.com", + }). + Serialize() + require.NoError(t, err) + + // No resource/audience is sent: the server defaults to its sole allowed + // audience (testAudience), which the agent client is registered for. + resp := makeTokenRequest(t, ts.Server.URL, url.Values{ + "grant_type": {oauthproto.GrantTypeTokenExchange}, + "subject_token": {subjectToken}, + "subject_token_type": {oauthproto.TokenTypeAccessToken}, + "client_id": {agentClientID}, + "client_secret": {agentClientSecret}, + }) + defer resp.Body.Close() + + body := parseTokenResponse(t, resp) + require.Equal(t, http.StatusOK, resp.StatusCode, + "token exchange should succeed, got %d (body: %v)", resp.StatusCode, body) + + // RFC 8693 §2.2.1 requires issued_token_type in the response. + assert.Equal(t, oauthproto.TokenTypeAccessToken, body["issued_token_type"], + "response must advertise the issued token type") + + delegated, ok := body["access_token"].(string) + require.True(t, ok, "access_token should be a string") + require.NotEmpty(t, delegated) + + // The delegated token is a JWT signed by the server; verify and inspect claims. + parsed, err := jwt.ParseSigned(delegated, []jose.SignatureAlgorithm{jose.RS256}) + require.NoError(t, err) + var claims map[string]any + require.NoError(t, parsed.Claims(ts.PrivateKey.Public(), &claims)) + + // The delegated token carries the USER as subject and the AGENT as the + // RFC 8693 §4.1 "act" (acting party). + assert.Equal(t, delegatedUserSub, claims["sub"], "delegated token subject must be the user") + assert.Equal(t, testIssuer, claims["iss"]) + act, ok := claims["act"].(map[string]any) + require.True(t, ok, "delegated token must carry an 'act' claim") + assert.Equal(t, agentClientID, act["sub"], "act.sub must identify the acting agent client") + + // Audience: no resource/audience was requested, so grantDefaultAudience must + // bind the token to the server's sole allowed audience. + aud, ok := claims["aud"].([]interface{}) + require.True(t, ok, "aud claim should be an array") + require.Len(t, aud, 1, "aud should have exactly one audience") + assert.Equal(t, testAudience, aud[0], "delegated token audience should default to the sole allowed audience") + + // Lifetime cap: the delegated token's exp must be min(subject_remaining=30m, + // delegationLifespan=15m default) ≈ 15m — NOT the subject token's 30m. This + // verifies the handler's min() cap and rules out a "subject token echoed + // back" regression, which would otherwise satisfy sub/iss/signature. + exp, ok := claims["exp"].(float64) + require.True(t, ok, "exp claim should be a number") + assert.WithinDuration(t, now.Add(15*time.Minute), time.Unix(int64(exp), 0), 2*time.Minute, + "delegated token exp must be capped at the 15m delegation lifespan, not the subject token's 30m") +} + // ============================================================================ // Full PKCE Flow Integration Tests with Mock Upstream IDP (using mockoidc) // ============================================================================ diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index 70102ee937..4431ad0796 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -204,7 +204,16 @@ func NewEmbeddedAuthServerWithStorage( return nil, fmt.Errorf("failed to build upstream configs: %w", err) } - // 6. Build the resolved Config. + // 6. Parse delegation token lifespan if configured. + var delegationLifespan time.Duration + if cfg.DelegationTokenLifespan != "" { + delegationLifespan, err = time.ParseDuration(cfg.DelegationTokenLifespan) + if err != nil { + return nil, fmt.Errorf("invalid delegation token lifespan: %w", err) + } + } + + // 7. Build the resolved Config. // // Defensive copies of the scope/audience slices: cfg is operator-supplied // input that may be retained or mutated by the caller (e.g. tests, a @@ -223,6 +232,7 @@ func NewEmbeddedAuthServerWithStorage( AccessTokenLifespan: accessLifespan, RefreshTokenLifespan: refreshLifespan, AuthCodeLifespan: authCodeLifespan, + DelegationTokenLifespan: delegationLifespan, Upstreams: upstreams, ScopesSupported: slices.Clone(cfg.ScopesSupported), BaselineClientScopes: slices.Clone(cfg.BaselineClientScopes), @@ -233,7 +243,7 @@ func NewEmbeddedAuthServerWithStorage( InsecureAllowHTTP: cfg.InsecureAllowHTTP, } - // 7. Create the auth server. authserver.New also asserts the DCR + // 8. Create the auth server. authserver.New also asserts the DCR // capability internally so its DCRStore() accessor returns the same // asserted handle this constructor used for buildUpstreamConfigs. server, err := authserver.New(ctx, resolvedCfg, stor) diff --git a/pkg/authserver/server_impl.go b/pkg/authserver/server_impl.go index 007446ce12..c218486fde 100644 --- a/pkg/authserver/server_impl.go +++ b/pkg/authserver/server_impl.go @@ -16,6 +16,7 @@ import ( oauthserver "github.com/stacklok/toolhive/pkg/authserver/server" "github.com/stacklok/toolhive/pkg/authserver/server/handlers" + "github.com/stacklok/toolhive/pkg/authserver/server/tokenexchange" "github.com/stacklok/toolhive/pkg/authserver/storage" "github.com/stacklok/toolhive/pkg/authserver/upstream" ) @@ -177,28 +178,17 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se // Wrap storage with the CIMD decorator before constructing the fosite provider // so that GetClient calls for HTTPS client_id values are intercepted at the // fosite level (not just the handler level). - if cfg.CIMDEnabled { - if len(cfg.BaselineClientScopes) > 0 { - slog.Warn("CIMD is enabled with baseline_client_scopes configured; "+ - "any third-party client resolved via CIMD will also receive these scopes — "+ - "ensure they are scopes you would grant by default to any unknown client", - "baseline_client_scopes", cfg.BaselineClientScopes) - } - stor, err = storage.NewCIMDStorageDecorator(stor, storage.CIMDDecoratorConfig{ - Enabled: true, - CacheMaxSize: cfg.CIMDCacheMaxSize, - FallbackTTL: cfg.CIMDCacheFallbackTTL, - ScopesSupported: cfg.ScopesSupported, - BaselineClientScopes: cfg.BaselineClientScopes, - }) - if err != nil { - return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err) - } + stor, err = decorateStorageForCIMD(cfg, stor) + if err != nil { + return nil, err } // Create fosite provider with the (possibly decorated) storage. slog.Debug("creating fosite OAuth2 provider") - fositeProvider := createProvider(authServerConfig, stor) + fositeProvider, err := buildProvider(cfg, authServerConfig, stor) + if err != nil { + return nil, fmt.Errorf("failed to create fosite OAuth2 provider: %w", err) + } // Give the handler a refresher so the authorization chain can transparently // refresh an expired upstream leg during login instead of skipping it and @@ -228,6 +218,44 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se }, nil } +// decorateStorageForCIMD wraps stor with the CIMD decorator when CIMD is enabled, +// so GetClient calls for HTTPS client_id values are intercepted at the fosite +// level (not just the handler level). Returns stor unchanged when CIMD is disabled. +func decorateStorageForCIMD(cfg Config, stor storage.Storage) (storage.Storage, error) { + if !cfg.CIMDEnabled { + return stor, nil + } + if len(cfg.BaselineClientScopes) > 0 { + slog.Warn("CIMD is enabled with baseline_client_scopes configured; "+ + "any third-party client resolved via CIMD will also receive these scopes — "+ + "ensure they are scopes you would grant by default to any unknown client", + "baseline_client_scopes", cfg.BaselineClientScopes) + } + decorated, err := storage.NewCIMDStorageDecorator(stor, storage.CIMDDecoratorConfig{ + Enabled: true, + CacheMaxSize: cfg.CIMDCacheMaxSize, + FallbackTTL: cfg.CIMDCacheFallbackTTL, + ScopesSupported: cfg.ScopesSupported, + BaselineClientScopes: cfg.BaselineClientScopes, + }) + if err != nil { + return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err) + } + return decorated, nil +} + +// buildProvider assembles the fosite OAuth2 provider, registering the RFC 8693 +// token-exchange handler as an extension grant alongside the standard grants. +func buildProvider( + cfg Config, authServerConfig *oauthserver.AuthorizationServerConfig, stor storage.Storage, +) (fosite.OAuth2Provider, error) { + tokenExchangeFactory, err := tokenexchange.Factory(cfg.DelegationTokenLifespan) + if err != nil { + return nil, fmt.Errorf("failed to create token exchange factory: %w", err) + } + return createProvider(authServerConfig, stor, tokenExchangeFactory) +} + // buildHandlerOptions assembles the handlers.Option list for NewHandler: the // refresher is always wired, and the filter is added only when the caller's // Config sets one so a nil Config.UpstreamFilter preserves the pre-filter @@ -294,9 +322,10 @@ func (s *server) Close() error { // createProvider creates a fosite OAuth2Provider configured for the authorization code flow. // -// Fosite is an OAuth 2.0 framework that implements the protocol details. The compose package -// provides a builder pattern to wire together configuration, storage, token strategies, -// and grant type handlers into a single OAuth2Provider that can handle all OAuth endpoints. +// Fosite is an OAuth 2.0 framework that implements the protocol details. We use +// server.NewAuthorizationServer which accepts server.Factory functions to register +// grant type handlers. The standard compose factories are wrapped via wrapComposeFactory +// and any extra factories (e.g., token exchange) are appended. // // The provider is configured with: // - JWT strategy for access tokens (asymmetric signing, distributed validation via JWKS) @@ -304,7 +333,12 @@ func (s *server) Close() error { // - Authorization code grant (RFC 6749 Section 4.1) // - Refresh token grant (RFC 6749 Section 6) // - PKCE (RFC 7636) for public client security -func createProvider(authServerConfig *oauthserver.AuthorizationServerConfig, stor storage.Storage) fosite.OAuth2Provider { +// - Any extra factories passed in (e.g., RFC 8693 token exchange) +func createProvider( + authServerConfig *oauthserver.AuthorizationServerConfig, + stor storage.Storage, + extraFactories ...oauthserver.Factory, +) (fosite.OAuth2Provider, error) { slog.Debug("configuring fosite OAuth2 provider", "key_id", authServerConfig.SigningKey.KeyID, "algorithm", authServerConfig.SigningKey.Algorithm, @@ -334,18 +368,21 @@ func createProvider(authServerConfig *oauthserver.AuthorizationServerConfig, sto authServerConfig.Config, ) - // compose.Compose wires together all the pieces into an OAuth2Provider: - // - Config: token lifespans, issuer URL, HMAC secret - // - Storage: where to persist authorization codes, tokens, and client data - // - Strategy: how to generate and validate tokens - // - Factories: which OAuth grant types to enable (each adds handlers for specific flows) - return compose.Compose( - authServerConfig.Config, + commonStrategy := &compose.CommonStrategy{CoreStrategy: jwtStrategy} + + // Wrap fosite's compose factories to match server.Factory signature. + factories := []oauthserver.Factory{ + wrapComposeFactory(compose.OAuth2AuthorizeExplicitFactory), // Authorization code grant + wrapComposeFactory(compose.OAuth2RefreshTokenGrantFactory), // Refresh token grant + wrapComposeFactory(compose.OAuth2PKCEFactory), // PKCE for public clients + } + factories = append(factories, extraFactories...) + + return oauthserver.NewAuthorizationServer( + authServerConfig, stor, - &compose.CommonStrategy{CoreStrategy: jwtStrategy}, - compose.OAuth2AuthorizeExplicitFactory, // Authorization code grant - compose.OAuth2RefreshTokenGrantFactory, // Refresh token grant - compose.OAuth2PKCEFactory, // PKCE for public clients + commonStrategy, + factories..., ) } @@ -376,3 +413,13 @@ func runLegacyMigration(ctx context.Context, stor storage.Storage, upstreams []U } return nil } + +// wrapComposeFactory adapts a compose.Factory to a server.Factory. +// Compose factories take (fosite.Configurator, interface{}, interface{}) while +// server factories take (*AuthorizationServerConfig, fosite.Storage, any). +// The embedded *fosite.Config satisfies fosite.Configurator. +func wrapComposeFactory(cf compose.Factory) oauthserver.Factory { + return func(config *oauthserver.AuthorizationServerConfig, storage fosite.Storage, strategy any) (any, error) { + return cf(config.Config, storage, strategy), nil + } +}