diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 97731aec..c63a544b 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -254,7 +254,7 @@ func getSubjectAndUsernameFromUpstreamIDToken( func getGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, -) ([]string, error) { +) (interface{}, error) { groupsClaim := upstreamIDPConfig.GetGroupsClaim() if groupsClaim == "" { return nil, nil @@ -271,8 +271,9 @@ func getGroupsFromUpstreamIDToken( return nil, httperr.New(http.StatusUnprocessableEntity, "no groups claim in upstream ID token") } - groups, ok := groupsAsInterface.([]string) - if !ok { + groupsAsArray, okAsArray := groupsAsInterface.([]string) + groupsAsString, okAsString := groupsAsInterface.(string) + if !okAsArray && !okAsString { plog.Warning( "groups claim in upstream ID token has invalid format", "upstreamName", upstreamIDPConfig.GetName(), @@ -282,10 +283,13 @@ func getGroupsFromUpstreamIDToken( return nil, httperr.New(http.StatusUnprocessableEntity, "groups claim in upstream ID token has invalid format") } - return groups, nil + if okAsArray { + return groupsAsArray, nil + } + return groupsAsString, nil } -func makeDownstreamSession(subject string, username string, groups []string) *openid.DefaultSession { +func makeDownstreamSession(subject string, username string, groups interface{}) *openid.DefaultSession { now := time.Now().UTC() openIDSession := &openid.DefaultSession{ Claims: &jwt.IDTokenClaims{ diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index eb7e4eea..2bb1411a 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -134,7 +134,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamGrantedScopes []string wantDownstreamIDTokenSubject string wantDownstreamIDTokenUsername string - wantDownstreamIDTokenGroups []string + wantDownstreamIDTokenGroups interface{} wantDownstreamRequestedScopes []string wantDownstreamNonce string wantDownstreamPKCEChallenge string @@ -199,6 +199,25 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, + { + name: "upstream IDP's configured groups claim in the ID token has a non-array value", + idp: happyUpstream().WithIDTokenClaim(upstreamGroupsClaim, "notAnArrayGroup1 notAnArrayGroup2").Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamUsername, + wantDownstreamIDTokenGroups: "notAnArrayGroup1 notAnArrayGroup2", + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, // Pre-upstream-exchange verification { @@ -682,8 +701,8 @@ func happyUpstream() *upstreamOIDCIdentityProviderBuilder { } } -func (u *upstreamOIDCIdentityProviderBuilder) WithUsernameClaim(claim string) *upstreamOIDCIdentityProviderBuilder { - u.usernameClaim = claim +func (u *upstreamOIDCIdentityProviderBuilder) WithUsernameClaim(value string) *upstreamOIDCIdentityProviderBuilder { + u.usernameClaim = value return u } @@ -750,7 +769,7 @@ func validateAuthcodeStorage( wantDownstreamGrantedScopes []string, wantDownstreamIDTokenSubject string, wantDownstreamIDTokenUsername string, - wantDownstreamIDTokenGroups []string, + wantDownstreamIDTokenGroups interface{}, wantDownstreamRequestedScopes []string, ) (*fosite.Request, *openid.DefaultSession) { t.Helper() @@ -789,9 +808,19 @@ func validateAuthcodeStorage( // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) - if wantDownstreamIDTokenGroups != nil { + if wantDownstreamIDTokenGroups != nil { //nolint:nestif // there are some nested if's here but its probably fine for a test require.Len(t, actualClaims.Extra, 2) - require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualClaims.Extra["groups"]) + wantArray, ok := wantDownstreamIDTokenGroups.([]string) + if ok { + require.ElementsMatch(t, wantArray, actualClaims.Extra["groups"]) + } else { + wantString, ok := wantDownstreamIDTokenGroups.(string) + if ok { + require.Equal(t, wantString, actualClaims.Extra["groups"]) + } else { + require.Fail(t, "wantDownstreamIDTokenGroups should be of type: either []string or string") + } + } } else { require.Len(t, actualClaims.Extra, 1) require.NotContains(t, actualClaims.Extra, "groups")