diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index 246df8759e..fc65b3959b 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -1654,6 +1654,15 @@ type OAuth2UpstreamConfig struct { // +optional ClientSecretRef *SecretKeyRef `json:"clientSecretRef,omitempty"` + // TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + // endpoint. When empty and a client secret is configured, client_secret_basic is + // used, matching the RFC 7591 default for confidential clients. Set this to + // client_secret_post only for providers that require credentials in the request body. + // Public clients without a secret use the "none" method. + // +kubebuilder:validation:Enum=none;client_secret_basic;client_secret_post + // +optional + TokenEndpointAuthMethod string `json:"tokenEndpointAuthMethod,omitempty"` + // RedirectURI is the callback URL where the upstream IdP will redirect after authentication. // When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the // URL associated with the resource (e.g., MCPServer or vMCP) using this config. diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index fa345187e3..1a5fe6d2cd 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -1369,6 +1369,7 @@ func buildOAuth2UpstreamRunConfig( AuthorizationEndpoint: cfg.AuthorizationEndpoint, TokenEndpoint: cfg.TokenEndpoint, ClientID: cfg.ClientID, + TokenEndpointAuthMethod: cfg.TokenEndpointAuthMethod, RedirectURI: redirectURI, Scopes: cfg.Scopes, AdditionalAuthorizationParams: cfg.AdditionalAuthorizationParams, diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index 58a498681c..32b3d0dc55 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -2062,6 +2062,35 @@ func TestBuildOAuth2UpstreamRunConfig_TransportOptions(t *testing.T) { assert.True(t, runConfig.AllowPrivateIPs) } +func TestBuildOAuth2UpstreamRunConfig_TokenEndpointAuthMethod(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + method string + }{ + {name: "empty passes through unset", method: ""}, + {name: "client_secret_basic propagates", method: "client_secret_basic"}, + {name: "client_secret_post propagates", method: "client_secret_post"}, + {name: "none propagates", method: "none"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + runConfig, err := buildOAuth2UpstreamRunConfig(&mcpv1beta1.OAuth2UpstreamConfig{ + AuthorizationEndpoint: "http://dex.default.svc.cluster.local/auth", + TokenEndpoint: "http://dex.default.svc.cluster.local/token", + ClientID: "client-id", + TokenEndpointAuthMethod: tt.method, + }, "", "", 0, "") + require.NoError(t, err) + assert.Equal(t, tt.method, runConfig.TokenEndpointAuthMethod) + }) + } +} + func TestDelegateClientsConversionAndEnvVars(t *testing.T) { t.Parallel() 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 6b29c0ebe4..973f97d235 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 @@ -1769,6 +1769,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. @@ -4457,6 +4469,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. 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 cfe115fe79..ce343a5b78 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 @@ -1645,6 +1645,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. @@ -6495,6 +6507,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. 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 3eacb211dd..d15318f907 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1772,6 +1772,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. @@ -4460,6 +4472,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. 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 4858264964..8daaca1e0a 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1648,6 +1648,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. @@ -6498,6 +6510,18 @@ spec: token endpoint. pattern: ^https?://.*$ type: string + tokenEndpointAuthMethod: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + enum: + - none + - client_secret_basic + - client_secret_post + type: string tokenResponseMapping: description: |- TokenResponseMapping configures custom field extraction from non-standard token responses. diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index bec48ee33a..40f0ea31bb 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -3712,6 +3712,7 @@ _Appears in:_ | `userInfo` _[api.v1beta1.UserInfoConfig](#apiv1beta1userinfoconfig)_ | UserInfo contains configuration for fetching user information from the upstream provider.
When omitted and IdentityFromToken is also unset, the embedded auth server runs in
synthesis mode for this upstream: a non-PII subject derived from the access token, no
Name/Email. Use this shape for upstreams with no userinfo surface and no identity in
the token response (e.g., MCP authorization servers per the MCP spec). When
IdentityFromToken is set instead, identity is resolved from the token response body
(e.g., Snowflake's "username" field, Slack's "authed_user.id"); the userinfo HTTP call
is skipped entirely. | | Optional: \{\}
| | `clientId` _string_ | ClientID is the OAuth 2.0 client identifier registered with the upstream IDP.
Mutually exclusive with DCRConfig: when DCRConfig is set, ClientID is obtained
at runtime via RFC 7591 Dynamic Client Registration and must be left empty. | | Optional: \{\}
| | `clientSecretRef` _[api.v1beta1.SecretKeyRef](#apiv1beta1secretkeyref)_ | ClientSecretRef references a Kubernetes Secret containing the OAuth 2.0 client secret.
Optional for public clients using PKCE instead of client secret. | | Optional: \{\}
| +| `tokenEndpointAuthMethod` _string_ | TokenEndpointAuthMethod selects how the client authenticates at the OAuth token
endpoint. When empty and a client secret is configured, client_secret_basic is
used, matching the RFC 7591 default for confidential clients. Set this to
client_secret_post only for providers that require credentials in the request body.
Public clients without a secret use the "none" method. | | Enum: [none client_secret_basic client_secret_post]
Optional: \{\}
| | `redirectUri` _string_ | RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
When not specified, defaults to `\{resourceUrl\}/oauth/callback` where `resourceUrl` is the
URL associated with the resource (e.g., MCPServer or vMCP) using this config. | | Optional: \{\}
| | `scopes` _string array_ | Scopes are the OAuth scopes to request from the upstream IdP. | | Optional: \{\}
| | `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: \{\}
| diff --git a/docs/server/docs.go b/docs/server/docs.go index 5bc3214c06..3523dba5f3 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -337,6 +337,10 @@ const docTemplate = `{ "description": "TokenEndpoint is the URL for the OAuth token endpoint.", "type": "string" }, + "token_endpoint_auth_method": { + "description": "TokenEndpointAuthMethod selects how the client authenticates at the OAuth token\nendpoint. When empty and a client secret is configured, client_secret_basic is\nused, matching the RFC 7591 default for confidential clients. Set this to\nclient_secret_post only for providers that require credentials in the request body.\nPublic clients without a secret use the \"none\" method.", + "type": "string" + }, "token_response_mapping": { "$ref": "#/components/schemas/authserver.TokenResponseMappingRunConfig" }, diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 5dce3e3555..b3d63e4c42 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -330,6 +330,10 @@ "description": "TokenEndpoint is the URL for the OAuth token endpoint.", "type": "string" }, + "token_endpoint_auth_method": { + "description": "TokenEndpointAuthMethod selects how the client authenticates at the OAuth token\nendpoint. When empty and a client secret is configured, client_secret_basic is\nused, matching the RFC 7591 default for confidential clients. Set this to\nclient_secret_post only for providers that require credentials in the request body.\nPublic clients without a secret use the \"none\" method.", + "type": "string" + }, "token_response_mapping": { "$ref": "#/components/schemas/authserver.TokenResponseMappingRunConfig" }, diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index a95ad2b30e..9998325b0b 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -392,6 +392,14 @@ components: token_endpoint: description: TokenEndpoint is the URL for the OAuth token endpoint. type: string + token_endpoint_auth_method: + description: |- + TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + endpoint. When empty and a client secret is configured, client_secret_basic is + used, matching the RFC 7591 default for confidential clients. Set this to + client_secret_post only for providers that require credentials in the request body. + Public clients without a secret use the "none" method. + type: string token_response_mapping: $ref: '#/components/schemas/authserver.TokenResponseMappingRunConfig' userinfo: diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index bd7a756258..1f66f8e6de 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -703,6 +703,13 @@ type OAuth2UpstreamRunConfig struct { // Mutually exclusive with ClientSecretFile. Optional for public clients using PKCE. ClientSecretEnvVar string `json:"client_secret_env_var,omitempty" yaml:"client_secret_env_var,omitempty"` + // TokenEndpointAuthMethod selects how the client authenticates at the OAuth token + // endpoint. When empty and a client secret is configured, client_secret_basic is + // used, matching the RFC 7591 default for confidential clients. Set this to + // client_secret_post only for providers that require credentials in the request body. + // Public clients without a secret use the "none" method. + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty" yaml:"token_endpoint_auth_method,omitempty"` + // RedirectURI is the callback URL where the upstream IDP will redirect after authentication. // When not specified, defaults to `{issuer}/oauth/callback`. RedirectURI string `json:"redirect_uri,omitempty" yaml:"redirect_uri,omitempty"` @@ -1420,6 +1427,33 @@ func (c *OAuth2UpstreamRunConfig) Validate() error { return fmt.Errorf("oauth2 upstream: identity_from_token.subject_path must not be empty when identity_from_token is configured") } + return c.validateTokenEndpointAuthMethod() +} + +// validateTokenEndpointAuthMethod checks TokenEndpointAuthMethod against the +// set of methods buildPureOAuth2Config/authStyleFromMethod support, and that +// the method is consistent with whether a client secret source is configured. +// Split out of Validate to keep that method's cyclomatic complexity down. +func (c *OAuth2UpstreamRunConfig) validateTokenEndpointAuthMethod() error { + hasSecretSource := c.ClientSecretFile != "" || c.ClientSecretEnvVar != "" + + switch c.TokenEndpointAuthMethod { + case "": + // Resolved from the presence of a secret in buildPureOAuth2Config. + case oauthproto.TokenEndpointAuthMethodNone: + if hasSecretSource { + return fmt.Errorf("oauth2 upstream: token_endpoint_auth_method none cannot be used with a client secret") + } + case oauthproto.TokenEndpointAuthMethodClientSecretBasic, oauthproto.TokenEndpointAuthMethodClientSecretPost: + if !hasSecretSource { + return fmt.Errorf( + "oauth2 upstream: token_endpoint_auth_method %q requires client_secret_file or client_secret_env_var", + c.TokenEndpointAuthMethod) + } + default: + return fmt.Errorf("oauth2 upstream: unsupported token_endpoint_auth_method %q", c.TokenEndpointAuthMethod) + } + return nil } diff --git a/pkg/authserver/config_test.go b/pkg/authserver/config_test.go index eca7dfe62d..b8682b3798 100644 --- a/pkg/authserver/config_test.go +++ b/pkg/authserver/config_test.go @@ -18,6 +18,7 @@ import ( "github.com/stacklok/toolhive/pkg/authserver/server/registration" "github.com/stacklok/toolhive/pkg/authserver/server/tokenexchange" "github.com/stacklok/toolhive/pkg/authserver/upstream" + "github.com/stacklok/toolhive/pkg/oauthproto" ) func TestValidateIssuerURL(t *testing.T) { @@ -515,6 +516,53 @@ func TestOAuth2UpstreamRunConfigValidate(t *testing.T) { ClientID: "c", }, }, + + // TokenEndpointAuthMethod / client secret source consistency. + { + name: "unrecognized TokenEndpointAuthMethod rejects", + config: OAuth2UpstreamRunConfig{ + ClientID: "c", + TokenEndpointAuthMethod: "not_a_real_method", + }, + wantErr: true, + errMsg: "unsupported token_endpoint_auth_method", + }, + { + name: "none with a configured client secret file rejects", + config: OAuth2UpstreamRunConfig{ + ClientID: "c", + ClientSecretFile: "/tmp/secret", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodNone, + }, + wantErr: true, + errMsg: "token_endpoint_auth_method none cannot be used with a client secret", + }, + { + name: "client_secret_basic without a secret source rejects", + config: OAuth2UpstreamRunConfig{ + ClientID: "c", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodClientSecretBasic, + }, + wantErr: true, + errMsg: `token_endpoint_auth_method "client_secret_basic" requires client_secret_file or client_secret_env_var`, + }, + { + name: "client_secret_post without a secret source rejects", + config: OAuth2UpstreamRunConfig{ + ClientID: "c", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodClientSecretPost, + }, + wantErr: true, + errMsg: `token_endpoint_auth_method "client_secret_post" requires client_secret_file or client_secret_env_var`, + }, + { + name: "client_secret_basic with a client secret env var configured is valid", + config: OAuth2UpstreamRunConfig{ + ClientID: "c", + ClientSecretEnvVar: "MY_CLIENT_SECRET", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodClientSecretBasic, + }, + }, } for _, tt := range tests { diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index 073162d8d4..26aa439367 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -26,6 +26,7 @@ import ( "github.com/stacklok/toolhive/pkg/authserver/storage" "github.com/stacklok/toolhive/pkg/authserver/upstream" "github.com/stacklok/toolhive/pkg/bodylimit" + "github.com/stacklok/toolhive/pkg/oauthproto" ) // Redis ACL credential environment variable names. @@ -748,6 +749,16 @@ func buildPureOAuth2Config(rc *authserver.UpstreamRunConfig, insecureAllowHTTP b return nil, fmt.Errorf("failed to resolve OAuth2 client secret: %w", err) } + authMethod := oauth2.TokenEndpointAuthMethod + if authMethod == "" && clientSecret != "" { + authMethod = oauthproto.TokenEndpointAuthMethodClientSecretBasic + } + if isConfidentialAuthMethod(authMethod) && clientSecret == "" { + return nil, fmt.Errorf( + "oauth2 upstream: token_endpoint_auth_method %q requires a non-empty client secret, "+ + "but the configured secret resolved to an empty value", authMethod) + } + cfg := &upstream.OAuth2Config{ CommonOAuthConfig: upstream.CommonOAuthConfig{ ClientID: oauth2.ClientID, @@ -756,12 +767,13 @@ func buildPureOAuth2Config(rc *authserver.UpstreamRunConfig, insecureAllowHTTP b Scopes: oauth2.Scopes, AdditionalAuthorizationParams: oauth2.AdditionalAuthorizationParams, }, - AuthorizationEndpoint: oauth2.AuthorizationEndpoint, - TokenEndpoint: oauth2.TokenEndpoint, - UserInfo: convertUserInfoConfig(oauth2.UserInfo), - CAFilePath: oauth2.CAFilePath, - AllowPrivateIPs: oauth2.AllowPrivateIPs, - InsecureAllowHTTP: insecureAllowHTTP || oauth2.InsecureAllowHTTP, + AuthorizationEndpoint: oauth2.AuthorizationEndpoint, + TokenEndpoint: oauth2.TokenEndpoint, + TokenEndpointAuthMethod: authMethod, + UserInfo: convertUserInfoConfig(oauth2.UserInfo), + CAFilePath: oauth2.CAFilePath, + AllowPrivateIPs: oauth2.AllowPrivateIPs, + InsecureAllowHTTP: insecureAllowHTTP || oauth2.InsecureAllowHTTP, } if oauth2.TokenResponseMapping != nil { @@ -784,6 +796,20 @@ func buildPureOAuth2Config(rc *authserver.UpstreamRunConfig, insecureAllowHTTP b return cfg, nil } +// isConfidentialAuthMethod reports whether method requires a client secret to +// be presented at the token endpoint. Used to catch a secret file that reads +// successfully but is empty after trimming -- a case OAuth2UpstreamRunConfig.Validate +// cannot see, since it only knows whether a secret source is configured, not +// what that source resolves to. +func isConfidentialAuthMethod(method string) bool { + switch method { + case oauthproto.TokenEndpointAuthMethodClientSecretBasic, oauthproto.TokenEndpointAuthMethodClientSecretPost: + return true + default: + return false + } +} + // resolveSecret reads a secret from file or environment variable. // File takes precedence over env var. Returns an error if file is specified but // unreadable, or if envVar is specified but not set. Returns empty string with diff --git a/pkg/authserver/runner/embeddedauthserver_test.go b/pkg/authserver/runner/embeddedauthserver_test.go index f6d3dd691f..6950b28f7b 100644 --- a/pkg/authserver/runner/embeddedauthserver_test.go +++ b/pkg/authserver/runner/embeddedauthserver_test.go @@ -596,12 +596,81 @@ func TestBuildPureOAuth2Config(t *testing.T) { assert.Equal(t, "https://example.com/token", cfg.TokenEndpoint) assert.Equal(t, "my-client-id", cfg.ClientID) assert.Equal(t, "my-client-secret", cfg.ClientSecret) + assert.Equal(t, oauthproto.TokenEndpointAuthMethodClientSecretBasic, cfg.TokenEndpointAuthMethod) assert.Equal(t, "https://my-app.com/callback", cfg.RedirectURI) assert.Equal(t, []string{"read", "write"}, cfg.Scopes) require.NotNil(t, cfg.UserInfo) assert.Equal(t, "https://example.com/userinfo", cfg.UserInfo.EndpointURL) }) + t.Run("leaves TokenEndpointAuthMethod empty for public client without secret", func(t *testing.T) { + t.Parallel() + + rc := &authserver.UpstreamRunConfig{ + Type: authserver.UpstreamProviderTypeOAuth2, + OAuth2Config: &authserver.OAuth2UpstreamRunConfig{ + AuthorizationEndpoint: "https://example.com/authorize", + TokenEndpoint: "https://example.com/token", + ClientID: "my-client-id", + RedirectURI: "https://my-app.com/callback", + }, + } + + cfg, err := buildPureOAuth2Config(rc, false) + require.NoError(t, err) + require.NotNil(t, cfg) + assert.Empty(t, cfg.TokenEndpointAuthMethod) + }) + + t.Run("preserves explicit client_secret_post over the client_secret_basic default", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + secretFile := filepath.Join(tmpDir, "client-secret") + require.NoError(t, os.WriteFile(secretFile, []byte("my-client-secret"), 0600)) + + rc := &authserver.UpstreamRunConfig{ + Type: authserver.UpstreamProviderTypeOAuth2, + OAuth2Config: &authserver.OAuth2UpstreamRunConfig{ + AuthorizationEndpoint: "https://example.com/authorize", + TokenEndpoint: "https://example.com/token", + ClientID: "my-client-id", + ClientSecretFile: secretFile, + RedirectURI: "https://my-app.com/callback", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodClientSecretPost, + }, + } + + cfg, err := buildPureOAuth2Config(rc, false) + require.NoError(t, err) + require.NotNil(t, cfg) + assert.Equal(t, oauthproto.TokenEndpointAuthMethodClientSecretPost, cfg.TokenEndpointAuthMethod) + }) + + t.Run("rejects client_secret_basic when the secret file resolves to empty", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + secretFile := filepath.Join(tmpDir, "empty-secret") + require.NoError(t, os.WriteFile(secretFile, []byte(" \n"), 0600)) + + rc := &authserver.UpstreamRunConfig{ + Type: authserver.UpstreamProviderTypeOAuth2, + OAuth2Config: &authserver.OAuth2UpstreamRunConfig{ + AuthorizationEndpoint: "https://example.com/authorize", + TokenEndpoint: "https://example.com/token", + ClientID: "my-client-id", + ClientSecretFile: secretFile, + RedirectURI: "https://my-app.com/callback", + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodClientSecretBasic, + }, + } + + _, err := buildPureOAuth2Config(rc, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "requires a non-empty client secret") + }) + t.Run("propagates AdditionalAuthorizationParams", func(t *testing.T) { t.Parallel() diff --git a/pkg/authserver/upstream/oauth2.go b/pkg/authserver/upstream/oauth2.go index 337e413b55..b10f174d0e 100644 --- a/pkg/authserver/upstream/oauth2.go +++ b/pkg/authserver/upstream/oauth2.go @@ -152,10 +152,9 @@ type OAuth2Config struct { // oauth2.AuthStyle and the rationale. When empty, the historical default // (POST body) is used. // - // Only the DCR path populates this, via applyResolutionToOAuth2Config. - // OAuth2UpstreamRunConfig has no corresponding field, so a statically- - // configured upstream cannot set it and always gets the default — an - // intentional limitation scoped to issue #5865 (DCR-negotiated clients). + // When empty, the caller's provider-specific default applies. Static + // OAuth2UpstreamRunConfig clients with a secret default to client_secret_basic; + // DCR clients receive the negotiated method via applyResolutionToOAuth2Config. //nolint:lll // field tags require full JSON+YAML names TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty" yaml:"token_endpoint_auth_method,omitempty"`