From b49d37ca5497f29ffdde3141081f91b77e4c69e0 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 19 Nov 2020 15:53:21 -0500 Subject: [PATCH] callback_handler.go: test invalid upstream ID token username/groups Signed-off-by: Ryan Richard --- internal/oidc/callback/callback_handler.go | 36 +++++++++++++++---- .../oidc/callback/callback_handler_test.go | 32 ++++++++++++++++- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index ee9a5621..6aea05c9 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -82,7 +82,11 @@ func NewHandler( return err } - groups := getGroupsFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims) + groups, err := getGroupsFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims) + if err != nil { + return err + } + openIDSession := makeDownstreamSession(downstreamIssuer, downstreamAuthParams.Get("client_id"), username, groups) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { @@ -209,7 +213,13 @@ func getUsernameFromUpstreamIDToken( username, ok := usernameAsInterface.(string) if !ok { - panic("todo bbb") // TODO + plog.Warning( + "username claim in upstream ID token has invalid format", + "upstreamName", upstreamIDPConfig.GetName(), + "configuredUsernameClaim", upstreamIDPConfig.GetUsernameClaim(), + "usernameClaim", usernameClaim, + ) + return "", httperr.New(http.StatusUnprocessableEntity, "username claim in upstream ID token has invalid format") } return username, nil @@ -218,23 +228,35 @@ func getUsernameFromUpstreamIDToken( func getGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, -) []string { +) ([]string, error) { groupsClaim := upstreamIDPConfig.GetGroupsClaim() if groupsClaim == "" { - return nil + return nil, nil } groupsAsInterface, ok := idTokenClaims[groupsClaim] if !ok { - panic("todo ccc") // TODO + plog.Warning( + "no groups claim in upstream ID token", + "upstreamName", upstreamIDPConfig.GetName(), + "configuredGroupsClaim", upstreamIDPConfig.GetGroupsClaim(), + "groupsClaim", groupsClaim, + ) + return nil, httperr.New(http.StatusUnprocessableEntity, "no groups claim in upstream ID token") } groups, ok := groupsAsInterface.([]string) if !ok { - panic("todo ddd") // TODO + plog.Warning( + "groups claim in upstream ID token has invalid format", + "upstreamName", upstreamIDPConfig.GetName(), + "configuredGroupsClaim", upstreamIDPConfig.GetGroupsClaim(), + "groupsClaim", groupsClaim, + ) + return nil, httperr.New(http.StatusUnprocessableEntity, "groups claim in upstream ID token has invalid format") } - return groups + return groups, nil } func makeDownstreamSession(issuer, clientID, username string, groups []string) *openid.DefaultSession { diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 6902773b..f2319893 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -360,6 +360,36 @@ func TestCallbackEndpoint(t *testing.T) { wantBody: "Unprocessable Entity: no username claim in upstream ID token\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, + { + name: "upstream ID token does not contain requested groups claim", + idp: happyUpstream().WithoutIDTokenClaim(upstreamGroupsClaim).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).WithCode(happyUpstreamAuthcode).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantBody: "Unprocessable Entity: no groups claim in upstream ID token\n", + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream ID token contains username claim with weird format", + idp: happyUpstream().WithIDTokenClaim(upstreamUsernameClaim, 42).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).WithCode(happyUpstreamAuthcode).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantBody: "Unprocessable Entity: username claim in upstream ID token has invalid format\n", + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, + { + name: "upstream ID token contains groups claim with weird format", + idp: happyUpstream().WithIDTokenClaim(upstreamGroupsClaim, 42).Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).WithCode(happyUpstreamAuthcode).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantBody: "Unprocessable Entity: groups claim in upstream ID token has invalid format\n", + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, } for _, test := range tests { test := test @@ -531,7 +561,7 @@ func (u *upstreamOIDCIdentityProviderBuilder) WithoutGroupsClaim() *upstreamOIDC return u } -func (u *upstreamOIDCIdentityProviderBuilder) WithIDTokenClaim(name, value string) *upstreamOIDCIdentityProviderBuilder { +func (u *upstreamOIDCIdentityProviderBuilder) WithIDTokenClaim(name string, value interface{}) *upstreamOIDCIdentityProviderBuilder { u.idToken[name] = value return u }