From 6b65fbd1b10c9a459c519d355db42c2e01c197aa Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Apr 2026 09:56:58 +0100 Subject: [PATCH] Add RoleClaimName to Cedar ConfigOptions Add RoleClaimName field to ConfigOptions and wire it through to the Authorizer struct, complementing the existing GroupClaimName. This supports IdPs that separate group and role concepts (e.g. Entra ID "groups" vs "roles" claims). When empty (the default), no role extraction occurs -- backward compatible. The field is config plumbing only; actual extraction and deduplication with GroupClaimName is handled by #4768. E2E tested against real IDP tokens in a Kind cluster with the operator built from this branch: - Okta: Authorization server with a custom "groups" claim configured to emit group display names in access tokens. OIDC app uses authorization code flow with "mcpserver" as audience. RoleClaimName left empty -- confirms backward compat. JWT shape: { "groups": ["Everyone", "engineering"], "sub": "jakub@stacklok.com" } Cedar policy: permit(principal in THVGroup::"engineering", action, resource in MCP::"okta-group-test"); - Entra ID: App registration with two app roles ("mcp-admin", "developer") assigned to a test user. The access token carries roles but no "groups" claim (no group claim in token config). RoleClaimName set to "roles". JWT shape: { "roles": ["mcp-admin", "developer"], "sub": "cljQrWT58zYW..." } Cedar policies: permit(principal in THVGroup::"developer", action == Action::"list_tools", resource in MCP::"entra-role-test"); permit(principal in THVGroup::"mcp-admin", action == Action::"call_tool", resource in MCP::"entra-role-test") when { resource.name == "echo" }; Closes #4763 Co-Authored-By: Claude Opus 4.6 --- pkg/authz/authorizers/cedar/core.go | 10 +++ pkg/authz/authorizers/cedar/core_test.go | 104 ++++++++++++++++++++--- 2 files changed, 102 insertions(+), 12 deletions(-) diff --git a/pkg/authz/authorizers/cedar/core.go b/pkg/authz/authorizers/cedar/core.go index bc1eb0a964..4c173085b4 100644 --- a/pkg/authz/authorizers/cedar/core.go +++ b/pkg/authz/authorizers/cedar/core.go @@ -164,6 +164,9 @@ type Authorizer struct { // groupClaimName is the JWT claim key that contains group membership. // When empty, the well-known defaults are checked ("groups", "roles", etc.). groupClaimName string + // roleClaimName is the JWT claim key that contains role membership. + // When empty, no role extraction is performed (backward compatible). + roleClaimName string // claimKeyLog rate-limits the diagnostic log of resolved JWT claim keys // so it emits at most once per 30 seconds instead of once per authorization check. claimKeyLog *syncutil.AtMost @@ -189,6 +192,12 @@ type ConfigOptions struct { // under a URI-style claim (e.g. "https://example.com/groups" in Auth0/Okta). // When empty, only the well-known claim names are checked. GroupClaimName string `json:"group_claim_name,omitempty" yaml:"group_claim_name,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 Cedar THVGroup entities. + // When empty, no role extraction is performed (backward compatible). + RoleClaimName string `json:"role_claim_name,omitempty" yaml:"role_claim_name,omitempty"` } // NewCedarAuthorizer creates a new Cedar authorizer. @@ -199,6 +208,7 @@ func NewCedarAuthorizer(options ConfigOptions) (authorizers.Authorizer, error) { entityFactory: NewEntityFactory(), primaryUpstreamProvider: options.PrimaryUpstreamProvider, groupClaimName: options.GroupClaimName, + roleClaimName: options.RoleClaimName, claimKeyLog: syncutil.NewAtMost(30 * time.Second), } diff --git a/pkg/authz/authorizers/cedar/core_test.go b/pkg/authz/authorizers/cedar/core_test.go index f3542408c0..92976cbff6 100644 --- a/pkg/authz/authorizers/cedar/core_test.go +++ b/pkg/authz/authorizers/cedar/core_test.go @@ -5,6 +5,7 @@ package cedar import ( "context" + "encoding/json" "testing" "github.com/golang-jwt/jwt/v5" @@ -32,11 +33,13 @@ func TestNewCedarAuthorizer(t *testing.T) { // Test cases testCases := []struct { - name string - policies []string - entitiesJSON string - expectError bool - errorType error + name string + policies []string + entitiesJSON string + roleClaimName string + expectError bool + errorType error + wantRoleClaimName string }{ { name: "Valid policy and empty entities", @@ -75,6 +78,22 @@ func TestNewCedarAuthorizer(t *testing.T) { entitiesJSON: `[{"uid": {"type": "User", "id": "alice"}, "attrs": {}, "parents": []}]`, expectError: false, }, + { + name: "Stores configured role claim", + policies: []string{`permit(principal, action, resource);`}, + entitiesJSON: `[]`, + roleClaimName: "roles", + expectError: false, + wantRoleClaimName: "roles", + }, + { + name: "Stores URI-style role claim", + policies: []string{`permit(principal, action, resource);`}, + entitiesJSON: `[]`, + roleClaimName: "https://example.com/roles", + expectError: false, + wantRoleClaimName: "https://example.com/roles", + }, } // Run test cases @@ -83,8 +102,9 @@ func TestNewCedarAuthorizer(t *testing.T) { t.Parallel() // Create a Cedar authorizer authorizer, err := NewCedarAuthorizer(ConfigOptions{ - Policies: tc.policies, - EntitiesJSON: tc.entitiesJSON, + Policies: tc.policies, + EntitiesJSON: tc.entitiesJSON, + RoleClaimName: tc.roleClaimName, }) // Check error expectations @@ -97,6 +117,10 @@ func TestNewCedarAuthorizer(t *testing.T) { } else { assert.NoError(t, err, "Unexpected error: %v", err) require.NotNil(t, authorizer, "Cedar authorizer is nil") + + cedarAuthz, ok := authorizer.(*Authorizer) + require.True(t, ok) + assert.Equal(t, tc.wantRoleClaimName, cedarAuthz.roleClaimName) } }) } @@ -1563,11 +1587,12 @@ func TestInjectUpstreamProvider(t *testing.T) { }, }, { - // GroupClaimName must survive the serialise→deserialise round-trip - // that InjectUpstreamProvider performs internally. A refactor that - // reconstructed ConfigOptions from scratch (populating only known - // fields) would silently drop GroupClaimName without this test. - name: "group_claim_name_preserved_after_inject", + // GroupClaimName and RoleClaimName must survive the + // serialise→deserialise round-trip that InjectUpstreamProvider + // performs internally. A refactor that reconstructed ConfigOptions + // from scratch (populating only known fields) would silently drop + // these claim name fields without this test. + name: "claim_names_preserved_after_inject", setup: func(t *testing.T) *authorizers.Config { t.Helper() cfg, err := authorizers.NewConfig(Config{ @@ -1577,6 +1602,7 @@ func TestInjectUpstreamProvider(t *testing.T) { Policies: []string{`permit(principal, action, resource);`}, EntitiesJSON: "[]", GroupClaimName: "https://example.com/groups", + RoleClaimName: "https://example.com/roles", }, }) require.NoError(t, err) @@ -1590,6 +1616,8 @@ func TestInjectUpstreamProvider(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://example.com/groups", extracted.Options.GroupClaimName, "GroupClaimName must be unchanged after InjectUpstreamProvider") + assert.Equal(t, "https://example.com/roles", extracted.Options.RoleClaimName, + "RoleClaimName must be unchanged after InjectUpstreamProvider") assert.Equal(t, "my-provider", extracted.Options.PrimaryUpstreamProvider, "PrimaryUpstreamProvider must be set by InjectUpstreamProvider") }, @@ -1780,3 +1808,55 @@ func TestExtractGroupsFromClaims(t *testing.T) { }) } } + +// TestConfigOptionsRoleClaimNameJSON verifies JSON marshal/unmarshal of the +// RoleClaimName field, including backward compatibility when the field is absent. +func TestConfigOptionsRoleClaimNameJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + jsonInput string + wantRole string + wantOmitOnMar bool // when true, marshal output must NOT contain "role_claim_name" + }{ + { + name: "present", + jsonInput: `{"policies":["permit(principal,action,resource);"],"role_claim_name":"roles"}`, + wantRole: "roles", + wantOmitOnMar: false, + }, + { + name: "absent_gives_empty_string", + jsonInput: `{"policies":["permit(principal,action,resource);"]}`, + wantRole: "", + wantOmitOnMar: true, + }, + { + name: "uri_style_claim", + jsonInput: `{"policies":["permit(principal,action,resource);"],"role_claim_name":"https://example.com/roles"}`, + wantRole: "https://example.com/roles", + wantOmitOnMar: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var opts ConfigOptions + err := json.Unmarshal([]byte(tt.jsonInput), &opts) + require.NoError(t, err) + assert.Equal(t, tt.wantRole, opts.RoleClaimName) + + marshalled, err := json.Marshal(opts) + require.NoError(t, err) + if tt.wantOmitOnMar { + assert.NotContains(t, string(marshalled), "role_claim_name", + "empty RoleClaimName must be omitted from JSON output") + } else { + assert.Contains(t, string(marshalled), "role_claim_name") + } + }) + } +}