From c479ba6386786a897dec8e4bf7d51c7eb0e14e69 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 1 Mar 2022 14:07:39 +1100 Subject: [PATCH] fix(oidc): make preferred_username a profile scope claim (#2930) This corrects an issue with the preferred_username which should be part of the profile scope as per https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims. Introduced in ddbb21a via #2829 --- docs/configuration/identity-providers/oidc.md | 12 ++--- internal/handlers/oidc.go | 5 +-- internal/handlers/oidc_test.go | 45 ++++++++----------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/docs/configuration/identity-providers/oidc.md b/docs/configuration/identity-providers/oidc.md index a72b3945..3fa8be18 100644 --- a/docs/configuration/identity-providers/oidc.md +++ b/docs/configuration/identity-providers/oidc.md @@ -514,7 +514,7 @@ individual user. Please use the claim `preferred_username` instead._ | Claim | JWT Type | Authelia Attribute | Description | |:------------------:|:-------------:|:------------------:|:---------------------------------------------:| -| sub | string | Username | The username the user used to login with | +| sub | string | username | The username the user used to login with | | scope | string | scopes | Granted scopes (space delimited) | | scp | array[string] | scopes | Granted scopes | | iss | string | hostname | The issuer name, determined by URL | @@ -525,7 +525,6 @@ individual user. Please use the claim `preferred_username` instead._ | rat | number | _N/A_ | The time when the token was requested | | iat | number | _N/A_ | The time when the token was issued | | jti | string(uuid) | _N/A_ | JWT Identifier | -| preferred_username | string | Username | The username the user used to login with | ### groups @@ -533,7 +532,7 @@ This scope includes the groups the authentication backend reports the user is a | Claim | JWT Type | Authelia Attribute | Description | |:------:|:-------------:|:------------------:|:----------------------:| -| groups | array[string] | Groups | The users display name | +| groups | array[string] | groups | The users display name | ### email @@ -549,9 +548,10 @@ This scope includes the email information the authentication backend reports abo This scope includes the profile information the authentication backend reports about the user in the token. -| Claim | JWT Type | Authelia Attribute | Description | -|:-----:|:--------:|:------------------:|:----------------------:| -| name | string | display_name | The users display name | +| Claim | JWT Type | Authelia Attribute | Description | +|:------------------:|:--------:|:------------------:|:----------------------------------------:| +| preferred_username | string | username | The username the user used to login with | +| name | string | display_name | The users display name | ## Endpoint Implementations diff --git a/internal/handlers/oidc.go b/internal/handlers/oidc.go index aa9c961e..40086505 100644 --- a/internal/handlers/oidc.go +++ b/internal/handlers/oidc.go @@ -33,9 +33,7 @@ func newOpenIDSession(subject string) *oidc.OpenIDSession { } func oidcGrantRequests(ar fosite.AuthorizeRequester, scopes, audiences []string, userSession *session.UserSession) (extraClaims map[string]interface{}) { - extraClaims = map[string]interface{}{ - oidc.ClaimPreferredUsername: userSession.Username, - } + extraClaims = map[string]interface{}{} for _, scope := range scopes { if ar != nil { @@ -46,6 +44,7 @@ func oidcGrantRequests(ar fosite.AuthorizeRequester, scopes, audiences []string, case oidc.ScopeGroups: extraClaims[oidc.ClaimGroups] = userSession.Groups case oidc.ScopeProfile: + extraClaims[oidc.ClaimPreferredUsername] = userSession.Username extraClaims[oidc.ClaimDisplayName] = userSession.DisplayName case oidc.ScopeEmail: if len(userSession.Emails) != 0 { diff --git a/internal/handlers/oidc_test.go b/internal/handlers/oidc_test.go index 7a504121..6ff9947f 100644 --- a/internal/handlers/oidc_test.go +++ b/internal/handlers/oidc_test.go @@ -34,47 +34,41 @@ func TestShouldDetectIfConsentIsMissing(t *testing.T) { assert.True(t, isConsentMissing(workflow, requestedScopes, requestedAudience)) } -func TestShouldGrantAppropriateClaimsForScopeOpenID(t *testing.T) { - extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeOpenID}, []string{}, &oidcUserSessionJohn) - - assert.Len(t, extraClaims, 1) - - require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) - assert.Equal(t, "john", extraClaims[oidc.ClaimPreferredUsername]) -} - -func TestShouldGrantAppropriateClaimsForScopeOpenIDAndGroups(t *testing.T) { - extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeGroups}, []string{}, &oidcUserSessionJohn) +func TestShouldGrantAppropriateClaimsForScopeProfile(t *testing.T) { + extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeProfile}, []string{}, &oidcUserSessionJohn) assert.Len(t, extraClaims, 2) require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) assert.Equal(t, "john", extraClaims[oidc.ClaimPreferredUsername]) + require.Contains(t, extraClaims, oidc.ClaimDisplayName) + assert.Equal(t, "John Smith", extraClaims[oidc.ClaimDisplayName]) +} + +func TestShouldGrantAppropriateClaimsForScopeGroups(t *testing.T) { + extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeGroups}, []string{}, &oidcUserSessionJohn) + + assert.Len(t, extraClaims, 1) + require.Contains(t, extraClaims, oidc.ClaimGroups) assert.Len(t, extraClaims[oidc.ClaimGroups], 2) assert.Contains(t, extraClaims[oidc.ClaimGroups], "admin") assert.Contains(t, extraClaims[oidc.ClaimGroups], "dev") - extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeGroups}, []string{}, &oidcUserSessionFred) + extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeGroups}, []string{}, &oidcUserSessionFred) - assert.Len(t, extraClaims, 2) - - require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) - assert.Equal(t, "fred", extraClaims[oidc.ClaimPreferredUsername]) + assert.Len(t, extraClaims, 1) require.Contains(t, extraClaims, oidc.ClaimGroups) assert.Len(t, extraClaims[oidc.ClaimGroups], 1) assert.Contains(t, extraClaims[oidc.ClaimGroups], "dev") } -func TestShouldGrantAppropriateClaimsForScopeOpenIDAndEmail(t *testing.T) { - extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeEmail}, []string{}, &oidcUserSessionJohn) +func TestShouldGrantAppropriateClaimsForScopeEmail(t *testing.T) { + extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeEmail}, []string{}, &oidcUserSessionJohn) - assert.Len(t, extraClaims, 4) - - require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) - assert.Equal(t, "john", extraClaims[oidc.ClaimPreferredUsername]) + assert.Len(t, extraClaims, 3) require.Contains(t, extraClaims, oidc.ClaimEmail) assert.Equal(t, "j.smith@authelia.com", extraClaims[oidc.ClaimEmail]) @@ -86,12 +80,9 @@ func TestShouldGrantAppropriateClaimsForScopeOpenIDAndEmail(t *testing.T) { require.Contains(t, extraClaims, oidc.ClaimEmailVerified) assert.Equal(t, true, extraClaims[oidc.ClaimEmailVerified]) - extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeEmail}, []string{}, &oidcUserSessionFred) + extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeEmail}, []string{}, &oidcUserSessionFred) - assert.Len(t, extraClaims, 3) - - require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) - assert.Equal(t, "fred", extraClaims[oidc.ClaimPreferredUsername]) + assert.Len(t, extraClaims, 2) require.Contains(t, extraClaims, oidc.ClaimEmail) assert.Equal(t, "f.smith@authelia.com", extraClaims[oidc.ClaimEmail])