From 06641cd15a07af5cd7d1388b2bb1f8c96eda6b72 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 18 Jan 2022 20:32:06 +1100 Subject: [PATCH] fix(oidc): add preferred username claim (#2801) This adds the missing preferred username claim to the ID Token for OIDC. Fixes #2798 --- docs/configuration/identity-providers/oidc.md | 98 ++++++++-------- .../handlers/handler_oidc_authorization.go | 38 +------ internal/handlers/oidc.go | 41 +++++++ internal/handlers/oidc_test.go | 106 ++++++++++++++++++ internal/oidc/const.go | 18 +++ 5 files changed, 218 insertions(+), 83 deletions(-) diff --git a/docs/configuration/identity-providers/oidc.md b/docs/configuration/identity-providers/oidc.md index d17de97a3..4229c2cea 100644 --- a/docs/configuration/identity-providers/oidc.md +++ b/docs/configuration/identity-providers/oidc.md @@ -8,9 +8,9 @@ nav_order: 2 # OpenID Connect -**Authelia** currently supports the [OpenID Connect] OP role as a [**beta**](#roadmap) feature. The OP role is the -[OpenID Connect] Provider role, not the Relying Party or RP role. This means other applications that implement the -[OpenID Connect] RP role can use Authelia as an authentication and authorization backend similar to how you may use +**Authelia** currently supports the [OpenID Connect] OP role as a [**beta**](#roadmap) feature. The OP role is the +[OpenID Connect] Provider role, not the Relying Party or RP role. This means other applications that implement the +[OpenID Connect] RP role can use Authelia as an authentication and authorization backend similar to how you may use social media or development platforms for login. The Relying Party role is the role which allows an application to use GitHub, Google, or other [OpenID Connect] @@ -19,10 +19,10 @@ providers for authentication and authorization. We do not intend to support this ## Roadmap We have decided to implement [OpenID Connect] as a beta feature, it's suggested you only utilize it for testing and -providing feedback, and should take caution in relying on it in production as of now. [OpenID Connect] and it's related endpoints -are not enabled by default unless you specifically configure the [OpenID Connect] section. +providing feedback, and should take caution in relying on it in production as of now. [OpenID Connect] and it's related +endpoints are not enabled by default unless you specifically configure the [OpenID Connect] section. -As [OpenID Connect] is fairly complex (the [OpenID Connect] Provider role especially so) it's intentional that it is +As [OpenID Connect] is fairly complex (the [OpenID Connect] Provider role especially so) it's intentional that it is both a beta and that the implemented features are part of a thoughtful roadmap. Items that are not immediately obvious as required (i.e. bug fixes or spec features), will likely be discussed in team meetings or on GitHub issues before being added to the list. We want to implement this feature in a very thoughtful way in order to avoid security issues. @@ -257,7 +257,7 @@ The maximum lifetime of a refresh token. The refresh token can be used to obtain new refresh tokens as well as access tokens or id tokens with an up-to-date expiration. For more information read these docs about [token lifespan]. -A good starting point is 50% more or 30 minutes more (which ever is less) time than the highest lifespan out of the +A good starting point is 50% more or 30 minutes more (which ever is less) time than the highest lifespan out of the [access token lifespan](#access_token_lifespan), the [authorize code lifespan](#authorize_code_lifespan), and the [id token lifespan](#id_token_lifespan). For instance the default for all of these is 60 minutes, so the default refresh token lifespan is 90 minutes. @@ -348,9 +348,9 @@ required: no {: .label .label-config .label-green } -This enables the public client type for this client. This is for clients that are not capable of maintaining +This enables the public client type for this client. This is for clients that are not capable of maintaining confidentiality of credentials, you can read more about client types in [RFC6749](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1). -This is particularly useful for SPA's and CLI tools. This option requires setting the [client secret](#secret) to a +This is particularly useful for SPA's and CLI tools. This option requires setting the [client secret](#secret) to a blank string. In addition to the standard rules for redirect URIs, public clients can use the `urn:ietf:wg:oauth:2.0:oob` redirect URI. @@ -412,7 +412,7 @@ their redirect URIs are as follows: 1. If a client attempts to authorize with Authelia and its redirect URI is not listed in the client configuration the attempt to authorize wil fail and an error will be generated. -2. The redirect URIs are case-sensitive. +2. The redirect URIs are case-sensitive. 3. The URI must include a scheme and that scheme must be one of `http` or `https`. 4. The client can ignore rule 3 and use `urn:ietf:wg:oauth:2.0:oob` if it is a [public](#public) client type. @@ -442,7 +442,7 @@ required: no {: .label .label-config .label-green } -A list of response types this client can return. _It is recommended that this isn't configured at this time unless you +A list of response types this client can return. _It is recommended that this isn't configured at this time unless you know what you're doing_. Valid options are: `code`, `code id_token`, `id_token`, `token id_token`, `token`, `token id_token code`. @@ -471,7 +471,7 @@ required: no {: .label .label-config .label-green } -The algorithm used to sign the userinfo endpoint responses. This can either be `none` or `RS256`. +The algorithm used to sign the userinfo endpoint responses. This can either be `none` or `RS256`. ## Generating a random secret @@ -489,48 +489,52 @@ characters. For Kubernetes, see [this section too](../secrets.md#Kubernetes). ### openid -This is the default scope for openid. This field is forced on every client by the configuration -validation that Authelia does. +This is the default scope for openid. This field is forced on every client by the configuration validation that Authelia +does. -|JWT Field|JWT Type |Authelia Attribute|Description | -|:-------:|:-----------:|:----------------:|:-------------------------------------------:| -|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 | -|at_hash |string |_N/A_ |Access Token Hash | -|aud |array[string]|_N/A_ |Audience | -|exp |number |_N/A_ |Expires | -|auth_time|number |_N/A_ |The time the user authenticated with Authelia| -|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 | +_**Important Note:** The claim `sub` is planned to be changed in the future to a randomly unique value to identify the +individual user. Please use the claim `preferred_username` instead._ + +| JWT Field | JWT Type | Authelia Attribute | Description | +|:------------------:|:-------------:|:------------------:|:---------------------------------------------:| +| 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 | +| at_hash | string | _N/A_ | Access Token Hash | +| aud | array[string] | _N/A_ | Audience | +| exp | number | _N/A_ | Expires | +| auth_time | number | _N/A_ | The time the user authenticated with Authelia | +| 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 This scope includes the groups the authentication backend reports the user is a member of in the token. -|JWT Field|JWT Type |Authelia Attribute|Description | -|:-------:|:-----------:|:----------------:|:--------------------:| -|groups |array[string]|Groups |The users display name| +| JWT Field | JWT Type | Authelia Attribute | Description | +|:---------:|:-------------:|:------------------:|:----------------------:| +| groups | array[string] | Groups | The users display name | ### email This scope includes the email information the authentication backend reports about the user in the token. -|JWT Field |JWT Type |Authelia Attribute|Description | -|:------------:|:-----------:|:----------------:|:-------------------------------------------------------:| -|email |string |email[0] |The first email address in the list of emails | -|email_verified|bool |_N/A_ |If the email is verified, assumed true for the time being| -|alt_emails |array[string]|email[1:] |All email addresses that are not in the email JWT field | +| JWT Field | JWT Type | Authelia Attribute | Description | +|:--------------:|:-------------:|:------------------:|:---------------------------------------------------------:| +| email | string | email[0] | The first email address in the list of emails | +| email_verified | bool | _N/A_ | If the email is verified, assumed true for the time being | +| alt_emails | array[string] | email[1:] | All email addresses that are not in the email JWT field | ### profile This scope includes the profile information the authentication backend reports about the user in the token. -|JWT Field|JWT Type|Authelia Attribute|Description | -|:-------:|:------:|:----------------:|:--------------------:| -|name |string | display_name |The users display name| +| JWT Field | JWT Type | Authelia Attribute | Description | +|:---------:|:--------:|:------------------:|:----------------------:| +| name | string | display_name | The users display name | ## Endpoint Implementations @@ -539,15 +543,15 @@ particularly those that don't use [discovery](https://openid.net/specs/openid-co appended to the end of the primary URL used to access Authelia. For example in the Discovery example provided you access Authelia via https://auth.example.com, the discovery URL is https://auth.example.com/.well-known/openid-configuration. -|Endpoint |Path | -|:-----------:|:------------------------------:| -|Discovery |.well-known/openid-configuration| -|JWKS |api/oidc/jwks | -|Authorization|api/oidc/authorize | -|Token |api/oidc/token | -|Introspection|api/oidc/introspect | -|Revocation |api/oidc/revoke | -|Userinfo |api/oidc/userinfo | +| Endpoint | Path | +|:-------------:|:--------------------------------:| +| Discovery | .well-known/openid-configuration | +| JWKS | api/oidc/jwks | +| Authorization | api/oidc/authorize | +| Token | api/oidc/token | +| Introspection | api/oidc/introspect | +| Revocation | api/oidc/revoke | +| Userinfo | api/oidc/userinfo | [OpenID Connect]: https://openid.net/connect/ [token lifespan]: https://docs.apigee.com/api-platform/antipatterns/oauth-long-expiration diff --git a/internal/handlers/handler_oidc_authorization.go b/internal/handlers/handler_oidc_authorization.go index 83501b1e8..395969e89 100644 --- a/internal/handlers/handler_oidc_authorization.go +++ b/internal/handlers/handler_oidc_authorization.go @@ -14,7 +14,6 @@ import ( "github.com/authelia/authelia/v4/internal/middlewares" "github.com/authelia/authelia/v4/internal/oidc" "github.com/authelia/authelia/v4/internal/session" - "github.com/authelia/authelia/v4/internal/utils" ) func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http.Request) { @@ -93,7 +92,8 @@ func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r * Headers: &jwt.Headers{Extra: map[string]interface{}{ "kid": ctx.Providers.OpenIDConnect.KeyManager.GetActiveKeyID(), }}, - Subject: userSession.Username, + Subject: userSession.Username, + Username: userSession.Username, }, ClientID: clientID, }) @@ -107,40 +107,6 @@ func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r * ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeResponse(rw, ar, response) } -func oidcGrantRequests(ar fosite.AuthorizeRequester, scopes, audiences []string, userSession *session.UserSession) (extraClaims map[string]interface{}) { - extraClaims = map[string]interface{}{} - - for _, scope := range scopes { - ar.GrantScope(scope) - - switch scope { - case "groups": - extraClaims["groups"] = userSession.Groups - case "profile": - extraClaims["name"] = userSession.DisplayName - case "email": - if len(userSession.Emails) != 0 { - extraClaims["email"] = userSession.Emails[0] - if len(userSession.Emails) > 1 { - extraClaims["alt_emails"] = userSession.Emails[1:] - } - // TODO (james-d-elliott): actually verify emails and record that information. - extraClaims["email_verified"] = true - } - } - } - - for _, audience := range audiences { - ar.GrantAudience(audience) - } - - if !utils.IsStringInSlice(ar.GetClient().GetID(), ar.GetGrantedAudience()) { - ar.GrantAudience(ar.GetClient().GetID()) - } - - return extraClaims -} - func oidcAuthorizeHandleAuthorizationOrConsentInsufficient( ctx *middlewares.AutheliaCtx, userSession session.UserSession, client *oidc.InternalClient, isAuthInsufficient bool, rw http.ResponseWriter, r *http.Request, diff --git a/internal/handlers/oidc.go b/internal/handlers/oidc.go index 633e85195..65247fbaf 100644 --- a/internal/handlers/oidc.go +++ b/internal/handlers/oidc.go @@ -1,6 +1,7 @@ package handlers import ( + "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" @@ -30,3 +31,43 @@ func newOpenIDSession(subject string) *oidc.OpenIDSession { Extra: map[string]interface{}{}, } } + +func oidcGrantRequests(ar fosite.AuthorizeRequester, scopes, audiences []string, userSession *session.UserSession) (extraClaims map[string]interface{}) { + extraClaims = map[string]interface{}{ + oidc.ClaimPreferredUsername: userSession.Username, + } + + for _, scope := range scopes { + if ar != nil { + ar.GrantScope(scope) + } + + switch scope { + case oidc.ScopeGroups: + extraClaims[oidc.ClaimGroups] = userSession.Groups + case oidc.ScopeProfile: + extraClaims[oidc.ClaimDisplayName] = userSession.DisplayName + case oidc.ScopeEmail: + if len(userSession.Emails) != 0 { + extraClaims[oidc.ClaimEmail] = userSession.Emails[0] + if len(userSession.Emails) > 1 { + extraClaims[oidc.ClaimAltEmails] = userSession.Emails[1:] + } + // TODO (james-d-elliott): actually verify emails and record that information. + extraClaims[oidc.ClaimEmailVerified] = true + } + } + } + + if ar != nil { + for _, audience := range audiences { + ar.GrantAudience(audience) + } + + if !utils.IsStringInSlice(ar.GetClient().GetID(), ar.GetGrantedAudience()) { + ar.GrantAudience(ar.GetClient().GetID()) + } + } + + return extraClaims +} diff --git a/internal/handlers/oidc_test.go b/internal/handlers/oidc_test.go index e80c84b25..9e96f49d4 100644 --- a/internal/handlers/oidc_test.go +++ b/internal/handlers/oidc_test.go @@ -4,7 +4,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/authelia/authelia/v4/internal/oidc" "github.com/authelia/authelia/v4/internal/session" ) @@ -31,3 +33,107 @@ func TestShouldDetectIfConsentIsMissing(t *testing.T) { requestedAudience = []string{"https://not.authelia.com"} 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) + + assert.Len(t, extraClaims, 2) + + require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) + assert.Equal(t, "john", extraClaims[oidc.ClaimPreferredUsername]) + + 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) + + assert.Len(t, extraClaims, 2) + + require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) + assert.Equal(t, "fred", extraClaims[oidc.ClaimPreferredUsername]) + + 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) + + assert.Len(t, extraClaims, 4) + + require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) + assert.Equal(t, "john", extraClaims[oidc.ClaimPreferredUsername]) + + require.Contains(t, extraClaims, oidc.ClaimEmail) + assert.Equal(t, "j.smith@authelia.com", extraClaims[oidc.ClaimEmail]) + + require.Contains(t, extraClaims, oidc.ClaimAltEmails) + assert.Len(t, extraClaims[oidc.ClaimAltEmails], 1) + assert.Contains(t, extraClaims[oidc.ClaimAltEmails], "admin@authelia.com") + + require.Contains(t, extraClaims, oidc.ClaimEmailVerified) + assert.Equal(t, true, extraClaims[oidc.ClaimEmailVerified]) + + extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeEmail}, []string{}, &oidcUserSessionFred) + + assert.Len(t, extraClaims, 3) + + require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) + assert.Equal(t, "fred", extraClaims[oidc.ClaimPreferredUsername]) + + require.Contains(t, extraClaims, oidc.ClaimEmail) + assert.Equal(t, "f.smith@authelia.com", extraClaims[oidc.ClaimEmail]) + + require.Contains(t, extraClaims, oidc.ClaimEmailVerified) + assert.Equal(t, true, extraClaims[oidc.ClaimEmailVerified]) +} + +func TestShouldGrantAppropriateClaimsForScopeOpenIDAndProfile(t *testing.T) { + extraClaims := oidcGrantRequests(nil, []string{oidc.ScopeOpenID, 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]) + + extraClaims = oidcGrantRequests(nil, []string{oidc.ScopeOpenID, oidc.ScopeProfile}, []string{}, &oidcUserSessionFred) + + assert.Len(t, extraClaims, 2) + + require.Contains(t, extraClaims, oidc.ClaimPreferredUsername) + assert.Equal(t, "fred", extraClaims[oidc.ClaimPreferredUsername]) + + require.Contains(t, extraClaims, oidc.ClaimDisplayName) + assert.Equal(t, extraClaims[oidc.ClaimDisplayName], "Fred Smith") +} + +var ( + oidcUserSessionJohn = session.UserSession{ + Username: "john", + Groups: []string{"admin", "dev"}, + DisplayName: "John Smith", + Emails: []string{"j.smith@authelia.com", "admin@authelia.com"}, + } + + oidcUserSessionFred = session.UserSession{ + Username: "fred", + Groups: []string{"dev"}, + DisplayName: "Fred Smith", + Emails: []string{"f.smith@authelia.com"}, + } +) diff --git a/internal/oidc/const.go b/internal/oidc/const.go index e3ccde2b3..c9c782f6a 100644 --- a/internal/oidc/const.go +++ b/internal/oidc/const.go @@ -8,3 +8,21 @@ var scopeDescriptions = map[string]string{ } var audienceDescriptions = map[string]string{} + +// Scope strings. +const ( + ScopeOpenID = "openid" + ScopeProfile = "profile" + ScopeEmail = "email" + ScopeGroups = "groups" +) + +// Claim strings. +const ( + ClaimGroups = "groups" + ClaimDisplayName = "name" + ClaimPreferredUsername = "preferred_username" + ClaimEmail = "email" + ClaimEmailVerified = "email_verified" + ClaimAltEmails = "alt_emails" +)