From 46eb00528fdb3f414a4fd93ce537cf5387b51b7e Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Tue, 25 Aug 2026 19:59:11 +0100 Subject: [PATCH] feat(authserver): support additional token-request params (RFC 8707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some authorization servers enforce RFC 8707 resource indicators on token requests as well as authorization requests: the code exchange and refresh are rejected with invalid_target unless the resource parameter is present in the POST form body (query-string placement is ignored). Nominal's MCP authorization server (api.gov.nominal.io) is a live example — with only additionalAuthorizationParams, the flow passes authorization and then fails at the code exchange. Add AdditionalTokenParams alongside AdditionalAuthorizationParams: - upstream.CommonOAuthConfig gains AdditionalTokenParams, applied in BaseOAuth2Provider.exchangeCodeForTokens and RefreshTokens via oauth2.SetAuthURLParam options (which land in the POST form body on Exchange). OIDC providers inherit both paths through the embedded base provider. - Reserved-parameter validation mirrors the authorization-side list with token-request semantics: grant_type, code, redirect_uri, client_id, client_secret, code_verifier, refresh_token, and scope are rejected. - CRD: additionalTokenParams on both oidcConfig and oauth2Config upstream provider types, plumbed through the operator run-config builders and validated at reconcile time (MCPExternalAuthConfig and VirtualMCPServer), matching the additionalAuthorizationParams treatment. - Regenerated deepcopy, CRD manifests, and CRD API docs. Co-Authored-By: Claude Fable 5 Signed-off-by: Aron Gates --- .../v1beta1/mcpexternalauthconfig_types.go | 52 +++++- .../mcpexternalauthconfig_types_test.go | 66 ++++++++ .../api/v1beta1/zz_generated.deepcopy.go | 14 ++ .../virtualmcpserver_controller.go | 9 +- .../pkg/controllerutil/authserver.go | 2 + ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 56 +++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 56 +++++++ ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 56 +++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 56 +++++++ docs/operator/crd-api.md | 2 + pkg/authserver/config.go | 12 ++ pkg/authserver/oauthparams/reserved.go | 26 +++ pkg/authserver/runner/embeddedauthserver.go | 2 + pkg/authserver/upstream/oauth2.go | 26 +++ pkg/authserver/upstream/oauth2_test.go | 157 ++++++++++++++++++ 15 files changed, 589 insertions(+), 3 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index 43a64067fc..37e81405a8 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -995,6 +995,18 @@ type OIDCUpstreamConfig struct { // +optional AdditionalAuthorizationParams map[string]string `json:"additionalAuthorizationParams,omitempty"` + // AdditionalTokenParams are extra form-body parameters to include in + // token requests (authorization code exchange and refresh) sent to the + // upstream provider's token endpoint. + // This is useful for providers that enforce RFC 8707 resource indicators + // on token requests, where the resource parameter must accompany the code + // exchange and refresh, not only the authorization request. + // Framework-managed parameters (grant_type, code, redirect_uri, client_id, + // client_secret, code_verifier, refresh_token, scope) are not allowed. + // +kubebuilder:validation:MaxProperties=16 + // +optional + AdditionalTokenParams map[string]string `json:"additionalTokenParams,omitempty"` + // SubjectClaim names the validated ID-token claim to use as the upstream // subject. Defaults to "sub" when empty. Set it for IdPs where "sub" isn't // stable per user — e.g. Entra/Azure AD, whose "sub" rotates per application @@ -1128,6 +1140,18 @@ type OAuth2UpstreamConfig struct { // +optional AdditionalAuthorizationParams map[string]string `json:"additionalAuthorizationParams,omitempty"` + // AdditionalTokenParams are extra form-body parameters to include in + // token requests (authorization code exchange and refresh) sent to the + // upstream provider's token endpoint. + // This is useful for providers that enforce RFC 8707 resource indicators + // on token requests, where the resource parameter must accompany the code + // exchange and refresh, not only the authorization request. + // Framework-managed parameters (grant_type, code, redirect_uri, client_id, + // client_secret, code_verifier, refresh_token, scope) are not allowed. + // +kubebuilder:validation:MaxProperties=16 + // +optional + AdditionalTokenParams map[string]string `json:"additionalTokenParams,omitempty"` + // InsecureAllowHTTP permits plain-HTTP authorization and token endpoint URLs // for this upstream. Only for in-cluster development environments (e.g. an // OAuth2 provider served over HTTP in a kind cluster) where TLS is not @@ -2070,7 +2094,12 @@ func (*MCPExternalAuthConfig) validateUpstreamProvider(index int, provider *Upst } // Validate additionalAuthorizationParams does not contain reserved keys - return ValidateAdditionalAuthorizationParams(prefix, provider.AdditionalAuthorizationParams()) + if err := ValidateAdditionalAuthorizationParams(prefix, provider.AdditionalAuthorizationParams()); err != nil { + return err + } + + // Validate additionalTokenParams does not contain reserved keys + return ValidateAdditionalTokenParams(prefix, provider.AdditionalTokenParams()) } // Length caps for DCR-related string fields. Mirror the @@ -2165,6 +2194,18 @@ func (p *UpstreamProviderConfig) AdditionalAuthorizationParams() map[string]stri return nil } +// AdditionalTokenParams returns the additional token-request parameters +// from whichever upstream config is set, or nil if none. +func (p *UpstreamProviderConfig) AdditionalTokenParams() map[string]string { + if p.OIDCConfig != nil { + return p.OIDCConfig.AdditionalTokenParams + } + if p.OAuth2Config != nil { + return p.OAuth2Config.AdditionalTokenParams + } + return nil +} + // SyntheticIdentityUpstreams returns the names of OAuth2 upstreams running // in synthesis mode (neither userInfo nor identityFromToken configured), // sorted lexically for deterministic condition messages. OIDC upstreams are @@ -2199,6 +2240,15 @@ func ValidateAdditionalAuthorizationParams(prefix string, params map[string]stri return nil } +// ValidateAdditionalTokenParams checks that no reserved OAuth2 token-request +// parameters are present in the additional token params map. +func ValidateAdditionalTokenParams(prefix string, params map[string]string) error { + if err := oauthparams.ValidateTokenParams(params); err != nil { + return fmt.Errorf("%s.additionalTokenParams: %w", prefix, err) + } + return nil +} + // validateAWSSts validates awsSts type configuration. // This performs complex business logic validation that CEL cannot express. func (r *MCPExternalAuthConfig) validateAWSSts() error { diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go index 9deffecf9e..0f9d7d5f2f 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go @@ -906,6 +906,72 @@ func TestMCPExternalAuthConfig_validateUpstreamProvider(t *testing.T) { }, expectErr: false, }, + { + name: "OIDC provider with valid additionalTokenParams", + provider: UpstreamProviderConfig{ + Name: "nominal", + Type: UpstreamProviderTypeOIDC, + OIDCConfig: &OIDCUpstreamConfig{ + IssuerURL: "https://idp.example.com", + ClientID: "client-id", + AdditionalTokenParams: map[string]string{ + "resource": "https://api.example.com/mcp", + }, + }, + }, + expectErr: false, + }, + { + name: "OAuth2 provider with valid additionalTokenParams", + provider: UpstreamProviderConfig{ + Name: "nominal", + Type: UpstreamProviderTypeOAuth2, + OAuth2Config: &OAuth2UpstreamConfig{ + AuthorizationEndpoint: "https://oauth.example.com/authorize", + TokenEndpoint: "https://oauth.example.com/token", + ClientID: "client-id", + UserInfo: &UserInfoConfig{EndpointURL: "https://oauth.example.com/userinfo"}, + AdditionalTokenParams: map[string]string{ + "resource": "https://api.example.com/mcp", + }, + }, + }, + expectErr: false, + }, + { + name: "OAuth2 provider with reserved token param grant_type", + provider: UpstreamProviderConfig{ + Name: "custom", + Type: UpstreamProviderTypeOAuth2, + OAuth2Config: &OAuth2UpstreamConfig{ + AuthorizationEndpoint: "https://oauth.example.com/authorize", + TokenEndpoint: "https://oauth.example.com/token", + ClientID: "client-id", + UserInfo: &UserInfoConfig{EndpointURL: "https://oauth.example.com/userinfo"}, + AdditionalTokenParams: map[string]string{ + "grant_type": "password", + }, + }, + }, + expectErr: true, + errMsg: "reserved token parameter \"grant_type\" is managed by the framework", + }, + { + name: "OIDC provider with reserved token param refresh_token", + provider: UpstreamProviderConfig{ + Name: "custom", + Type: UpstreamProviderTypeOIDC, + OIDCConfig: &OIDCUpstreamConfig{ + IssuerURL: "https://idp.example.com", + ClientID: "client-id", + AdditionalTokenParams: map[string]string{ + "refresh_token": "override-attempt", + }, + }, + }, + expectErr: true, + errMsg: "reserved token parameter \"refresh_token\" is managed by the framework", + }, { name: "OAuth2 provider with valid DCRConfig (discoveryUrl only)", provider: UpstreamProviderConfig{ diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index 0813e3c099..4037cae9d8 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -2324,6 +2324,13 @@ func (in *OAuth2UpstreamConfig) DeepCopyInto(out *OAuth2UpstreamConfig) { (*out)[key] = val } } + if in.AdditionalTokenParams != nil { + in, out := &in.AdditionalTokenParams, &out.AdditionalTokenParams + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.DCRConfig != nil { in, out := &in.DCRConfig, &out.DCRConfig *out = new(DCRUpstreamConfig) @@ -2396,6 +2403,13 @@ func (in *OIDCUpstreamConfig) DeepCopyInto(out *OIDCUpstreamConfig) { (*out)[key] = val } } + if in.AdditionalTokenParams != nil { + in, out := &in.AdditionalTokenParams, &out.AdditionalTokenParams + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCUpstreamConfig. diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller.go b/cmd/thv-operator/controllers/virtualmcpserver_controller.go index 8f475dc184..8a887ec224 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller.go @@ -610,11 +610,16 @@ func (*VirtualMCPServerReconciler) validateAuthServerConfig( return stderrors.New(message) } - // Validate additionalAuthorizationParams on each upstream provider + // Validate additionalAuthorizationParams / additionalTokenParams on each + // upstream provider for i := range cfg.UpstreamProviders { prefix := fmt.Sprintf("spec.authServerConfig.upstreamProviders[%d]", i) params := cfg.UpstreamProviders[i].AdditionalAuthorizationParams() - if err := mcpv1beta1.ValidateAdditionalAuthorizationParams(prefix, params); err != nil { + err := mcpv1beta1.ValidateAdditionalAuthorizationParams(prefix, params) + if err == nil { + err = mcpv1beta1.ValidateAdditionalTokenParams(prefix, cfg.UpstreamProviders[i].AdditionalTokenParams()) + } + if err != nil { message := err.Error() statusManager.SetPhase(mcpv1beta1.VirtualMCPServerPhaseFailed) statusManager.SetMessage(message) diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 8a9dcaade1..4427bb8a7d 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -986,6 +986,7 @@ func buildOIDCUpstreamRunConfig( RedirectURI: redirectURI, Scopes: cfg.Scopes, AdditionalAuthorizationParams: cfg.AdditionalAuthorizationParams, + AdditionalTokenParams: cfg.AdditionalTokenParams, SubjectClaim: cfg.SubjectClaim, } if cfg.ClientSecretRef != nil { @@ -1031,6 +1032,7 @@ func buildOAuth2UpstreamRunConfig( RedirectURI: redirectURI, Scopes: cfg.Scopes, AdditionalAuthorizationParams: cfg.AdditionalAuthorizationParams, + AdditionalTokenParams: cfg.AdditionalTokenParams, } if cfg.ClientSecretRef != nil { runConfig.ClientSecretEnvVar = clientSecretEnvVar diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 0401f12f26..03d15a0c6e 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1023,6 +1023,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -1339,6 +1353,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. @@ -2996,6 +3024,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -3312,6 +3354,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index c9d92c0a94..197c7ea7a8 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -899,6 +899,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -1215,6 +1229,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. @@ -4890,6 +4918,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -5206,6 +5248,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index b698589c17..88f81ace7f 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1026,6 +1026,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -1342,6 +1356,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. @@ -2999,6 +3027,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -3315,6 +3357,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 4738f3df92..295c6e727a 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -902,6 +902,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -1218,6 +1232,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. @@ -4893,6 +4921,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object allowPrivateIPs: description: |- AllowPrivateIPs permits the upstream provider's HTTP client to connect to @@ -5209,6 +5251,20 @@ spec: scope, state, code_challenge, code_challenge_method, nonce) are not allowed. maxProperties: 16 type: object + additionalTokenParams: + additionalProperties: + type: string + description: |- + AdditionalTokenParams are extra form-body parameters to include in + token requests (authorization code exchange and refresh) sent to the + upstream provider's token endpoint. + This is useful for providers that enforce RFC 8707 resource indicators + on token requests, where the resource parameter must accompany the code + exchange and refresh, not only the authorization request. + Framework-managed parameters (grant_type, code, redirect_uri, client_id, + client_secret, code_verifier, refresh_token, scope) are not allowed. + maxProperties: 16 + type: object clientId: description: ClientID is the OAuth 2.0 client identifier registered with the upstream IdP. diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 4ca0746e9a..02f27a696c 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -3669,6 +3669,7 @@ _Appears in:_ | `tokenResponseMapping` _[api.v1beta1.TokenResponseMapping](#apiv1beta1tokenresponsemapping)_ | TokenResponseMapping configures custom field extraction from non-standard token responses.
Some OAuth providers (e.g., GovSlack) nest token fields under non-standard paths
instead of returning them at the top level. When set, ToolHive performs the token
exchange HTTP call directly and extracts fields using the configured dot-notation paths.
If nil, standard OAuth 2.0 token response parsing is used.
For extracting user identity from the token response, see IdentityFromToken. | | Optional: \{\}
| | `identityFromToken` _[api.v1beta1.IdentityFromTokenConfig](#apiv1beta1identityfromtokenconfig)_ | IdentityFromToken extracts user identity (subject, name, email) directly
from the OAuth2 token-endpoint response body using gjson dot-notation paths.
When set, the embedded auth server skips the userinfo HTTP call entirely
and resolves identity from the token response. See IdentityFromTokenConfig
for trust-model and uniqueness considerations. | | Optional: \{\}
| | `additionalAuthorizationParams` _object (keys:string, values:string)_ | AdditionalAuthorizationParams are extra query parameters to include in
authorization requests sent to the upstream provider.
This is useful for providers that require custom parameters, such as
Google's access_type=offline for obtaining refresh tokens.
Framework-managed parameters (response_type, client_id, redirect_uri,
scope, state, code_challenge, code_challenge_method, nonce) are not allowed. | | MaxProperties: 16
Optional: \{\}
| +| `additionalTokenParams` _object (keys:string, values:string)_ | AdditionalTokenParams are extra form-body parameters to include in
token requests (authorization code exchange and refresh) sent to the
upstream provider's token endpoint.
This is useful for providers that enforce RFC 8707 resource indicators
on token requests, where the resource parameter must accompany the code
exchange and refresh, not only the authorization request.
Framework-managed parameters (grant_type, code, redirect_uri, client_id,
client_secret, code_verifier, refresh_token, scope) are not allowed. | | MaxProperties: 16
Optional: \{\}
| | `insecureAllowHTTP` _boolean_ | InsecureAllowHTTP permits plain-HTTP authorization and token endpoint URLs
for this upstream. Only for in-cluster development environments (e.g. an
OAuth2 provider served over HTTP in a kind cluster) where TLS is not
available. Never set this in production. | | Optional: \{\}
| | `allowPrivateIPs` _boolean_ | AllowPrivateIPs permits the upstream provider's HTTP client to connect to
private IP ranges (RFC-1918, link-local). Use only when the upstream is
hosted inside the same cluster and has no public endpoint. HTTP-scheme
restrictions are unchanged — HTTPS is still required for non-localhost
hosts unless InsecureAllowHTTP is set. Defaults to false. | | Optional: \{\}
| | `dcrConfig` _[api.v1beta1.DCRUpstreamConfig](#apiv1beta1dcrupstreamconfig)_ | DCRConfig enables RFC 7591 Dynamic Client Registration against the upstream
authorization server. When set, the client credentials are obtained at
runtime rather than being pre-provisioned, and ClientID must be left empty.
Mutually exclusive with ClientID. | | Optional: \{\}
| @@ -3752,6 +3753,7 @@ _Appears in:_ | `scopes` _string array_ | Scopes are the OAuth scopes to request from the upstream IdP.
If not specified, defaults to ["openid", "offline_access"].
When using additionalAuthorizationParams with provider-specific refresh token
mechanisms (e.g., Google's access_type=offline), set explicit scopes to avoid
sending both offline_access and the provider-specific parameter. | | Optional: \{\}
| | `userInfoOverride` _[api.v1beta1.UserInfoConfig](#apiv1beta1userinfoconfig)_ | UserInfoOverride allows customizing UserInfo fetching behavior for OIDC providers.
By default, the UserInfo endpoint is discovered automatically via OIDC discovery.
Use this to override the endpoint URL, HTTP method, or field mappings for providers
that return non-standard claim names in their UserInfo response. | | Optional: \{\}
| | `additionalAuthorizationParams` _object (keys:string, values:string)_ | AdditionalAuthorizationParams are extra query parameters to include in
authorization requests sent to the upstream provider.
This is useful for providers that require custom parameters, such as
Google's access_type=offline for obtaining refresh tokens.
Note: when using access_type=offline, also set explicit scopes to avoid
the default offline_access scope being sent alongside it.
Framework-managed parameters (response_type, client_id, redirect_uri,
scope, state, code_challenge, code_challenge_method, nonce) are not allowed. | | MaxProperties: 16
Optional: \{\}
| +| `additionalTokenParams` _object (keys:string, values:string)_ | AdditionalTokenParams are extra form-body parameters to include in
token requests (authorization code exchange and refresh) sent to the
upstream provider's token endpoint.
This is useful for providers that enforce RFC 8707 resource indicators
on token requests, where the resource parameter must accompany the code
exchange and refresh, not only the authorization request.
Framework-managed parameters (grant_type, code, redirect_uri, client_id,
client_secret, code_verifier, refresh_token, scope) are not allowed. | | MaxProperties: 16
Optional: \{\}
| | `subjectClaim` _string_ | SubjectClaim names the validated ID-token claim to use as the upstream
subject. Defaults to "sub" when empty. Set it for IdPs where "sub" isn't
stable per user — e.g. Entra/Azure AD, whose "sub" rotates per application
and whose stable identifier is "oid".
The value is looked up verbatim as a top-level claim name, so it is
constrained to a claim-name shape: it must start with a letter or
underscore and contain only letters, digits, and underscores. This rejects
dotted, colon-namespaced, or whitespace-containing values at admission
rather than letting a typo silently miss the claim at login, and keeps the
field aligned with the directory service's per-issuer bindingClaim.
Changing this on a live deployment re-keys existing users (the value
resolves to the internal user ID), so treat it as immutable once users
exist.
Per-IdP notes:
- Entra/Azure AD: use "oid"; it is only emitted when the upstream scopes
include "profile". "oid" is unique within a single tenant — multi-tenant
apps need oid+tid, which this single-claim field cannot express.
- Okta: the org auth server already puts the stable id in "sub" (default
works). A custom auth server's "sub" is the mutable login/email and the
stable "uid" lives only in the access token, not the ID token — map a
custom ID-token claim and point subjectClaim at it.
The pattern matches the claim-name shape and allows empty (defaults to
"sub"). Using Pattern rather than a CEL XValidation rule keeps this off the
CRD's CEL cost budget — a single-field format check via CEL is rejected by
the apiserver as too expensive once multiplied across the upstreams list. | | MaxLength: 128
Pattern: `^([a-zA-Z_][a-zA-Z0-9_]*)?$`
Optional: \{\}
| diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 24b7710f97..74adb45c28 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -561,6 +561,12 @@ type OIDCUpstreamRunConfig struct { //nolint:lll // field tags require full JSON+YAML names AdditionalAuthorizationParams map[string]string `json:"additional_authorization_params,omitempty" yaml:"additional_authorization_params,omitempty"` + // AdditionalTokenParams are extra form-body parameters to include in + // token requests (authorization code exchange and refresh). Useful for + // providers that enforce RFC 8707 resource indicators on token requests. + //nolint:lll // field tags require full JSON+YAML names + AdditionalTokenParams map[string]string `json:"additional_token_params,omitempty" yaml:"additional_token_params,omitempty"` + // SubjectClaim names the validated ID-token claim to use as the upstream // subject. Defaults to "sub" when empty. Set for IdPs where "sub" isn't // stable per user (e.g. Entra/Azure AD's "oid"). See upstream.OIDCConfig. @@ -637,6 +643,12 @@ type OAuth2UpstreamRunConfig struct { //nolint:lll // field tags require full JSON+YAML names AdditionalAuthorizationParams map[string]string `json:"additional_authorization_params,omitempty" yaml:"additional_authorization_params,omitempty"` + // AdditionalTokenParams are extra form-body parameters to include in + // token requests (authorization code exchange and refresh). Useful for + // providers that enforce RFC 8707 resource indicators on token requests. + //nolint:lll // field tags require full JSON+YAML names + AdditionalTokenParams map[string]string `json:"additional_token_params,omitempty" yaml:"additional_token_params,omitempty"` + // DCRConfig enables RFC 7591 Dynamic Client Registration against the // upstream authorization server. When set, the client credentials are // obtained at runtime rather than being pre-provisioned via ClientID / diff --git a/pkg/authserver/oauthparams/reserved.go b/pkg/authserver/oauthparams/reserved.go index fd8ffbf7c7..02fd37b1fe 100644 --- a/pkg/authserver/oauthparams/reserved.go +++ b/pkg/authserver/oauthparams/reserved.go @@ -20,6 +20,20 @@ var ReservedAuthorizationParams = map[string]bool{ "nonce": true, } +// ReservedTokenParams are OAuth2 parameters managed by the framework that +// must not be set via AdditionalTokenParams. They cover both token-endpoint +// grant types the framework issues (authorization_code and refresh_token). +var ReservedTokenParams = map[string]bool{ + "grant_type": true, + "code": true, + "redirect_uri": true, + "client_id": true, + "client_secret": true, + "code_verifier": true, + "refresh_token": true, + "scope": true, +} + // Validate checks that no key in params is a reserved OAuth2 authorization // parameter. Reserved parameters are managed by the framework and cannot be // overridden via additional authorization params. @@ -31,3 +45,15 @@ func Validate(params map[string]string) error { } return nil } + +// ValidateTokenParams checks that no key in params is a reserved OAuth2 +// token-request parameter. Reserved parameters are managed by the framework +// and cannot be overridden via additional token params. +func ValidateTokenParams(params map[string]string) error { + for k := range params { + if ReservedTokenParams[k] { + return fmt.Errorf("reserved token parameter %q is managed by the framework and cannot be overridden", k) + } + } + return nil +} diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index 44dc0b61a8..94319ddc9b 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -627,6 +627,7 @@ func buildOIDCConfig(rc *authserver.UpstreamRunConfig, insecureAllowHTTP bool) ( RedirectURI: oidc.RedirectURI, Scopes: scopes, AdditionalAuthorizationParams: oidc.AdditionalAuthorizationParams, + AdditionalTokenParams: oidc.AdditionalTokenParams, }, Issuer: oidc.IssuerURL, SubjectClaim: oidc.SubjectClaim, @@ -662,6 +663,7 @@ func buildPureOAuth2Config(rc *authserver.UpstreamRunConfig, insecureAllowHTTP b RedirectURI: oauth2.RedirectURI, Scopes: oauth2.Scopes, AdditionalAuthorizationParams: oauth2.AdditionalAuthorizationParams, + AdditionalTokenParams: oauth2.AdditionalTokenParams, }, AuthorizationEndpoint: oauth2.AuthorizationEndpoint, TokenEndpoint: oauth2.TokenEndpoint, diff --git a/pkg/authserver/upstream/oauth2.go b/pkg/authserver/upstream/oauth2.go index bb44b31643..391f1c07d1 100644 --- a/pkg/authserver/upstream/oauth2.go +++ b/pkg/authserver/upstream/oauth2.go @@ -117,6 +117,18 @@ type CommonOAuthConfig struct { // and will be rejected during validation. //nolint:lll // field tags require full JSON+YAML names AdditionalAuthorizationParams map[string]string `json:"additional_authorization_params,omitempty" yaml:"additional_authorization_params,omitempty"` + + // AdditionalTokenParams are extra form-body parameters to include in + // token requests (authorization code exchange and refresh) sent to the + // upstream IDP's token endpoint. This is useful for providers that + // enforce RFC 8707 resource indicators on token requests, where the + // resource parameter must accompany the code exchange and refresh, not + // only the authorization request. + // Framework-managed parameters (grant_type, code, redirect_uri, client_id, + // client_secret, code_verifier, refresh_token, scope) are not allowed here + // and will be rejected during validation. + //nolint:lll // field tags require full JSON+YAML names + AdditionalTokenParams map[string]string `json:"additional_token_params,omitempty" yaml:"additional_token_params,omitempty"` } // ValidateWithInsecure validates CommonOAuthConfig, allowing http:// redirect URIs for @@ -131,6 +143,9 @@ func (c *CommonOAuthConfig) ValidateWithInsecure(insecureAllowHTTP bool) error { if err := oauthparams.Validate(c.AdditionalAuthorizationParams); err != nil { return err } + if err := oauthparams.ValidateTokenParams(c.AdditionalTokenParams); err != nil { + return err + } if insecureAllowHTTP { return oauthproto.ValidateRedirectURI(c.RedirectURI, oauthproto.RedirectURIPolicyAllowHTTP) } @@ -672,6 +687,12 @@ func (p *BaseOAuth2Provider) exchangeCodeForTokens( if codeVerifier != "" { opts = append(opts, oauth2.VerifierOption(codeVerifier)) } + // AuthCodeOptions passed to Exchange land in the POST form body, so + // configured additional token params (e.g. an RFC 8707 resource + // indicator) reach the token endpoint the way such ASes require. + for k, v := range p.config.AdditionalTokenParams { + opts = append(opts, oauth2.SetAuthURLParam(k, v)) + } token, err := p.oauth2Config.Exchange(ctx, code, opts...) if err != nil { @@ -737,6 +758,11 @@ func (p *BaseOAuth2Provider) RefreshTokens(ctx context.Context, refreshToken, _ if len(p.oauth2Config.Scopes) > 0 { opts = append(opts, oauth2.SetAuthURLParam("scope", strings.Join(p.oauth2Config.Scopes, " "))) } + // Same rationale as in exchangeCodeForTokens: ASes that enforce + // RFC 8707 on token requests require these on refresh as well. + for k, v := range p.config.AdditionalTokenParams { + opts = append(opts, oauth2.SetAuthURLParam(k, v)) + } token, err := p.oauth2Config.Exchange(ctx, "", opts...) if err != nil { diff --git a/pkg/authserver/upstream/oauth2_test.go b/pkg/authserver/upstream/oauth2_test.go index d00b63afa2..7cfab25351 100644 --- a/pkg/authserver/upstream/oauth2_test.go +++ b/pkg/authserver/upstream/oauth2_test.go @@ -2870,3 +2870,160 @@ func TestOAuth2Config_AllowPrivateIPs(t *testing.T) { assert.False(t, provider.config.AllowPrivateIPs) }) } + +func TestValidateAdditionalTokenParams(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + params map[string]string + wantErr bool + errContains string + }{ + { + name: "nil map", + params: nil, + }, + { + name: "empty map", + params: map[string]string{}, + }, + { + name: "valid RFC 8707 resource", + params: map[string]string{"resource": "https://api.example.com/mcp"}, + }, + { + name: "valid multiple params", + params: map[string]string{"resource": "https://api.example.com/mcp", "audience": "mcp"}, + }, + { + name: "reserved: grant_type", + params: map[string]string{"grant_type": "password"}, + wantErr: true, + errContains: "grant_type", + }, + { + name: "reserved: code", + params: map[string]string{"code": "x"}, + wantErr: true, + errContains: "code", + }, + { + name: "reserved: code_verifier", + params: map[string]string{"code_verifier": "x"}, + wantErr: true, + errContains: "code_verifier", + }, + { + name: "reserved: refresh_token", + params: map[string]string{"refresh_token": "x"}, + wantErr: true, + errContains: "refresh_token", + }, + { + name: "reserved: client_id", + params: map[string]string{"client_id": "x"}, + wantErr: true, + errContains: "client_id", + }, + { + name: "reserved: client_secret", + params: map[string]string{"client_secret": "x"}, + wantErr: true, + errContains: "client_secret", + }, + { + name: "reserved: redirect_uri", + params: map[string]string{"redirect_uri": "http://evil.com"}, + wantErr: true, + errContains: "redirect_uri", + }, + { + name: "reserved: scope", + params: map[string]string{"scope": "admin"}, + wantErr: true, + errContains: "scope", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + config := &CommonOAuthConfig{ + ClientID: "test-client", + RedirectURI: "http://localhost:8080/callback", + AdditionalTokenParams: tt.params, + } + + err := config.ValidateWithInsecure(false) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + } + }) + } +} + +// TestTokenRequests_AdditionalTokenParams verifies that configured additional +// token params reach the token endpoint's POST form body on both the +// authorization-code exchange and the refresh grant — the RFC 8707 use case, +// where an AS rejects token requests lacking a resource indicator. +func TestTokenRequests_AdditionalTokenParams(t *testing.T) { + t.Parallel() + + newProviderWithCapture := func(t *testing.T) (*BaseOAuth2Provider, *url.Values) { + t.Helper() + var captured url.Values + mux := http.NewServeMux() + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + captured = r.PostForm + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"a","token_type":"Bearer","refresh_token":"r"}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + config := &OAuth2Config{ + CommonOAuthConfig: CommonOAuthConfig{ + ClientID: "test-client", + RedirectURI: "http://localhost:8080/callback", + AdditionalTokenParams: map[string]string{ + "resource": "https://api.example.com/mcp", + }, + }, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + } + provider, err := NewOAuth2Provider(config) + require.NoError(t, err) + return provider, &captured + } + + t.Run("code exchange includes params in form body", func(t *testing.T) { + t.Parallel() + + provider, captured := newProviderWithCapture(t) + _, err := provider.ExchangeCodeForIdentity(context.Background(), "test-code", "test-verifier", "") + require.NoError(t, err) + + assert.Equal(t, "https://api.example.com/mcp", captured.Get("resource")) + assert.Equal(t, "authorization_code", captured.Get("grant_type")) + assert.Equal(t, "test-verifier", captured.Get("code_verifier")) + }) + + t.Run("refresh includes params in form body", func(t *testing.T) { + t.Parallel() + + provider, captured := newProviderWithCapture(t) + _, err := provider.RefreshTokens(context.Background(), "old-refresh", "") + require.NoError(t, err) + + assert.Equal(t, "https://api.example.com/mcp", captured.Get("resource")) + assert.Equal(t, "refresh_token", captured.Get("grant_type")) + }) +}