From b77297c68d5bbd5d6dffd52c890c296da58c0767 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 25 Jan 2021 09:53:52 -0800 Subject: [PATCH] Validate the upstream `email_verified` claim when it makes sense --- internal/oidc/callback/callback_handler.go | 56 +++++++++--- .../oidc/callback/callback_handler_test.go | 87 +++++++++++++++++++ 2 files changed, 129 insertions(+), 14 deletions(-) diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 6a6e6431..c95d40b3 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -24,6 +24,14 @@ import ( "go.pinniped.dev/internal/plog" ) +const ( + // The name of the email claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + emailClaimName = "email" + + // The name of the email_verified claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + emailVerifiedClaimName = "email_verified" +) + func NewHandler( idpListGetter oidc.IDPListGetter, oauthHelper fosite.OAuth2Provider, @@ -222,18 +230,41 @@ func getSubjectAndUsernameFromUpstreamIDToken( subject := fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, upstreamSubject) - usernameClaim := upstreamIDPConfig.GetUsernameClaim() - if usernameClaim == "" { + usernameClaimName := upstreamIDPConfig.GetUsernameClaim() + if usernameClaimName == "" { return subject, subject, nil } - usernameAsInterface, ok := idTokenClaims[usernameClaim] + // If the upstream username claim is configured to be the special "email" claim and the upstream "email_verified" + // claim is present, then validate that the "email_verified" claim is true. + emailVerifiedAsInterface, ok := idTokenClaims[emailVerifiedClaimName] + if usernameClaimName == emailClaimName && ok { + emailVerified, ok := emailVerifiedAsInterface.(bool) + if !ok { + plog.Warning( + "username claim configured as \"email\" and upstream email_verified claim is not a boolean", + "upstreamName", upstreamIDPConfig.GetName(), + "configuredUsernameClaim", usernameClaimName, + "emailVerifiedClaim", emailVerifiedAsInterface, + ) + return "", "", httperr.New(http.StatusUnprocessableEntity, "email_verified claim in upstream ID token has invalid format") + } + if !emailVerified { + plog.Warning( + "username claim configured as \"email\" and upstream email_verified claim has false value", + "upstreamName", upstreamIDPConfig.GetName(), + "configuredUsernameClaim", usernameClaimName, + ) + return "", "", httperr.New(http.StatusUnprocessableEntity, "email_verified claim in upstream ID token has false value") + } + } + + usernameAsInterface, ok := idTokenClaims[usernameClaimName] if !ok { plog.Warning( "no username claim in upstream ID token", "upstreamName", upstreamIDPConfig.GetName(), - "configuredUsernameClaim", upstreamIDPConfig.GetUsernameClaim(), - "usernameClaim", usernameClaim, + "configuredUsernameClaim", usernameClaimName, ) return "", "", httperr.New(http.StatusUnprocessableEntity, "no username claim in upstream ID token") } @@ -243,8 +274,7 @@ func getSubjectAndUsernameFromUpstreamIDToken( plog.Warning( "username claim in upstream ID token has invalid format", "upstreamName", upstreamIDPConfig.GetName(), - "configuredUsernameClaim", upstreamIDPConfig.GetUsernameClaim(), - "usernameClaim", usernameClaim, + "configuredUsernameClaim", usernameClaimName, ) return "", "", httperr.New(http.StatusUnprocessableEntity, "username claim in upstream ID token has invalid format") } @@ -256,18 +286,17 @@ func getGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) ([]string, error) { - groupsClaim := upstreamIDPConfig.GetGroupsClaim() - if groupsClaim == "" { + groupsClaimName := upstreamIDPConfig.GetGroupsClaim() + if groupsClaimName == "" { return nil, nil } - groupsAsInterface, ok := idTokenClaims[groupsClaim] + groupsAsInterface, ok := idTokenClaims[groupsClaimName] if !ok { plog.Warning( "no groups claim in upstream ID token", "upstreamName", upstreamIDPConfig.GetName(), - "configuredGroupsClaim", upstreamIDPConfig.GetGroupsClaim(), - "groupsClaim", groupsClaim, + "configuredGroupsClaim", groupsClaimName, ) return nil, nil // the upstream IDP may have omitted the claim if the user has no groups } @@ -277,8 +306,7 @@ func getGroupsFromUpstreamIDToken( plog.Warning( "groups claim in upstream ID token has invalid format", "upstreamName", upstreamIDPConfig.GetName(), - "configuredGroupsClaim", upstreamIDPConfig.GetGroupsClaim(), - "groupsClaim", groupsClaim, + "configuredGroupsClaim", groupsClaimName, ) return nil, httperr.New(http.StatusUnprocessableEntity, "groups claim in upstream ID token has invalid format") } diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 3e7db1af..218b6753 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -180,6 +180,93 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, + { + name: "upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is missing", + idp: happyUpstream().WithUsernameClaim("email"). + WithIDTokenClaim("email", "joe@whitehouse.gov").Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: "joe@whitehouse.gov", + wantDownstreamIDTokenGroups: upstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with true value", + idp: happyUpstream().WithUsernameClaim("email"). + WithIDTokenClaim("email", "joe@whitehouse.gov"). + WithIDTokenClaim("email_verified", true).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: "joe@whitehouse.gov", + wantDownstreamIDTokenGroups: upstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream IDP configures username claim as anything other than special claim `email` and `email_verified` upstream claim is present with false value", + idp: happyUpstream().WithUsernameClaim("some-claim"). + WithIDTokenClaim("some-claim", "joe"). + WithIDTokenClaim("email", "joe@whitehouse.gov"). + WithIDTokenClaim("email_verified", false).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, // succeed despite `email_verified=false` because we're not using the email claim for anything + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: "joe", + wantDownstreamIDTokenGroups: upstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with illegal value", + idp: happyUpstream().WithUsernameClaim("email"). + WithIDTokenClaim("email", "joe@whitehouse.gov"). + WithIDTokenClaim("email_verified", "supposed to be boolean").Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantBody: "Unprocessable Entity: email_verified claim in upstream ID token has invalid format\n", + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with false value", + idp: happyUpstream().WithUsernameClaim("email"). + WithIDTokenClaim("email", "joe@whitehouse.gov"). + WithIDTokenClaim("email_verified", false).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantBody: "Unprocessable Entity: email_verified claim in upstream ID token has false value\n", + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, { name: "upstream IDP provides username claim configuration as `sub`, so the downstream token subject should be exactly what they asked for", idp: happyUpstream().WithUsernameClaim("sub").Build(),