Resolve authz ConfigMap for VirtualMCPServer

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go

@@ -232,6 +232,26 @@ type EmbeddedAuthServerConfig struct {
 	// +listMapKey=name
 	UpstreamProviders []UpstreamProviderConfig `json:"upstreamProviders"`
 
+	// 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.
+	// +optional
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`
+	PrimaryUpstreamProvider string `json:"primaryUpstreamProvider,omitempty"`
+
 	// Storage configures the storage backend for the embedded auth server.
 	// If not specified, defaults to in-memory storage.
 	// +optional

cmd/thv-operator/api/v1beta1/mcpserver_types.go

@@ -96,6 +96,12 @@ const (
 	// when spec.authzConfig.inline.primaryUpstreamProvider is non-empty on a CR type
 	// that has no embedded auth server (MCPServer / MCPRemoteProxy). The field has
 	// no effect on those resources and is documented as VirtualMCPServer-only.
+	//
+	// Tied to the deprecated InlineAuthzConfig.PrimaryUpstreamProvider field
+	// (see mcpserver_types.go). When that field is removed at end of the
+	// deprecation cycle, this condition and ConditionReasonAuthzPrimaryUpstreamProviderIgnored
+	// below should be removed in the same change: there is no other path that
+	// fires this advisory.
 	ConditionTypeAuthzPrimaryUpstreamProviderIgnored = "AuthzPrimaryUpstreamProviderIgnored"
 )
 
@@ -652,16 +658,46 @@ type AuthzConfigRef struct {
 	// Only used when Type is "inline"
 	// +optional
 	Inline *InlineAuthzConfig `json:"inline,omitempty"`
+
+	// GroupClaimName is the JWT claim key that contains group membership for the
+	// principal. When set, takes priority over the well-known defaults
+	// ("groups", "roles", "cognito:groups"). Use this for IDPs that place
+	// groups under a URI-style claim (e.g. "https://example.com/groups"). When
+	// Type is "configMap", a group_claim_name entry in the referenced ConfigMap
+	// is overridden by this field if both are set.
+	// +optional
+	// +kubebuilder:validation:MaxLength=253
+	GroupClaimName string `json:"groupClaimName,omitempty"`
+
+	// RoleClaimName is the JWT claim key that contains role membership for the
+	// principal. When set, the claim is extracted separately from GroupClaimName
+	// and both are mapped to the configured GroupEntityType. When Type is
+	// "configMap", a role_claim_name entry in the referenced ConfigMap is
+	// overridden by this field if both are set.
+	// +optional
+	// +kubebuilder:validation:MaxLength=253
+	RoleClaimName string `json:"roleClaimName,omitempty"`
+
+	// GroupEntityType is the Cedar entity type name used for principal parent
+	// UIDs synthesised from JWT group/role claims. Defaults to "THVGroup" when
+	// empty. Must match the entity type used in the static entity store for
+	// transitive `in` checks (e.g. `ClaimGroup → PlatformRole`) to resolve.
+	// Namespaced names (`Foo::Bar`) are not yet supported. When Type is
+	// "configMap", a group_entity_type entry in the referenced ConfigMap is
+	// overridden by this field if both are set.
+	// +optional
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:Pattern=`^[A-Za-z_][A-Za-z0-9_]*$`
+	GroupEntityType string `json:"groupEntityType,omitempty"`
 }
 
-// ExplicitPrimaryUpstreamProvider returns the user-specified primary upstream
-// provider name from the authz config, or "" if none is set.
-//
-// Currently reads from inline config only. ConfigMap-sourced authz needs to
-// load and parse the referenced ConfigMap; until that path lands (see the
-// matching TODO in cmd/thv-operator/pkg/vmcpconfig/converter.go), configMap
-// users always fall through to auto-selection of the first upstream.
-func (r *AuthzConfigRef) ExplicitPrimaryUpstreamProvider() string {
+// DeprecatedInlinePrimaryUpstreamProvider returns the legacy inline
+// PrimaryUpstreamProvider value, or "" when the field or the AuthzConfigRef
+// is nil. The field has moved to spec.authServerConfig.primaryUpstreamProvider
+// on VirtualMCPServer; this accessor is the single read point for the
+// deprecated location so callers can emit a deprecation warning when it
+// returns a non-empty value.
+func (r *AuthzConfigRef) DeprecatedInlinePrimaryUpstreamProvider() string {
 	if r == nil || r.Inline == nil {
 		return ""
 	}
@@ -736,30 +772,39 @@ func (r *MCPGroupRef) GetName() string {
 	return r.Name
 }
 
-// InlineAuthzConfig contains direct authorization configuration
+// InlineAuthzConfig contains direct authorization configuration.
+//
+// Source-agnostic Cedar JWT-claim mapping settings (GroupClaimName,
+// RoleClaimName, GroupEntityType) live on the parent AuthzConfigRef so they
+// work the same way for inline and configMap-sourced authz.
 type InlineAuthzConfig struct {
 	// Policies is a list of Cedar policy strings
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:MinItems=1
 	// +listType=atomic
 	Policies []string `json:"policies"`
 
-	// EntitiesJSON is a JSON string representing Cedar entities
+	// EntitiesJSON is a JSON string representing Cedar entities. Required when
+	// transitive policies (e.g. `ClaimGroup → PlatformRole`) need a static
+	// entity store; defaults to "[]".
 	// +kubebuilder:default="[]"
 	// +optional
 	EntitiesJSON string `json:"entitiesJson,omitempty"`
 
-	// PrimaryUpstreamProvider names the upstream IDP whose access token's claims
-	// Cedar should evaluate. Currently honored only when the parent
-	// AuthzConfigRef.Type is "inline"; configMap-sourced policies will support
-	// this in a future release (see #5208). Only meaningful for VirtualMCPServer
-	// with an embedded auth server. When empty and an embedded auth server has
-	// upstreams configured, the controller defaults to the first upstream
-	// provider. The name must match one of the upstreams declared on
-	// spec.authServerConfig.upstreamProviders; otherwise the VirtualMCPServer is
-	// rejected with AuthServerConfigValidated=False. MCPServer and MCPRemoteProxy
-	// have no embedded auth server; setting this field on those CRs surfaces an
-	// AuthzPrimaryUpstreamProviderIgnored advisory condition on the resource.
+	// PrimaryUpstreamProvider names the upstream IDP whose access token's
+	// claims Cedar should evaluate.
+	//
+	// Deprecated: on VirtualMCPServer this field has moved to
+	// spec.authServerConfig.primaryUpstreamProvider. The old location is
+	// still read for one release for backward compatibility; the
+	// VirtualMCPServer controller emits an AuthzPrimaryUpstreamProviderDeprecated
+	// Warning event whenever it is consumed, and removal is planned for the
+	// release after the deprecation cycle.
+	//
+	// On MCPServer and MCPRemoteProxy this field has always been a structural
+	// no-op (those CRDs do not run an embedded auth server). Setting it
+	// continues to surface the AuthzPrimaryUpstreamProviderIgnored advisory
+	// condition; the deprecation does not change that behaviour.
 	// +optional
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=63

cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go

@@ -338,6 +338,13 @@ const (
 	// ConditionReasonIncomingAuthInvalid indicates incoming auth is invalid
 	ConditionReasonIncomingAuthInvalid = "IncomingAuthInvalid"
 
+	// Note: ConditionReasonAuthzConfigMapNotFound is shared with MCPRemoteProxy and is
+	// declared in mcpremoteproxy_types.go.
+
+	// ConditionReasonAuthzConfigMapInvalid indicates the referenced authz ConfigMap was
+	// found but its payload is missing/empty/malformed or fails Cedar validation.
+	ConditionReasonAuthzConfigMapInvalid = "AuthzConfigMapInvalid"
+
 	// ConditionReasonGroupRefValid indicates the GroupRef is valid
 	ConditionReasonVirtualMCPServerGroupRefValid = "GroupRefValid"
 
@@ -501,6 +508,28 @@ func (r *VirtualMCPServer) ResolveGroupName() string {
 	return r.Spec.GroupRef.GetName()
 }
 
+// ExplicitPrimaryUpstreamProvider returns the user-configured primary upstream
+// provider name and a flag indicating whether the value came from the
+// deprecated spec.incomingAuth.authzConfig.inline.primaryUpstreamProvider
+// location (fromDeprecated=true) or the canonical
+// spec.authServerConfig.primaryUpstreamProvider location (fromDeprecated=false).
+// Returns ("", false) when neither location is set.
+//
+// Precedence: the canonical location wins if set; the deprecated location is
+// read only as a backward-compatibility fallback. Callers should emit a
+// Warning event when fromDeprecated is true.
+func (r *VirtualMCPServer) ExplicitPrimaryUpstreamProvider() (name string, fromDeprecated bool) {
+	if r.Spec.AuthServerConfig != nil && r.Spec.AuthServerConfig.PrimaryUpstreamProvider != "" {
+		return r.Spec.AuthServerConfig.PrimaryUpstreamProvider, false
+	}
+	if r.Spec.IncomingAuth != nil {
+		if dep := r.Spec.IncomingAuth.AuthzConfig.DeprecatedInlinePrimaryUpstreamProvider(); dep != "" {
+			return dep, true
+		}
+	}
+	return "", false
+}
+
 // Validate performs validation for VirtualMCPServer
 // This method is called by the controller during reconciliation
 func (r *VirtualMCPServer) Validate() error {

cmd/thv-operator/api/v1beta1/virtualmcpserver_types_test.go

@@ -633,3 +633,132 @@ func TestVirtualMCPServer_ResolveGroupName(t *testing.T) {
 		})
 	}
 }
+
+// TestVirtualMCPServer_ExplicitPrimaryUpstreamProvider locks the precedence
+// between the canonical spec.authServerConfig.primaryUpstreamProvider location
+// and the deprecated spec.incomingAuth.authzConfig.inline.primaryUpstreamProvider
+// fallback. The returned fromDeprecated flag is the signal callers use to emit
+// the AuthzPrimaryUpstreamProviderDeprecated warning event, so its semantics
+// matter beyond the name string.
+func TestVirtualMCPServer_ExplicitPrimaryUpstreamProvider(t *testing.T) {
+	t.Parallel()
+
+	withCanonical := func(primary string) *EmbeddedAuthServerConfig {
+		return &EmbeddedAuthServerConfig{
+			UpstreamProviders:       []UpstreamProviderConfig{{Name: "okta"}, {Name: "github"}},
+			PrimaryUpstreamProvider: primary,
+		}
+	}
+	withDeprecatedInline := func(primary string) *IncomingAuthConfig {
+		return &IncomingAuthConfig{
+			Type: "oidc",
+			AuthzConfig: &AuthzConfigRef{
+				Type: "inline",
+				Inline: &InlineAuthzConfig{
+					Policies:                []string{`permit(principal, action, resource);`},
+					PrimaryUpstreamProvider: primary,
+				},
+			},
+		}
+	}
+
+	tests := []struct {
+		name               string
+		authServerConfig   *EmbeddedAuthServerConfig
+		incomingAuth       *IncomingAuthConfig
+		wantName           string
+		wantFromDeprecated bool
+	}{
+		{
+			name:               "neither location set returns empty",
+			authServerConfig:   &EmbeddedAuthServerConfig{},
+			incomingAuth:       &IncomingAuthConfig{Type: "anonymous"},
+			wantName:           "",
+			wantFromDeprecated: false,
+		},
+		{
+			name:               "canonical set returns canonical value with fromDeprecated=false",
+			authServerConfig:   withCanonical("github"),
+			incomingAuth:       &IncomingAuthConfig{Type: "anonymous"},
+			wantName:           "github",
+			wantFromDeprecated: false,
+		},
+		{
+			name:               "deprecated inline set returns inline value with fromDeprecated=true",
+			authServerConfig:   nil,
+			incomingAuth:       withDeprecatedInline("okta"),
+			wantName:           "okta",
+			wantFromDeprecated: true,
+		},
+		{
+			name:               "canonical wins when both are set",
+			authServerConfig:   withCanonical("github"),
+			incomingAuth:       withDeprecatedInline("okta"),
+			wantName:           "github",
+			wantFromDeprecated: false,
+		},
+		{
+			name:               "nil authServerConfig falls through to deprecated inline",
+			authServerConfig:   nil,
+			incomingAuth:       withDeprecatedInline("okta"),
+			wantName:           "okta",
+			wantFromDeprecated: true,
+		},
+		{
+			name:               "nil IncomingAuth with empty canonical returns empty",
+			authServerConfig:   &EmbeddedAuthServerConfig{},
+			incomingAuth:       nil,
+			wantName:           "",
+			wantFromDeprecated: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			vmcp := &VirtualMCPServer{
+				Spec: VirtualMCPServerSpec{
+					AuthServerConfig: tt.authServerConfig,
+					IncomingAuth:     tt.incomingAuth,
+				},
+			}
+			gotName, gotFromDeprecated := vmcp.ExplicitPrimaryUpstreamProvider()
+			assert.Equal(t, tt.wantName, gotName, "name mismatch")
+			assert.Equal(t, tt.wantFromDeprecated, gotFromDeprecated, "fromDeprecated mismatch")
+		})
+	}
+}
+
+// TestAuthzConfigRef_DeprecatedInlinePrimaryUpstreamProvider validates the
+// helper that reads the legacy InlineAuthzConfig.PrimaryUpstreamProvider field.
+// Callers depend on the empty-string return for nil receivers and nil
+// Inline subtrees to keep the deprecation-fallback path safe.
+func TestAuthzConfigRef_DeprecatedInlinePrimaryUpstreamProvider(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		ref  *AuthzConfigRef
+		want string
+	}{
+		{name: "nil receiver", ref: nil, want: ""},
+		{name: "nil Inline", ref: &AuthzConfigRef{Type: "configMap"}, want: ""},
+		{name: "Inline without primary", ref: &AuthzConfigRef{
+			Type:   "inline",
+			Inline: &InlineAuthzConfig{Policies: []string{`permit(principal, action, resource);`}},
+		}, want: ""},
+		{name: "Inline with primary set", ref: &AuthzConfigRef{
+			Type: "inline",
+			Inline: &InlineAuthzConfig{
+				Policies:                []string{`permit(principal, action, resource);`},
+				PrimaryUpstreamProvider: "okta",
+			},
+		}, want: "okta"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, tt.ref.DeprecatedInlinePrimaryUpstreamProvider())
+		})
+	}
+}

cmd/thv-operator/controllers/mcpremoteproxy_controller.go

@@ -1091,7 +1091,7 @@ func (r *MCPRemoteProxyReconciler) validateGroupRef(ctx context.Context, proxy *
 // Mirrors the validateGroupRef convention: this only sets/removes the
 // condition; the caller is responsible for persisting status.
 func (*MCPRemoteProxyReconciler) validateAuthzPrimaryUpstreamProviderIgnored(proxy *mcpv1beta1.MCPRemoteProxy) {
-	provider := proxy.Spec.AuthzConfig.ExplicitPrimaryUpstreamProvider()
+	provider := proxy.Spec.AuthzConfig.DeprecatedInlinePrimaryUpstreamProvider()
 	conditionType := mcpv1beta1.ConditionTypeAuthzPrimaryUpstreamProviderIgnored
 	if provider == "" {
 		meta.RemoveStatusCondition(&proxy.Status.Conditions, conditionType)