Skip to content

Commit 8ebe826

Browse files
committed
Document Groups field design decision
Add concise documentation explaining why Identity.Groups is intentionally not populated by OIDCIncomingAuthenticator. This clarifies that group extraction is an authorization concern handled via the Claims map, as different OIDC providers use different claim names.
1 parent 2a2c08d commit 8ebe826

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

pkg/vmcp/auth/auth.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ type Identity struct {
8585
Email string
8686

8787
// Groups are the groups this identity belongs to.
88+
//
89+
// NOTE: This field is intentionally NOT populated by OIDCIncomingAuthenticator.
90+
// Authorization logic MUST extract groups from the Claims map, as group claim
91+
// names vary by provider (e.g., "groups", "roles", "cognito:groups").
8892
Groups []string
8993

9094
// Claims contains additional claims from the auth token.

pkg/vmcp/auth/incoming_oidc.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,13 @@ func (o *OIDCIncomingAuthenticator) Middleware() func(http.Handler) http.Handler
106106
}
107107

108108
// claimsToIdentity converts JWT claims to an Identity structure.
109-
// It extracts only the universal JWT fields (sub, name, email) and stores
110-
// all claims in the Claims map. Groups extraction is left to authorization
111-
// logic that can interpret provider-specific claim structures.
112109
//
113-
// Returns an error if the required 'sub' claim is missing or invalid, as mandated
114-
// by OpenID Connect Core 1.0 specification section 5.1.
110+
// Groups are intentionally NOT extracted here because:
111+
// - OIDC providers use different claim names ("groups", "roles", etc.)
112+
// - Group extraction is an authorization concern, not authentication
113+
// - Authorization policies access groups via the Claims map
114+
//
115+
// Returns an error if the required 'sub' claim is missing or invalid.
115116
func claimsToIdentity(claims jwt.MapClaims) (*Identity, error) {
116117
identity := &Identity{
117118
Claims: make(map[string]any),

0 commit comments

Comments
 (0)