diff --git a/cmd/thv-operator/api/v1beta1/crd_schema_test.go b/cmd/thv-operator/api/v1beta1/crd_schema_test.go new file mode 100644 index 0000000000..b4599feffa --- /dev/null +++ b/cmd/thv-operator/api/v1beta1/crd_schema_test.go @@ -0,0 +1,96 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package v1beta1 + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" +) + +// loadCRDManifest reads a generated CRD manifest and decodes it into a +// generic JSON-like tree so the assertions stay independent of the typed +// API types the manifest is generated from. +func loadCRDManifest(t *testing.T, path string) map[string]any { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + var doc map[string]any + require.NoError(t, yaml.Unmarshal(data, &doc)) + return doc +} + +// collectSchemasForProperty walks a generic schema tree and collects every +// nested schema object registered under the given property name. +func collectSchemasForProperty(node any, propertyName string, out *[]map[string]any) { + switch v := node.(type) { + case map[string]any: + if prop, ok := v[propertyName]; ok { + if schema, ok := prop.(map[string]any); ok { + *out = append(*out, schema) + } + } + for _, child := range v { + collectSchemasForProperty(child, propertyName, out) + } + case []any: + for _, item := range v { + collectSchemasForProperty(item, propertyName, out) + } + } +} + +// TestCRDUpstreamCredentialScopeSchema asserts the generated CRD schemas +// expose upstreamCredentialScope with the session/platformUser enum and the +// permanent "session" default. This is a generated-artifact assertion: it +// fails until controller-gen regenerates the manifests. +func TestCRDUpstreamCredentialScopeSchema(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + path string + container string + minMatches int + }{ + { + name: "mcpexternalauthconfigs", + path: "../../../../deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml", + container: "embeddedAuthServer", + minMatches: 1, + }, + { + name: "virtualmcpservers", + path: "../../../../deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml", + container: "authServerConfig", + minMatches: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc := loadCRDManifest(t, tc.path) + + var schemas []map[string]any + collectSchemasForProperty(doc, tc.container, &schemas) + require.GreaterOrEqual(t, len(schemas), tc.minMatches, + "expected at least one %q schema in %s", tc.container, tc.path) + + for i, schema := range schemas { + props, ok := schema["properties"].(map[string]any) + require.Truef(t, ok, "%q schema %d has no properties", tc.container, i) + + field, ok := props["upstreamCredentialScope"].(map[string]any) + require.Truef(t, ok, "%q schema %d has no upstreamCredentialScope property", tc.container, i) + + assert.Equal(t, []any{"session", "platformUser"}, field["enum"], + "%q schema %d enum mismatch", tc.container, i) + assert.Equal(t, "session", field["default"], + "%q schema %d default mismatch", tc.container, i) + } + }) + } +} diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index a67c8499dd..fa97662313 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -702,6 +702,19 @@ type EmbeddedAuthServerConfig struct { // +optional DisableUpstreamTokenInjection bool `json:"disableUpstreamTokenInjection,omitempty"` + // UpstreamCredentialScope selects which identity model upstream credential + // lookup trusts. `session` (the permanent default) keeps today's behavior: + // the session-based TokenReader stays wired as-is and no platform-user + // trust checks run. `platformUser` is the explicit opt-in to the future + // durable platform-user credential model; it is validated and propagated + // but runtime activation fails as unsupported until platform-user storage + // is implemented. Only a genuinely absent value maps to `session`; unknown + // non-empty values are rejected, never reinterpreted as the default. + // +kubebuilder:default=session + // +optional + // +kubebuilder:validation:Enum=session;platformUser + UpstreamCredentialScope authserver.UpstreamCredentialScope `json:"upstreamCredentialScope,omitempty"` + // InsecureAllowHTTP permits an http:// issuer URL for non-localhost hosts. // Only set this for in-cluster Kubernetes deployments where traffic between // pods traverses a trusted network (e.g. the in-cluster service mesh). @@ -875,6 +888,13 @@ func (c *EmbeddedAuthServerConfig) ValidateConfidentialClientTransport() error { ) } +// EffectiveUpstreamCredentialScope maps an absent scope to session. The CRD +// default is applied by Kubernetes, but direct Go construction and older +// serialized objects may still carry an empty value. +func (c *EmbeddedAuthServerConfig) EffectiveUpstreamCredentialScope() (authserver.UpstreamCredentialScope, error) { + return authserver.EffectiveUpstreamCredentialScope(string(c.UpstreamCredentialScope)) +} + // TokenLifespanConfig holds configuration for token lifetimes. type TokenLifespanConfig struct { // AccessTokenLifespan is the duration that access tokens are valid. diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 3d9d821c84..3b957d93a3 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -757,7 +757,7 @@ func AddEmbeddedAuthServerConfigOptions( ) } - if err := validateOIDCConfigForEmbeddedAuthServer(oidcConfig); err != nil { + if err := validateOIDCConfigForEmbeddedAuthServer(oidcConfig, authServerConfig); err != nil { return invalidEmbeddedAuthServerConfigFrom(EmbeddedAuthServerConfigSourceExternalAuthConfigRef, err) } @@ -792,7 +792,9 @@ func AddEmbeddedAuthServerConfigOptions( // overriding Audience with ResourceURL) so that operators see exactly what // values are in play and control both sides explicitly. This mirrors the // existing vMCP inline config validation (ValidateAuthServerIntegration). -func validateOIDCConfigForEmbeddedAuthServer(oidcConfig *oidc.OIDCConfig) error { +// +//nolint:lll // parameter list exceeds the line limit +func validateOIDCConfigForEmbeddedAuthServer(oidcConfig *oidc.OIDCConfig, authServerConfig *mcpv1beta1.EmbeddedAuthServerConfig) error { if oidcConfig == nil { return fmt.Errorf("OIDC config is required for embedded auth server: OIDCConfigRef must be set on the MCPServer") } @@ -813,6 +815,25 @@ func validateOIDCConfigForEmbeddedAuthServer(oidcConfig *oidc.OIDCConfig) error oidcConfig.Audience, oidcConfig.ResourceURL, oidcConfig.ResourceURL, ) } + if authServerConfig != nil { + scope, err := authServerConfig.EffectiveUpstreamCredentialScope() + if err != nil { + return err + } + if scope == authserver.UpstreamCredentialScopePlatformUser { + if oidcConfig.Issuer == "" { + return fmt.Errorf("oidcConfigRef.issuer is required when upstreamCredentialScope is platformUser: " + + "the token carrying the platform-user identity must be validated as issued by the embedded auth server") + } + if oidcConfig.Issuer != authServerConfig.Issuer { + return fmt.Errorf( + "oidcConfigRef.issuer %q must exactly match the embedded auth server issuer %q "+ + "when upstreamCredentialScope is platformUser (no normalization or trailing-slash tolerance)", + oidcConfig.Issuer, authServerConfig.Issuer, + ) + } + } + } return nil } @@ -926,6 +947,9 @@ func applySimpleAuthServerConfigFields(config *authserver.RunConfig, authConfig // Wire through upstream token injection flag config.DisableUpstreamTokenInjection = authConfig.DisableUpstreamTokenInjection + // Wire through the upstream credential scope (session/platformUser). + config.UpstreamCredentialScope = authConfig.UpstreamCredentialScope + // Wire through the insecure HTTP issuer flag from the CRD field. // This replaces any auto-inference and moves control to the deployer. config.InsecureAllowHTTP = authConfig.InsecureAllowHTTP @@ -972,8 +996,9 @@ func validateDelegateClientsAndTrustedIssuers(config *authserver.RunConfig) erro InsecureAllowHTTP: config.InsecureAllowHTTP, AllowPrivateKeyJWTRegistration: config.AllowPrivateKeyJWTRegistration, InsecureAllowConfidentialOverLoopbackHTTP: config.InsecureAllowConfidentialOverLoopbackHTTP, - DelegateClients: config.DelegateClients, - TrustedIssuers: config.TrustedIssuers, + UpstreamCredentialScope: config.UpstreamCredentialScope, + DelegateClients: config.DelegateClients, + TrustedIssuers: config.TrustedIssuers, } if err := validationConfig.Validate(); err != nil { return fmt.Errorf("invalid embedded auth server delegate clients or trusted issuers: %w", err) @@ -1378,7 +1403,7 @@ func AddAuthServerRefOptions( ) } - if err := validateOIDCConfigForEmbeddedAuthServer(oidcConfig); err != nil { + if err := validateOIDCConfigForEmbeddedAuthServer(oidcConfig, authServerConfig); err != nil { return invalidEmbeddedAuthServerConfigFrom(EmbeddedAuthServerConfigSourceAuthServerRef, err) } diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index 0e5130febc..4728e409e1 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -3368,3 +3368,133 @@ func TestBuildTrustedIssuerRunConfigs_JWTBearerGrant(t *testing.T) { acceptedAudiences[0] = "https://auth.example.com/source-mutated" assert.Equal(t, "https://auth.example.com/legacy-token", configs[0].JWTBearerGrant.AcceptedAudiences[0]) } + +func TestBuildAuthServerRunConfig_UpstreamCredentialScope(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + scope authserver.UpstreamCredentialScope + }{ + {name: "empty stays empty", scope: ""}, + {name: "session propagated", scope: authserver.UpstreamCredentialScopeSession}, + {name: "platformUser propagated", scope: authserver.UpstreamCredentialScopePlatformUser}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + config, err := BuildAuthServerRunConfig( + "default", "test-server", + &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + UpstreamCredentialScope: tt.scope, + }, + []string{"https://resource.example.com"}, []string{"openid"}, "https://resource.example.com", + ) + require.NoError(t, err) + assert.Equal(t, tt.scope, config.UpstreamCredentialScope) + }) + } +} + +// The reconcile-time partial validation copy must carry the scope: a +// platformUser config with delegate clients has to fail at reconcile with the +// unsupported-capability error instead of silently reaching pod start. +func TestBuildAuthServerRunConfig_PlatformUserWithDelegateClientsRejected(t *testing.T) { + t.Parallel() + + _, err := BuildAuthServerRunConfig( + "default", "test-server", + &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + UpstreamCredentialScope: authserver.UpstreamCredentialScopePlatformUser, + DelegateClients: []mcpv1beta1.DelegateClientConfig{{ + ClientID: "delegate", + ClientSecretRef: &mcpv1beta1.SecretKeyRef{Name: "delegate-secret", Key: "credential"}, + Scopes: []string{"openid"}, + Audiences: []string{"https://resource.example.com"}, + }}, + }, + []string{"https://resource.example.com"}, []string{"openid"}, "https://resource.example.com", + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "not supported") +} + +func TestValidateOIDCConfigForEmbeddedAuthServer_UpstreamCredentialScope(t *testing.T) { + t.Parallel() + + oidcConfig := func(issuer string) *oidc.OIDCConfig { + return &oidc.OIDCConfig{ + ResourceURL: "https://my-vmcp", + Audience: "https://my-vmcp", + Issuer: issuer, + } + } + authServerConfig := func(scope authserver.UpstreamCredentialScope, issuer string) *mcpv1beta1.EmbeddedAuthServerConfig { + return &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: issuer, + UpstreamCredentialScope: scope, + } + } + + tests := []struct { + name string + oidcConfig *oidc.OIDCConfig + authServerConfig *mcpv1beta1.EmbeddedAuthServerConfig + wantErr bool + errMsg string + }{ + { + name: "session scope with mismatched issuer still passes", + oidcConfig: oidcConfig("https://login.corp.example"), + authServerConfig: authServerConfig(authserver.UpstreamCredentialScopeSession, "https://auth.example"), + }, + { + name: "empty scope with mismatched issuer still passes", + oidcConfig: oidcConfig("https://login.corp.example"), + authServerConfig: authServerConfig("", "https://auth.example"), + }, + { + name: "platformUser requires oidc issuer", + oidcConfig: oidcConfig(""), + authServerConfig: authServerConfig(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + wantErr: true, + errMsg: "oidcConfigRef.issuer is required", + }, + { + name: "platformUser rejects mismatched issuer", + oidcConfig: oidcConfig("https://login.corp.example"), + authServerConfig: authServerConfig(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + wantErr: true, + errMsg: "must exactly match", + }, + { + name: "platformUser accepts exact match", + oidcConfig: oidcConfig("https://auth.example"), + authServerConfig: authServerConfig(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + }, + { + name: "garbage scope rejected", + oidcConfig: oidcConfig("https://auth.example"), + authServerConfig: authServerConfig("garbage", "https://auth.example"), + wantErr: true, + errMsg: "invalid upstreamCredentialScope", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validateOIDCConfigForEmbeddedAuthServer(tt.oidcConfig, tt.authServerConfig) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errMsg) + return + } + require.NoError(t, err) + }) + } +} 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 6eeebb1005..ff56adf047 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 @@ -1036,6 +1036,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. @@ -3158,6 +3173,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. 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 f4520c6555..d95b0b8f43 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 @@ -912,6 +912,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. @@ -5045,6 +5060,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. 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 bc1399b0be..c281b63ca3 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1039,6 +1039,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. @@ -3161,6 +3176,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. 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 0e2fe27e91..113df01cd6 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -915,6 +915,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. @@ -5048,6 +5063,21 @@ spec: maxItems: 20 type: array x-kubernetes-list-type: atomic + upstreamCredentialScope: + default: session + description: |- + UpstreamCredentialScope selects which identity model upstream credential + lookup trusts. `session` (the permanent default) keeps today's behavior: + the session-based TokenReader stays wired as-is and no platform-user + trust checks run. `platformUser` is the explicit opt-in to the future + durable platform-user credential model; it is validated and propagated + but runtime activation fails as unsupported until platform-user storage + is implemented. Only a genuinely absent value maps to `session`; unknown + non-empty values are rejected, never reinterpreted as the default. + enum: + - session + - platformUser + type: string upstreamProviders: description: |- UpstreamProviders configures connections to upstream Identity Providers. diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 17be058246..f346cd4eb0 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -1981,6 +1981,7 @@ _Appears in:_ | `primaryUpstreamProvider` _string_ | PrimaryUpstreamProvider names the upstream IDP whose access token Cedar
should read claims from when authorising a request. Must match the name
of one of the entries in UpstreamProviders. When empty, the controller
auto-selects the first entry of UpstreamProviders.
Only meaningful on VirtualMCPServer, where multiple upstream providers
can be configured and Cedar needs to pick which token's claims to
evaluate. The VirtualMCPServer controller validates this field against
UpstreamProviders at admission and rejects unresolvable values.
On MCPServer and MCPRemoteProxy this field is structurally present (the
EmbeddedAuthServerConfig struct is shared) but has no runtime effect:
those CRDs are restricted to a single upstream so there is no choice to
make. Setting it on those CRDs is silently ignored. | | MaxLength: 63
MinLength: 1
Pattern: `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`
Optional: \{\}
| | `storage` _[api.v1beta1.AuthServerStorageConfig](#apiv1beta1authserverstorageconfig)_ | Storage configures the storage backend for the embedded auth server.
If not specified, defaults to in-memory storage. | | Optional: \{\}
| | `disableUpstreamTokenInjection` _boolean_ | DisableUpstreamTokenInjection prevents the embedded auth server from injecting
upstream IdP tokens into requests forwarded to the backend MCP server.
When true, the embedded auth server still handles OAuth flows for clients,
but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS
the client's credential headers (Authorization, Cookie, Proxy-Authorization)
after validating the JWT — the backend receives an unauthenticated request.
Use headerForward to attach static credentials (e.g. an API key) if the
backend needs them. Cannot be combined with token exchange, AWS STS, or OBO
middleware, which would re-add credentials after the strip.
This is useful when the backend MCP server does not require authentication
(e.g., public documentation servers) but you still want client authentication. | false | Optional: \{\}
| +| `upstreamCredentialScope` _[pkg.authserver.UpstreamCredentialScope](#pkgauthserverupstreamcredentialscope)_ | UpstreamCredentialScope selects which identity model upstream credential
lookup trusts. `session` (the permanent default) keeps today's behavior:
the session-based TokenReader stays wired as-is and no platform-user
trust checks run. `platformUser` is the explicit opt-in to the future
durable platform-user credential model; it is validated and propagated
but runtime activation fails as unsupported until platform-user storage
is implemented. Only a genuinely absent value maps to `session`; unknown
non-empty values are rejected, never reinterpreted as the default. | session | Enum: [session platformUser]
Optional: \{\}
| | `insecureAllowHTTP` _boolean_ | InsecureAllowHTTP permits an http:// issuer URL for non-localhost hosts.
Only set this for in-cluster Kubernetes deployments where traffic between
pods traverses a trusted network (e.g. the in-cluster service mesh).
Production deployments reachable outside the cluster MUST use https://.
On VirtualMCPServer: when false (the default), http:// issuers for non-localhost
hosts are rejected at reconcile time with an AuthServerConfigValidated=False condition.
On MCPServer and MCPRemoteProxy (via MCPExternalAuthConfig): this field is
structurally present but enforcement is deferred to pod startup via Config.Validate();
a misconfigured issuer will cause the pod to crash at startup rather than surface
as an operator condition.
One combination is rejected at admission on all three CRDs regardless of the
above: setting this field alongside allowConfidentialClientRegistration, which
would issue client secrets in cleartext over an unauthenticated registration
endpoint (see the XValidation rule on EmbeddedAuthServerConfig). | false | Optional: \{\}
| | `baselineClientScopes` _string array_ | BaselineClientScopes is a baseline set of OAuth 2.0 scopes guaranteed to be
included in every client registration. The embedded auth server unions these
scopes into the registered set returned by RFC 7591 Dynamic Client
Registration, so a client that narrows the `scope` field at /oauth/register
can still request the baseline scopes at /oauth/authorize. All values must
be present in the upstream-derived scopesSupported set; the auth server
fails to start if any value is missing.
Security: every client registered via /oauth/register will gain the
ability to request these scopes at /oauth/authorize, regardless of what
the client itself requested. Keep the baseline narrow (typically
"openid" and "offline_access"). Adding a privileged scope here — e.g.
"admin:read" — would grant it to every DCR-registered client, including
public clients like Claude Code, Cursor, and VS Code.
When cimd.enabled is true, every dynamically resolved CIMD client will
also gain the ability to request these scopes, including third-party
clients resolved from arbitrary HTTPS URLs. | | MaxItems: 10
items:MinLength: 1
items:Pattern: `^[\x21\x23-\x5B\x5D-\x7E]+$`
Optional: \{\}
| | `allowConfidentialClientRegistration` _boolean_ | AllowConfidentialClientRegistration permits RFC 7591 Dynamic Client
Registration of confidential clients: when true, /oauth/register
accepts token_endpoint_auth_method values client_secret_basic and
client_secret_post in addition to "none" (still the default on
omission) and mints a client_secret returned exactly once.
Confidential registrations are restricted to https non-loopback
redirect URIs, and on the Redis storage backend all DCR-issued
registrations are evicted after 30 days of inactivity and must
re-register. This gates registration only: disabling it does not
revoke or reject already-minted secrets at the token endpoint.
Security: registration is unauthenticated, so enabling this lets any
caller who can reach the endpoint obtain a client credential.
Combining it with insecureAllowHTTP is rejected at validation. | false | Optional: \{\}
| diff --git a/docs/server/docs.go b/docs/server/docs.go index 00ebcea864..7e71182689 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -73,6 +73,10 @@ const docTemplate = `{ "description": "Audience is the expected audience for the token", "type": "string" }, + "authServerIssuer": { + "description": "AuthServerIssuer is the issuer of the embedded ToolHive auth server that\nowns the platform-user credential namespace. It is only required when a\nUserTokenReader is configured, in which case it must be non-empty and\nexactly equal to Issuer.", + "type": "string" + }, "authTokenFile": { "description": "AuthTokenFile is the path to file containing bearer token for authentication", "type": "string" @@ -455,6 +459,10 @@ const docTemplate = `{ "type": "array", "uniqueItems": false }, + "upstream_credential_scope": { + "description": "UpstreamCredentialScope selects the credential-lookup identity model for\nthis auth server. Only a genuinely absent value maps to session:\nKubernetes defaulting does not cover old serialized RunConfigs, direct\nvMCP YAML, or direct Go construction.", + "type": "string" + }, "upstreams": { "description": "Upstreams configures connections to upstream Identity Providers for\ninteractive authorization. It may be empty only when DelegateClients or a\nTrustedIssuer with JWTBearerGrant enables token-only operation.\nMultiple upstreams are supported for sequential authorization chains.", "items": { diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 2c0cf0ddac..29e2be2fee 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -66,6 +66,10 @@ "description": "Audience is the expected audience for the token", "type": "string" }, + "authServerIssuer": { + "description": "AuthServerIssuer is the issuer of the embedded ToolHive auth server that\nowns the platform-user credential namespace. It is only required when a\nUserTokenReader is configured, in which case it must be non-empty and\nexactly equal to Issuer.", + "type": "string" + }, "authTokenFile": { "description": "AuthTokenFile is the path to file containing bearer token for authentication", "type": "string" @@ -448,6 +452,10 @@ "type": "array", "uniqueItems": false }, + "upstream_credential_scope": { + "description": "UpstreamCredentialScope selects the credential-lookup identity model for\nthis auth server. Only a genuinely absent value maps to session:\nKubernetes defaulting does not cover old serialized RunConfigs, direct\nvMCP YAML, or direct Go construction.", + "type": "string" + }, "upstreams": { "description": "Upstreams configures connections to upstream Identity Providers for\ninteractive authorization. It may be empty only when DelegateClients or a\nTrustedIssuer with JWTBearerGrant enables token-only operation.\nMultiple upstreams are supported for sequential authorization chains.", "items": { diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index 2f2d4067da..e123dc5fed 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -88,6 +88,13 @@ components: audience: description: Audience is the expected audience for the token type: string + authServerIssuer: + description: |- + AuthServerIssuer is the issuer of the embedded ToolHive auth server that + owns the platform-user credential namespace. It is only required when a + UserTokenReader is configured, in which case it must be non-empty and + exactly equal to Issuer. + type: string authTokenFile: description: AuthTokenFile is the path to file containing bearer token for authentication @@ -639,6 +646,13 @@ components: $ref: '#/components/schemas/tokenexchange.TrustedIssuer' type: array uniqueItems: false + upstream_credential_scope: + description: |- + UpstreamCredentialScope selects the credential-lookup identity model for + this auth server. Only a genuinely absent value maps to session: + Kubernetes defaulting does not cover old serialized RunConfigs, direct + vMCP YAML, or direct Go construction. + type: string upstreams: description: |- Upstreams configures connections to upstream Identity Providers for diff --git a/pkg/auth/token.go b/pkg/auth/token.go index 7147daefc4..57f8cdd09a 100644 --- a/pkg/auth/token.go +++ b/pkg/auth/token.go @@ -350,6 +350,17 @@ var ( ErrFailedToFetchJWKS = errors.New("failed to fetch JWKS") ErrFailedToDiscoverOIDC = errors.New("failed to discover OIDC configuration") ErrMissingIssuerAndJWKSURL = errors.New("either issuer or JWKS URL must be provided") + + // ErrAuthServerIssuerRequired rejects a UserTokenReader configuration with + // no embedded auth server issuer: credential lookup by platform-user + // identity must know which issuer owns the credential namespace. + ErrAuthServerIssuerRequired = errors.New("auth server issuer is required when a user token reader is configured") + + // ErrAuthServerIssuerMismatch rejects a UserTokenReader configuration whose + // incoming OIDC issuer does not exactly equal the embedded auth server + // issuer: the token carrying the platform-user identity must be validated + // as issued by the auth server owning the credential namespace. + ErrAuthServerIssuerMismatch = errors.New("token validator issuer does not exactly match the embedded auth server issuer") ) // TokenValidator validates JWT or opaque tokens using OIDC configuration. @@ -372,6 +383,10 @@ type TokenValidator struct { // nil means no enrichment (no embedded auth server). upstreamTokenReader upstreamtoken.TokenReader + // userTokenReader is the seam for the future platform-user credential + // lookup; nil in session mode. + userTokenReader upstreamtoken.UserTokenReader + // keyProvider provides in-process JWKS key lookups from the embedded auth // server's key provider. When set, getKeyFromJWKS resolves keys locally // before falling back to HTTP. Eliminates self-referential HTTP calls. @@ -398,6 +413,12 @@ type TokenValidatorConfig struct { // Issuer is the OIDC issuer URL (e.g., https://accounts.google.com) Issuer string + // AuthServerIssuer is the issuer of the embedded ToolHive auth server that + // owns the platform-user credential namespace. It is only required when a + // UserTokenReader is configured, in which case it must be non-empty and + // exactly equal to Issuer. + AuthServerIssuer string + // Audience is the expected audience for the token Audience string @@ -537,6 +558,7 @@ func registerIntrospectionProviders(config TokenValidatorConfig, clientSecret st type tokenValidatorOptions struct { envReader env.Reader upstreamTokenReader upstreamtoken.TokenReader + userTokenReader upstreamtoken.UserTokenReader keyProvider keys.PublicKeyProvider } @@ -562,6 +584,17 @@ func WithUpstreamTokenReader(reader upstreamtoken.TokenReader) TokenValidatorOpt } } +// WithUserTokenReader configures the token validator to look up upstream +// credentials by durable platform-user identity. It is a preparatory seam for +// the platform-user credential storage work; nothing wires a real reader yet. +// When set, the strict issuer precondition is enforced at construction: the +// configured issuer must be non-empty and exactly equal to AuthServerIssuer. +func WithUserTokenReader(reader upstreamtoken.UserTokenReader) TokenValidatorOption { + return func(o *tokenValidatorOptions) { + o.userTokenReader = reader + } +} + // WithKeyProvider configures the token validator to use an in-process key // provider for JWKS lookups instead of fetching keys over HTTP. This is used // when the embedded auth server's key provider is available in the same process, @@ -643,6 +676,26 @@ func NewTokenValidator(ctx context.Context, config TokenValidatorConfig, opts .. ) } + // Platform-user credential lookup trusts identity claims carried by tokens + // validated as issued by the embedded auth server that owns the credential + // namespace. Enforce that precondition at construction so a misconfigured + // trust relationship fails at startup instead of at request time. The + // comparison is exact: no trimming, normalization, or trailing-slash + // tolerance. + if o.userTokenReader != nil { + if config.AuthServerIssuer == "" { + return nil, ErrAuthServerIssuerRequired + } + if config.Issuer == "" { + return nil, fmt.Errorf("%w: token validator issuer is empty but embedded auth server issuer is %q", + ErrAuthServerIssuerMismatch, config.AuthServerIssuer) + } + if config.Issuer != config.AuthServerIssuer { + return nil, fmt.Errorf("%w: token validator issuer %q does not exactly match embedded auth server issuer %q", + ErrAuthServerIssuerMismatch, config.Issuer, config.AuthServerIssuer) + } + } + jwksURL, err := resolveJWKSDiscovery(config, o) if err != nil { return nil, err @@ -697,6 +750,7 @@ func NewTokenValidator(ctx context.Context, config TokenValidatorConfig, opts .. registry: registry, insecureAllowHTTP: config.InsecureAllowHTTP, upstreamTokenReader: o.upstreamTokenReader, + userTokenReader: o.userTokenReader, keyProvider: o.keyProvider, } diff --git a/pkg/auth/token_test.go b/pkg/auth/token_test.go index 5f5aaa8fee..f206777a7f 100644 --- a/pkg/auth/token_test.go +++ b/pkg/auth/token_test.go @@ -3111,3 +3111,106 @@ func TestNewTokenValidator_GoogleTokeninfoRequiresAudience(t *testing.T) { }) } } + +func TestNewTokenValidator_UserTokenReaderIssuerPrecondition(t *testing.T) { + t.Parallel() + + const ( + authServerIssuer = "https://auth.example" + otherIssuer = "https://other.example" + ) + + // skipDiscoveryEnv returns an env reader reporting + // TOOLHIVE_SKIP_OIDC_DISCOVERY=true so the constructor needs no network + // access when an explicit JWKS URL is provided. + skipDiscoveryEnv := func(t *testing.T) TokenValidatorOption { + t.Helper() + ctrl := gomock.NewController(t) + mockEnv := envmocks.NewMockReader(ctrl) + mockEnv.EXPECT().Getenv(gomock.Any()).DoAndReturn(func(key string) string { + if key == "TOOLHIVE_SKIP_OIDC_DISCOVERY" { + return "true" + } + return "" + }).AnyTimes() + return WithEnvReader(mockEnv) + } + + tests := []struct { + name string + issuer string + authServerIssuer string + jwksURL string + withUserReader bool + wantErr error + }{ + { + name: "matching issuers succeeds", + issuer: authServerIssuer, + authServerIssuer: authServerIssuer, + jwksURL: authServerIssuer + "/jwks", + withUserReader: true, + }, + { + name: "empty auth server issuer fails", + issuer: authServerIssuer, + authServerIssuer: "", + withUserReader: true, + wantErr: ErrAuthServerIssuerRequired, + }, + { + name: "empty incoming issuer fails", + issuer: "", + authServerIssuer: authServerIssuer, + withUserReader: true, + wantErr: ErrAuthServerIssuerMismatch, + }, + { + name: "mismatched issuers fail", + issuer: otherIssuer, + authServerIssuer: authServerIssuer, + withUserReader: true, + wantErr: ErrAuthServerIssuerMismatch, + }, + { + name: "trailing slash mismatch fails exactly", + issuer: authServerIssuer + "/", + authServerIssuer: authServerIssuer, + withUserReader: true, + wantErr: ErrAuthServerIssuerMismatch, + }, + { + name: "no user token reader keeps legacy behavior", + issuer: authServerIssuer, + authServerIssuer: otherIssuer, + jwksURL: authServerIssuer + "/jwks", + withUserReader: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + opts := []TokenValidatorOption{skipDiscoveryEnv(t)} + if tt.withUserReader { + opts = append(opts, WithUserTokenReader(upstreamtokenmocks.NewMockUserTokenReader(ctrl))) + } + + validator, err := NewTokenValidator(context.Background(), TokenValidatorConfig{ + Issuer: tt.issuer, + AuthServerIssuer: tt.authServerIssuer, + JWKSURL: tt.jwksURL, + Audience: "test-audience", + }, opts...) + + if tt.wantErr != nil { + require.Error(t, err) + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.NotNil(t, validator) + }) + } +} diff --git a/pkg/auth/upstreamtoken/mocks/mock_user_token_reader.go b/pkg/auth/upstreamtoken/mocks/mock_user_token_reader.go new file mode 100644 index 0000000000..bf7847ba7f --- /dev/null +++ b/pkg/auth/upstreamtoken/mocks/mock_user_token_reader.go @@ -0,0 +1,57 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/stacklok/toolhive/pkg/auth/upstreamtoken (interfaces: UserTokenReader) +// +// Generated by this command: +// +// mockgen -destination=mocks/mock_user_token_reader.go -package=mocks github.com/stacklok/toolhive/pkg/auth/upstreamtoken UserTokenReader +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + upstreamtoken "github.com/stacklok/toolhive/pkg/auth/upstreamtoken" + gomock "go.uber.org/mock/gomock" +) + +// MockUserTokenReader is a mock of UserTokenReader interface. +type MockUserTokenReader struct { + ctrl *gomock.Controller + recorder *MockUserTokenReaderMockRecorder + isgomock struct{} +} + +// MockUserTokenReaderMockRecorder is the mock recorder for MockUserTokenReader. +type MockUserTokenReaderMockRecorder struct { + mock *MockUserTokenReader +} + +// NewMockUserTokenReader creates a new mock instance. +func NewMockUserTokenReader(ctrl *gomock.Controller) *MockUserTokenReader { + mock := &MockUserTokenReader{ctrl: ctrl} + mock.recorder = &MockUserTokenReaderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUserTokenReader) EXPECT() *MockUserTokenReaderMockRecorder { + return m.recorder +} + +// GetUserUpstreamCredential mocks base method. +func (m *MockUserTokenReader) GetUserUpstreamCredential(ctx context.Context, userID, providerName string) (*upstreamtoken.UpstreamCredential, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserUpstreamCredential", ctx, userID, providerName) + ret0, _ := ret[0].(*upstreamtoken.UpstreamCredential) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserUpstreamCredential indicates an expected call of GetUserUpstreamCredential. +func (mr *MockUserTokenReaderMockRecorder) GetUserUpstreamCredential(ctx, userID, providerName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserUpstreamCredential", reflect.TypeOf((*MockUserTokenReader)(nil).GetUserUpstreamCredential), ctx, userID, providerName) +} diff --git a/pkg/auth/upstreamtoken/types.go b/pkg/auth/upstreamtoken/types.go index 90beaaac04..30878aa680 100644 --- a/pkg/auth/upstreamtoken/types.go +++ b/pkg/auth/upstreamtoken/types.go @@ -6,6 +6,7 @@ package upstreamtoken //go:generate go run go.uber.org/mock/mockgen -destination=mocks/mock_token_reader.go -package=mocks github.com/stacklok/toolhive/pkg/auth/upstreamtoken TokenReader +//go:generate go run go.uber.org/mock/mockgen -destination=mocks/mock_user_token_reader.go -package=mocks github.com/stacklok/toolhive/pkg/auth/upstreamtoken UserTokenReader import "context" @@ -75,6 +76,19 @@ type TokenReader interface { creds map[string]UpstreamCredential, failed []string, err error) } +// UserTokenReader retrieves upstream provider credentials for a durable +// platform-user identity. It is a preparatory seam for the platform-user +// credential model: it is deliberately separate from TokenReader, which +// resolves credentials per token session (tsid). A real implementation +// arrives with platform-user storage; nothing in this repository may wrap +// or reuse the session TokenReader as a UserTokenReader. +type UserTokenReader interface { + // GetUserUpstreamCredential returns the stored upstream credential for + // the given durable platform-user identity and upstream provider name. + // Returns an error if the user has no stored credential for the provider. + GetUserUpstreamCredential(ctx context.Context, userID, providerName string) (*UpstreamCredential, error) +} + // Service owns the upstream token lifecycle: read, refresh, error handling. type Service interface { // GetValidTokens returns a valid upstream credential for a session and provider. diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 7eb7d7119c..f3e54f406e 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -112,6 +112,13 @@ type RunConfig struct { //nolint:lll // field tags require full JSON+YAML names DisableUpstreamTokenInjection bool `json:"disable_upstream_token_injection,omitempty" yaml:"disable_upstream_token_injection,omitempty"` + // UpstreamCredentialScope selects the credential-lookup identity model for + // this auth server. Only a genuinely absent value maps to session: + // Kubernetes defaulting does not cover old serialized RunConfigs, direct + // vMCP YAML, or direct Go construction. + //nolint:lll // field tags require full JSON+YAML names + UpstreamCredentialScope UpstreamCredentialScope `json:"upstream_credential_scope,omitempty" yaml:"upstream_credential_scope,omitempty"` + // CIMD controls client_id metadata document support. When enabled, the // embedded authorization server accepts HTTPS URLs as client_id values // and resolves them via the CIMD protocol instead of requiring DCR. @@ -268,6 +275,19 @@ type DelegateClientRunConfig struct { // catches operator-supplied misconfiguration early so server startup fails // loudly instead of degrading silently at runtime. func (c *RunConfig) Validate() error { + // Resolve the credential-lookup scope first so an unknown value fails as + // a configuration error and platformUser fails closed before any other + // startup work runs. + scope, err := EffectiveUpstreamCredentialScope(string(c.UpstreamCredentialScope)) + if err != nil { + return err + } + if scope == UpstreamCredentialScopePlatformUser { + return fmt.Errorf( + "upstreamCredentialScope %q is not supported yet: platform-user credential storage is not implemented; "+ + "the scope is reserved for upcoming work", scope) + } + if c.CIMD != nil { if err := c.CIMD.Validate(); err != nil { return fmt.Errorf("cimd: %w", err) @@ -496,6 +516,40 @@ const ( UpstreamProviderTypeOAuth2 UpstreamProviderType = "oauth2" ) +// UpstreamCredentialScope selects which identity model upstream credential +// lookup trusts. session (permanent default) preserves today's session-based +// behavior; platformUser opts in to the future durable platform-user model. +type UpstreamCredentialScope string + +const ( + // UpstreamCredentialScopeSession keeps the session-based credential lookup + // wired as-is; no platform-user trust checks run. This is the permanent default. + UpstreamCredentialScopeSession UpstreamCredentialScope = "session" + + // UpstreamCredentialScopePlatformUser opts in to the future durable + // platform-user credential model. It is validated and propagated, but + // runtime activation fails as unsupported until platform-user storage is + // implemented. + UpstreamCredentialScopePlatformUser UpstreamCredentialScope = "platformUser" +) + +// EffectiveUpstreamCredentialScope resolves an upstreamCredentialScope value +// to its effective scope. Only a genuinely absent value maps to session; +// unknown non-empty values are rejected, never reinterpreted as the default. +func EffectiveUpstreamCredentialScope(scope string) (UpstreamCredentialScope, error) { + switch UpstreamCredentialScope(scope) { + case "": + return UpstreamCredentialScopeSession, nil + case UpstreamCredentialScopeSession: + return UpstreamCredentialScopeSession, nil + case UpstreamCredentialScopePlatformUser: + return UpstreamCredentialScopePlatformUser, nil + default: + return "", fmt.Errorf( + "invalid upstreamCredentialScope %q: must be empty, \"session\", or \"platformUser\"", scope) + } +} + // DefaultUpstreamName is the name assigned to a single unnamed upstream. const DefaultUpstreamName = "default" diff --git a/pkg/authserver/config_test.go b/pkg/authserver/config_test.go index 760842fcf1..7abd0ab13b 100644 --- a/pkg/authserver/config_test.go +++ b/pkg/authserver/config_test.go @@ -1621,3 +1621,115 @@ func TestConfig_WarnTrustedIssuerAudiences(t *testing.T) { }) } } + +func TestEffectiveUpstreamCredentialScope(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + scope string + want UpstreamCredentialScope + wantErr bool + }{ + { + name: "empty maps to session", + scope: "", + want: UpstreamCredentialScopeSession, + }, + { + name: "session passes through", + scope: "session", + want: UpstreamCredentialScopeSession, + }, + { + name: "platformUser passes through", + scope: "platformUser", + want: UpstreamCredentialScopePlatformUser, + }, + { + name: "capital Session rejected", + scope: "Session", + wantErr: true, + }, + { + name: "trailing slash rejected", + scope: "session/", + wantErr: true, + }, + { + name: "garbage rejected", + scope: "garbage", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := EffectiveUpstreamCredentialScope(tt.scope) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestRunConfigValidate_UpstreamCredentialScope(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + scope UpstreamCredentialScope + wantErr bool + check func(t *testing.T, err error) + }{ + { + name: "empty scope keeps existing validation result", + scope: "", + }, + { + name: "session scope keeps existing validation result", + scope: UpstreamCredentialScopeSession, + }, + { + name: "platformUser rejected as unsupported capability", + scope: UpstreamCredentialScopePlatformUser, + wantErr: true, + check: func(t *testing.T, err error) { + t.Helper() + require.Regexp(t, "not.*support", err.Error()) + }, + }, + { + name: "garbage scope rejected mentioning the field", + scope: UpstreamCredentialScope("garbage"), + wantErr: true, + check: func(t *testing.T, err error) { + t.Helper() + require.Contains(t, err.Error(), "upstreamCredentialScope") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + config := RunConfig{ + ScopesSupported: []string{"openid"}, + UpstreamCredentialScope: tt.scope, + } + err := config.Validate() + if !tt.wantErr { + require.NoError(t, err) + return + } + require.Error(t, err) + if tt.check != nil { + tt.check(t, err) + } + }) + } +} diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go index f89d216498..36f68e4fd5 100644 --- a/pkg/vmcp/cli/serve.go +++ b/pkg/vmcp/cli/serve.go @@ -154,6 +154,12 @@ func Serve(ctx context.Context, cfg ServeConfig) error { } } + // The operator validates this integration at reconcile time; the CLI must + // apply the same rules to directly supplied vMCP configuration. + if err := config.ValidateAuthServerIntegration(vmcpCfg, authServerRC); err != nil { + return err + } + // Auto-populate SubjectProviderName on backend auth strategies that // omitted it when an embedded auth server is active. if err := config.InjectSubjectProviderNames(vmcpCfg, authServerRC); err != nil { diff --git a/pkg/vmcp/config/validator.go b/pkg/vmcp/config/validator.go index 92ea73e1d8..09d536d854 100644 --- a/pkg/vmcp/config/validator.go +++ b/pkg/vmcp/config/validator.go @@ -646,6 +646,11 @@ func ValidateAuthServerIntegration(cfg *Config, rc *authserver.RunConfig) error return err } + // PlatformUser scope trust requirements (unknown scope values also fail here). + if err := validateAuthServerCredentialScope(cfg, rc); err != nil { + return err + } + // Auth server requires OIDC incoming auth to validate issued tokens. if err := validateAuthServerRequiresOIDC(cfg); err != nil { return err @@ -681,6 +686,34 @@ func validateAuthServerRunConfig(rc *authserver.RunConfig) error { return nil } +// validateAuthServerCredentialScope enforces the platformUser +// upstreamCredentialScope trust requirements: the incoming OIDC issuer +// must be present and exactly equal the auth server issuer. Session-mode +// configs are untouched: these checks deliberately do not run for them. +func validateAuthServerCredentialScope(cfg *Config, rc *authserver.RunConfig) error { + if rc == nil { + return nil + } + scope, err := authserver.EffectiveUpstreamCredentialScope(string(rc.UpstreamCredentialScope)) + if err != nil { + return err + } + if scope != authserver.UpstreamCredentialScopePlatformUser { + return nil + } + if !hasOIDCIncoming(cfg) { + return fmt.Errorf("upstreamCredentialScope platformUser requires OIDC incoming auth: " + + "the token carrying the platform-user identity must be validated against the embedded auth server issuer") + } + if cfg.IncomingAuth.OIDC.Issuer != rc.Issuer { + return fmt.Errorf( + "upstreamCredentialScope platformUser requires incomingAuth.oidc.issuer %q to exactly match the auth server issuer %q", + cfg.IncomingAuth.OIDC.Issuer, rc.Issuer, + ) + } + return nil +} + // validateUpstreamInjectProviders checks that every upstream_inject strategy // references a provider that exists in the auth server upstreams. func validateUpstreamInjectProviders( diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go index 4dbc2a83dc..857e15f419 100644 --- a/pkg/vmcp/config/validator_test.go +++ b/pkg/vmcp/config/validator_test.go @@ -1706,3 +1706,94 @@ func TestValidator_ValidateStaticBackends(t *testing.T) { }) } } + +func TestValidateAuthServerIntegration_UpstreamCredentialScope(t *testing.T) { + t.Parallel() + + oidcIncoming := func(issuer string) *IncomingAuthConfig { + return &IncomingAuthConfig{ + Type: IncomingAuthTypeOIDC, + OIDC: &OIDCConfig{Issuer: issuer, Audience: "https://my-vmcp"}, + } + } + inlineOutgoing := &OutgoingAuthConfig{Source: "inline"} + // validRC builds a structurally valid auth server RunConfig with the + // given scope and issuer, matching the fixtures used by + // TestValidateAuthServerIntegration. + validRC := func(scope authserver.UpstreamCredentialScope, issuer string) *authserver.RunConfig { + return &authserver.RunConfig{ + Issuer: issuer, + UpstreamCredentialScope: scope, + Upstreams: []authserver.UpstreamRunConfig{ + {Name: "default", Type: authserver.UpstreamProviderTypeOIDC}, + }, + AllowedAudiences: []string{"https://my-vmcp"}, + } + } + + tests := []struct { + name string + cfg *Config + rc *authserver.RunConfig + wantErr bool + errMsg string + }{ + { + name: "garbage scope rejected", + cfg: &Config{IncomingAuth: oidcIncoming("https://auth.example"), OutgoingAuth: inlineOutgoing}, + rc: validRC("garbage", "https://auth.example"), + wantErr: true, + errMsg: "invalid upstreamCredentialScope", + }, + { + name: "platformUser without OIDC incoming rejected", + cfg: &Config{ + IncomingAuth: &IncomingAuthConfig{Type: IncomingAuthTypeAnonymous}, + OutgoingAuth: inlineOutgoing, + }, + rc: validRC(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + wantErr: true, + errMsg: "platformUser requires OIDC incoming auth", + }, + { + name: "platformUser with mismatched incoming issuer rejected", + cfg: &Config{IncomingAuth: oidcIncoming("https://login.corp.example"), OutgoingAuth: inlineOutgoing}, + rc: validRC(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + wantErr: true, + errMsg: "to exactly match", + }, + { + name: "platformUser with exact issuer match passes", + cfg: &Config{IncomingAuth: oidcIncoming("https://auth.example"), OutgoingAuth: inlineOutgoing}, + rc: validRC(authserver.UpstreamCredentialScopePlatformUser, "https://auth.example"), + wantErr: false, + }, + { + name: "session scope with mismatched issuer keeps existing error", + cfg: &Config{IncomingAuth: oidcIncoming("https://login.corp.example"), OutgoingAuth: inlineOutgoing}, + rc: validRC(authserver.UpstreamCredentialScopeSession, "https://auth.example"), + wantErr: true, + errMsg: "auth server issuer mismatch", + }, + { + name: "empty scope with mismatched issuer keeps existing error", + cfg: &Config{IncomingAuth: oidcIncoming("https://login.corp.example"), OutgoingAuth: inlineOutgoing}, + rc: validRC("", "https://auth.example"), + wantErr: true, + errMsg: "auth server issuer mismatch", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateAuthServerIntegration(tt.cfg, tt.rc) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errMsg) + return + } + require.NoError(t, err) + }) + } +}