diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index 64ac1418..ec85f07e 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -126,7 +126,7 @@ func GetDownstreamIdentityFromUpstreamIDToken( return "", "", nil, err } - groups, err := getGroupsFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims) + groups, err := GetGroupsFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims) if err != nil { return "", "", nil, err } @@ -231,7 +231,10 @@ func downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString string, upstreamSu return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, url.QueryEscape(upstreamSubject)) } -func getGroupsFromUpstreamIDToken( +// GetGroupsFromUpstreamIDToken returns mapped group names coerced into a slice of strings. +// It returns nil when there is no configured groups claim name, or then when the configured claim name is not found +// in the provided map of claims. It returns an error when the claim exists but its value cannot be parsed. +func GetGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) ([]string, error) { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 56200aad..4820a6d6 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -15,10 +15,10 @@ import ( "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc" + "go.pinniped.dev/internal/oidc/downstreamsession" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" - "go.pinniped.dev/pkg/oidcclient/oidctypes" ) var ( @@ -140,19 +140,43 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, // https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse _, hasIDTok := tokens.Extra("id_token").(string) + // We may or may not have an ID token, and we may or may not have a userinfo endpoint to call for more claims. + // Use what we can (one, both, or neither) and return the union of their claims. If we stored an access token, + // then require that the userinfo endpoint exists and returns a successful response, or else we would have no + // way to check that the user's session was not revoked on the server. // The spec is not 100% clear about whether an ID token from the refresh flow should include a nonce, and at // least some providers do not include one, so we skip the nonce validation here (but not other validations). validatedTokens, err := p.ValidateTokenAndMergeWithUserInfo(ctx, tokens, "", hasIDTok, accessTokenStored) if err != nil { return errorsx.WithStack(errUpstreamRefreshError.WithHintf( - "Upstream refresh returned an invalid ID token or UserInfo response.").WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + "Upstream refresh returned an invalid ID token or UserInfo response.").WithWrap(err). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } + mergedClaims := validatedTokens.IDToken.Claims - err = validateIdentityUnchangedSinceInitialLogin(validatedTokens, session, p.GetUsernameClaim()) + // To the extent possible, check that the user's basic identity hasn't changed. + err = validateIdentityUnchangedSinceInitialLogin(mergedClaims, session, p.GetUsernameClaim()) if err != nil { return err } + // If possible, update the user's group memberships. The configured groups claim name (if there is one) may or + // may not be included in the newly fetched and merged claims. It could be missing due to a misconfiguration of the + // claim name. It could also be missing because the claim was originally found in the ID token during login, but + // now we might not have a refreshed ID token. + // If the claim is found, then use it to update the user's group membership in the session. + // If the claim is not found, then we have no new information about groups, so skip updating the group membership + // and let any old groups memberships in the session remain. + refreshedGroups, err := downstreamsession.GetGroupsFromUpstreamIDToken(p, mergedClaims) + if err != nil { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh error while extracting groups claim.").WithWrap(err). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + } + if refreshedGroups != nil { + session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = refreshedGroups + } + // Upstream refresh may or may not return a new refresh token. If we got a new refresh token, then update it in // the user's session. If we did not get a new refresh token, then keep the old one in the session by avoiding // overwriting the old one. @@ -165,9 +189,8 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, return nil } -func validateIdentityUnchangedSinceInitialLogin(validatedTokens *oidctypes.Token, session *psession.PinnipedSession, usernameClaimName string) error { +func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interface{}, session *psession.PinnipedSession, usernameClaimName string) error { s := session.Custom - mergedClaims := validatedTokens.IDToken.Claims // If we have any claims at all, we better have a subject, and it better match the previous value. // but it's possible that we don't because both returning a new id token on refresh and having a userinfo @@ -229,7 +252,8 @@ func findOIDCProviderByNameAndValidateUID( } } return nil, errorsx.WithStack(errUpstreamRefreshError. - WithHint("Provider from upstream session data was not found.").WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + WithHint("Provider from upstream session data was not found."). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentityProvidersLister, session *psession.PinnipedSession) error { @@ -272,7 +296,8 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit }) if err != nil { return errorsx.WithStack(errUpstreamRefreshError.WithHint( - "Upstream refresh failed.").WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + "Upstream refresh failed.").WithWrap(err). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } return nil @@ -296,14 +321,16 @@ func findLDAPProviderByNameAndValidateUID( if p.GetName() == s.ProviderName { if p.GetResourceUID() != s.ProviderUID { return nil, "", errorsx.WithStack(errUpstreamRefreshError.WithHint( - "Provider from upstream session data has changed its resource UID since authentication.").WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + "Provider from upstream session data has changed its resource UID since authentication."). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } return p, dn, nil } } return nil, "", errorsx.WithStack(errUpstreamRefreshError. - WithHint("Provider from upstream session data was not found.").WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + WithHint("Provider from upstream session data was not found."). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) (string, error) { diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 11405ed4..8ef3c742 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -77,8 +77,7 @@ const ( var ( goodAuthTime = time.Date(1, 2, 3, 4, 5, 6, 7, time.UTC) goodRequestedAtTime = time.Date(7, 6, 5, 4, 3, 2, 1, time.UTC) - goodGroups = []string{"group1", "groups2"} - expectedGoodGroups = []interface{}{"group1", "groups2"} + goodGroups = []string{"group1", "groups2"} // the default groups set by the authorize endpoint for these tests hmacSecretFunc = func() []byte { return []byte(hmacSecret) @@ -191,6 +190,13 @@ var ( } `) + fositeUpstreamGroupClaimErrorBody = here.Doc(` + { + "error": "error", + "error_description": "Error during upstream refresh. Upstream refresh error while extracting groups claim." + } + `) + happyAuthRequest = &http.Request{ Form: url.Values{ "response_type": {"code"}, @@ -234,6 +240,7 @@ type tokenEndpointResponseExpectedValues struct { wantErrorResponseBody string wantRequestedScopes []string wantGrantedScopes []string + wantGroups []string wantUpstreamRefreshCall *expectedUpstreamRefresh wantUpstreamOIDCValidateTokenCall *expectedUpstreamValidateTokens wantCustomSessionDataStored *psession.CustomSessionData @@ -252,7 +259,7 @@ type authcodeExchangeInputs struct { want tokenEndpointResponseExpectedValues } -func TestTokenEndpoint(t *testing.T) { +func TestTokenEndpointAuthcodeExchange(t *testing.T) { tests := []struct { name string authcodeExchange authcodeExchangeInputs @@ -266,6 +273,7 @@ func TestTokenEndpoint(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, // no refresh token wantRequestedScopes: []string{"openid", "profile", "email"}, wantGrantedScopes: []string{"openid"}, + wantGroups: goodGroups, }, }, }, @@ -278,6 +286,7 @@ func TestTokenEndpoint(t *testing.T) { wantSuccessBodyFields: []string{"access_token", "token_type", "scope", "expires_in"}, // no id or refresh tokens wantRequestedScopes: []string{"profile", "email"}, wantGrantedScopes: []string{}, + wantGroups: goodGroups, }, }, }, @@ -290,6 +299,7 @@ func TestTokenEndpoint(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in", "refresh_token"}, // all possible tokens wantRequestedScopes: []string{"openid", "offline_access"}, wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: goodGroups, }, }, }, @@ -302,6 +312,7 @@ func TestTokenEndpoint(t *testing.T) { wantSuccessBodyFields: []string{"access_token", "token_type", "scope", "expires_in", "refresh_token"}, // no id token wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, + wantGroups: goodGroups, }, }, }, @@ -561,6 +572,7 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access", "profile", "email"}, wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: goodGroups, }, }, }, @@ -601,7 +613,7 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { // Note that customSessionData is only relevant to refresh grant, so we leave it as nil for this // authcode exchange test, even though in practice it would actually be in the session. requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, - test.authcodeExchange.want.wantRequestedScopes, test.authcodeExchange.want.wantGrantedScopes, nil) + test.authcodeExchange.want.wantRequestedScopes, test.authcodeExchange.want.wantGrantedScopes, test.authcodeExchange.want.wantGroups, nil) // Check that the access token and refresh token storage were both deleted, and the number of other storage objects did not change. testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) @@ -614,12 +626,13 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { } } -func TestTokenExchange(t *testing.T) { +func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn:ietf:params:oauth:grant-type:token-exchange" successfulAuthCodeExchange := tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "pinniped:request-audience"}, wantGrantedScopes: []string{"openid", "pinniped:request-audience"}, + wantGroups: goodGroups, } doValidAuthCodeExchange := authcodeExchangeInputs{ @@ -726,6 +739,7 @@ func TestTokenExchange(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid"}, wantGrantedScopes: []string{"openid"}, + wantGroups: goodGroups, }, }, requestedAudience: "some-workload-cluster", @@ -743,6 +757,7 @@ func TestTokenExchange(t *testing.T) { wantSuccessBodyFields: []string{"access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"pinniped:request-audience"}, wantGrantedScopes: []string{"pinniped:request-audience"}, + wantGroups: goodGroups, }, }, requestedAudience: "some-workload-cluster", @@ -805,7 +820,7 @@ func TestTokenExchange(t *testing.T) { require.Contains(t, rsp.Body.String(), test.wantResponseBodyContains) } - // The remaining assertions apply only the the happy path. + // The remaining assertions apply only to the happy path. if rsp.Code != http.StatusOK { return } @@ -844,7 +859,7 @@ func TestTokenExchange(t *testing.T) { require.Equal(t, goodSubject, tokenClaims["sub"]) require.Equal(t, goodIssuer, tokenClaims["iss"]) require.Equal(t, goodUsername, tokenClaims["username"]) - require.Equal(t, expectedGoodGroups, tokenClaims["groups"]) + require.Equal(t, toSliceOfInterface(test.authcodeExchange.want.wantGroups), tokenClaims["groups"]) // Also assert that some are the same as the original downstream ID token. requireClaimsAreEqual(t, "iss", claimsOfFirstIDToken, tokenClaims) // issuer @@ -991,6 +1006,7 @@ func TestRefreshGrant(t *testing.T) { wantRequestedScopes: []string{"openid", "offline_access"}, wantGrantedScopes: []string{"openid", "offline_access"}, wantCustomSessionDataStored: wantCustomSessionDataStored, + wantGroups: goodGroups, } return want } @@ -1128,13 +1144,14 @@ func TestRefreshGrant(t *testing.T) { customSessionData: initialUpstreamOIDCAccessTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCAccessTokenCustomSessionData()), - }, // do not want upstreamRefreshRequest??? + }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "id_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access"}, wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: goodGroups, wantUpstreamOIDCValidateTokenCall: &expectedUpstreamValidateTokens{ oidcUpstreamName, &oidctestutil.ValidateTokenAndMergeWithUserInfoArgs{ @@ -1199,12 +1216,154 @@ func TestRefreshGrant(t *testing.T) { wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access"}, wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: goodGroups, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken(), false), wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), }, }, }, + { + name: "happy path refresh grant when the upstream refresh returns new group memberships (as strings) from the merged ID token and userinfo results, it updates groups", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "sub": goodUpstreamSubject, + "my-groups-claim": []string{"new-group1", "new-group2", "new-group3"}, // refreshed claims includes updated groups + }, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: []string{"new-group1", "new-group2", "new-group3"}, + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, + }, + }, + { + name: "happy path refresh grant when the upstream refresh returns new group memberships (as interface{} types) from the merged ID token and userinfo results, it updates groups", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "sub": goodUpstreamSubject, + "my-groups-claim": []interface{}{"new-group1", "new-group2", "new-group3"}, // refreshed claims includes updated groups + }, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: []string{"new-group1", "new-group2", "new-group3"}, + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, + }, + }, + { + name: "happy path refresh grant when the upstream refresh returns new group memberships as an empty list from the merged ID token and userinfo results, it updates groups to be empty", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "sub": goodUpstreamSubject, + "my-groups-claim": []string{}, // refreshed groups claims is updated to be an empty list + }, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: []string{}, // the user no longer belongs to any groups + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, + }, + }, + { + name: "happy path refresh grant when the upstream refresh does not return new group memberships from the merged ID token and userinfo results by omitting claim, it keeps groups from initial login", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "sub": goodUpstreamSubject, + // "my-groups-claim" is omitted from the refreshed claims + }, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantGroups: goodGroups, // the same groups as from the initial login + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, + }, + }, + { + name: "error from refresh grant when the upstream refresh does not return new group memberships from the merged ID token and userinfo results by returning group claim with illegal nil value", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "sub": goodUpstreamSubject, + "my-groups-claim": nil, + }, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: fositeUpstreamGroupClaimErrorBody, + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + }, + }, + }, { name: "happy path refresh grant when the upstream refresh does not return a new refresh token", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( @@ -1270,6 +1429,7 @@ func TestRefreshGrant(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, + wantGroups: goodGroups, wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, @@ -1282,6 +1442,7 @@ func TestRefreshGrant(t *testing.T) { wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, + wantGroups: goodGroups, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), @@ -2597,7 +2758,8 @@ func TestRefreshGrant(t *testing.T) { requireTokenEndpointBehavior(t, test.refreshRequest.want, - test.authcodeExchange.customSessionData, + test.authcodeExchange.want.wantGroups, // the old groups from the initial login + test.authcodeExchange.customSessionData, // the old custom session data from the initial login wantAtHashClaimInIDToken, wantNonceValueInIDToken, refreshResponse, @@ -2724,7 +2886,8 @@ func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs, idps p requireTokenEndpointBehavior(t, test.want, - test.customSessionData, + goodGroups, // the old groups from the initial login + test.customSessionData, // the old custom session data from the initial login wantAtHashClaimInIDToken, wantNonceValueInIDToken, rsp, @@ -2740,6 +2903,7 @@ func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs, idps p func requireTokenEndpointBehavior( t *testing.T, test tokenEndpointResponseExpectedValues, + oldGroups []string, oldCustomSessionData *psession.CustomSessionData, wantAtHashClaimInIDToken bool, wantNonceValueInIDToken bool, @@ -2763,10 +2927,10 @@ func requireTokenEndpointBehavior( wantRefreshToken := contains(test.wantSuccessBodyFields, "refresh_token") requireInvalidAuthCodeStorage(t, authCode, oauthStore, secrets) - requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, test.wantCustomSessionDataStored, secrets) + requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, test.wantGroups, test.wantCustomSessionDataStored, secrets) requireInvalidPKCEStorage(t, authCode, oauthStore) - // Performing a refresh does not update the OIDC storage, so after a refresh it should still have the old custom session data from the initial login. - requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, oldCustomSessionData) + // Performing a refresh does not update the OIDC storage, so after a refresh it should still have the old custom session data and old groups from the initial login. + requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, oldGroups, oldCustomSessionData) expectedNumberOfRefreshTokenSessionsStored := 0 if wantRefreshToken { @@ -2775,10 +2939,10 @@ func requireTokenEndpointBehavior( expectedNumberOfIDSessionsStored := 0 if wantIDToken { expectedNumberOfIDSessionsStored = 1 - requireValidIDToken(t, parsedResponseBody, jwtSigningKey, wantAtHashClaimInIDToken, wantNonceValueInIDToken, parsedResponseBody["access_token"].(string)) + requireValidIDToken(t, parsedResponseBody, jwtSigningKey, wantAtHashClaimInIDToken, wantNonceValueInIDToken, test.wantGroups, parsedResponseBody["access_token"].(string)) } if wantRefreshToken { - requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, test.wantCustomSessionDataStored, secrets) + requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, test.wantGroups, test.wantCustomSessionDataStored, secrets) } testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) @@ -3016,6 +3180,7 @@ func requireValidRefreshTokenStorage( storage fositeoauth2.CoreStorage, wantRequestedScopes []string, wantGrantedScopes []string, + wantGroups []string, wantCustomSessionData *psession.CustomSessionData, secrets v1.SecretInterface, ) { @@ -3038,6 +3203,7 @@ func requireValidRefreshTokenStorage( wantRequestedScopes, wantGrantedScopes, true, + wantGroups, wantCustomSessionData, ) @@ -3050,6 +3216,7 @@ func requireValidAccessTokenStorage( storage fositeoauth2.CoreStorage, wantRequestedScopes []string, wantGrantedScopes []string, + wantGroups []string, wantCustomSessionData *psession.CustomSessionData, secrets v1.SecretInterface, ) { @@ -3091,6 +3258,7 @@ func requireValidAccessTokenStorage( wantRequestedScopes, wantGrantedScopes, true, + wantGroups, wantCustomSessionData, ) @@ -3133,6 +3301,7 @@ func requireValidOIDCStorage( storage openid.OpenIDConnectRequestStorage, wantRequestedScopes []string, wantGrantedScopes []string, + wantGroups []string, wantCustomSessionData *psession.CustomSessionData, ) { t.Helper() @@ -3156,6 +3325,7 @@ func requireValidOIDCStorage( wantRequestedScopes, wantGrantedScopes, false, + wantGroups, wantCustomSessionData, ) } else { @@ -3171,6 +3341,7 @@ func requireValidStoredRequest( wantRequestedScopes []string, wantGrantedScopes []string, wantAccessTokenExpiresAt bool, + wantGroups []string, wantCustomSessionData *psession.CustomSessionData, ) { t.Helper() @@ -3198,7 +3369,7 @@ func requireValidStoredRequest( // Our custom claims from the authorize endpoint should still be set. require.Equal(t, map[string]interface{}{ "username": goodUsername, - "groups": expectedGoodGroups, + "groups": toSliceOfInterface(wantGroups), }, claims.Extra) // We are in charge of setting these fields. For the purpose of testing, we ensure that the @@ -3287,6 +3458,7 @@ func requireValidIDToken( jwtSigningKey *ecdsa.PrivateKey, wantAtHashClaimInIDToken bool, wantNonceValueInIDToken bool, + wantGroupsInIDToken []string, actualAccessToken string, ) { t.Helper() @@ -3335,7 +3507,7 @@ func requireValidIDToken( require.NoError(t, err) require.Equal(t, goodSubject, claims.Subject) require.Equal(t, goodUsername, claims.Username) - require.Equal(t, goodGroups, claims.Groups) + require.Equal(t, wantGroupsInIDToken, claims.Groups) require.Len(t, claims.Audience, 1) require.Equal(t, goodClient, claims.Audience[0]) require.Equal(t, goodIssuer, claims.Issuer) @@ -3388,3 +3560,11 @@ func contains(haystack []string, needle string) bool { } return false } + +func toSliceOfInterface(s []string) []interface{} { + r := make([]interface{}, len(s)) + for i := range s { + r[i] = s[i] + } + return r +}