From 9903c5f79e9c78e1317623bb3d8918baad2e8ae6 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 22 Jun 2022 08:21:16 -0700 Subject: [PATCH] Handle refresh requests without groups scope Signed-off-by: Margo Crawford --- .../downstreamsession/downstream_session.go | 3 +- internal/oidc/login/post_login_handler.go | 1 - .../provider/dynamic_upstream_idp_provider.go | 2 + internal/oidc/token/token_handler.go | 69 ++-- internal/oidc/token/token_handler_test.go | 356 +++++++++++++----- test/integration/supervisor_login_test.go | 73 +++- test/integration/supervisor_warnings_test.go | 3 +- 7 files changed, 369 insertions(+), 138 deletions(-) diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index d5783e5e..fbb0ca52 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -10,12 +10,11 @@ import ( "net/url" "time" - "k8s.io/utils/strings/slices" - "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/strings/slices" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/constable" diff --git a/internal/oidc/login/post_login_handler.go b/internal/oidc/login/post_login_handler.go index fdc480c0..dafe6e06 100644 --- a/internal/oidc/login/post_login_handler.go +++ b/internal/oidc/login/post_login_handler.go @@ -8,7 +8,6 @@ import ( "net/url" coreosoidc "github.com/coreos/go-oidc/v3/oidc" - "github.com/ory/fosite" "go.pinniped.dev/internal/httputil/httperr" diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 79771943..befcddb8 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -111,6 +111,8 @@ type UpstreamLDAPIdentityProviderI interface { PerformRefresh(ctx context.Context, storedRefreshAttributes StoredRefreshAttributes) (groups []string, err error) } +// StoredRefreshAttributes contains information about the user from the original login request +// and previous refreshes. type StoredRefreshAttributes struct { Username string Subject string diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 15f50ec1..6c3bf575 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -15,6 +15,7 @@ import ( "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/warning" + "k8s.io/utils/strings/slices" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc" @@ -106,19 +107,21 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } + grantedScopes := accessRequest.GetGrantedScopes() + switch customSessionData.ProviderType { case psession.ProviderTypeOIDC: - return upstreamOIDCRefresh(ctx, session, providerCache) + return upstreamOIDCRefresh(ctx, session, providerCache, grantedScopes) case psession.ProviderTypeLDAP: - return upstreamLDAPRefresh(ctx, providerCache, session) + return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes) case psession.ProviderTypeActiveDirectory: - return upstreamLDAPRefresh(ctx, providerCache, session) + return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes) default: return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } } -func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, providerCache oidc.UpstreamIdentityProvidersLister) error { +func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, providerCache oidc.UpstreamIdentityProvidersLister, grantedScopes []string) error { s := session.Custom if s.OIDC == nil { return errorsx.WithStack(errMissingUpstreamSessionInternalError()) @@ -177,30 +180,33 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, 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 errUpstreamRefreshError().WithHintf( - "Upstream refresh error while extracting groups claim.").WithTrace(err). - WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) - } - if refreshedGroups != nil { - oldGroups, err := getDownstreamGroupsFromPinnipedSession(session) + groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) + if groupsScope { + // 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 err + return errUpstreamRefreshError().WithHintf( + "Upstream refresh error while extracting groups claim.").WithTrace(err). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) } - username, err := getDownstreamUsernameFromPinnipedSession(session) - if err != nil { - return err + if refreshedGroups != nil { + oldGroups, err := getDownstreamGroupsFromPinnipedSession(session) + if err != nil { + return err + } + username, err := getDownstreamUsernameFromPinnipedSession(session) + if err != nil { + return err + } + warnIfGroupsChanged(ctx, oldGroups, refreshedGroups, username) + session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = refreshedGroups } - warnIfGroupsChanged(ctx, oldGroups, refreshedGroups, username) - 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 @@ -291,7 +297,7 @@ func findOIDCProviderByNameAndValidateUID( WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } -func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentityProvidersLister, session *psession.PinnipedSession) error { +func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentityProvidersLister, session *psession.PinnipedSession, grantedScopes []string) error { username, err := getDownstreamUsernameFromPinnipedSession(session) if err != nil { return err @@ -339,10 +345,13 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit "Upstream refresh failed.").WithTrace(err). WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) } - // Replace the old value with the new value. - session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = groups + groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) + if groupsScope { + // Replace the old value with the new value. + session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = groups - warnIfGroupsChanged(ctx, oldGroups, groups, username) + warnIfGroupsChanged(ctx, oldGroups, groups, username) + } return nil } @@ -400,7 +409,7 @@ func getDownstreamGroupsFromPinnipedSession(session *psession.PinnipedSession) ( } downstreamGroupsInterface := extra[oidc.DownstreamGroupsClaim] if downstreamGroupsInterface == nil { - return nil, errorsx.WithStack(errMissingUpstreamSessionInternalError()) + return nil, nil } downstreamGroupsInterfaceList, ok := downstreamGroupsInterface.([]interface{}) if !ok { diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index ea0d9290..423a8c55 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -200,7 +200,7 @@ var ( happyAuthRequest = &http.Request{ Form: url.Values{ "response_type": {"code"}, - "scope": {"openid profile email"}, + "scope": {"openid profile email groups"}, "client_id": {goodClient}, "state": {"some-state-value-with-enough-bytes-to-exceed-min-allowed"}, "nonce": {goodNonce}, @@ -268,11 +268,12 @@ func TestTokenEndpointAuthcodeExchange(t *testing.T) { { name: "request is valid and tokens are issued", authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid profile email groups") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, // no refresh token - wantRequestedScopes: []string{"openid", "profile", "email"}, - wantGrantedScopes: []string{"openid"}, + wantRequestedScopes: []string{"openid", "profile", "email", "groups"}, + wantGrantedScopes: []string{"openid", "groups"}, wantGroups: goodGroups, }, }, @@ -299,7 +300,7 @@ func TestTokenEndpointAuthcodeExchange(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, + wantGroups: nil, }, }, }, @@ -316,6 +317,19 @@ func TestTokenEndpointAuthcodeExchange(t *testing.T) { }, }, }, + { + name: "groups scope is requested", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid profile email groups") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, // no refresh token + wantRequestedScopes: []string{"openid", "profile", "email", "groups"}, + wantGrantedScopes: []string{"openid", "groups"}, + wantGroups: goodGroups, + }, + }, + }, // sad path { @@ -566,12 +580,12 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { { name: "authcode exchange succeeds once and then fails when the same authcode is used again", authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access profile email") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access profile email groups") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "profile", "email", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: goodGroups, }, }, @@ -630,14 +644,14 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn 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"}, + wantRequestedScopes: []string{"openid", "pinniped:request-audience", "groups"}, + wantGrantedScopes: []string{"openid", "pinniped:request-audience", "groups"}, wantGroups: goodGroups, } doValidAuthCodeExchange := authcodeExchangeInputs{ modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "openid pinniped:request-audience") + authRequest.Form.Set("scope", "openid pinniped:request-audience groups") }, want: successfulAuthCodeExchange, } @@ -732,13 +746,13 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn name: "access token missing pinniped:request-audience scope", authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "openid") + authRequest.Form.Set("scope", "openid groups") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"}, - wantRequestedScopes: []string{"openid"}, - wantGrantedScopes: []string{"openid"}, + wantRequestedScopes: []string{"openid", "groups"}, + wantGrantedScopes: []string{"openid", "groups"}, wantGroups: goodGroups, }, }, @@ -750,13 +764,13 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn name: "access token missing openid scope", authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "pinniped:request-audience") + authRequest.Form.Set("scope", "pinniped:request-audience groups") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"access_token", "token_type", "expires_in", "scope"}, - wantRequestedScopes: []string{"pinniped:request-audience"}, - wantGrantedScopes: []string{"pinniped:request-audience"}, + wantRequestedScopes: []string{"pinniped:request-audience", "groups"}, + wantGrantedScopes: []string{"pinniped:request-audience", "groups"}, wantGroups: goodGroups, }, }, @@ -765,11 +779,28 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn wantResponseBodyContains: `missing the 'openid' scope`, }, { - name: "token minting failure", + name: "access token missing groups scope", authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(authRequest *http.Request) { authRequest.Form.Set("scope", "openid pinniped:request-audience") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"access_token", "token_type", "expires_in", "scope", "id_token"}, + wantRequestedScopes: []string{"openid", "pinniped:request-audience"}, + wantGrantedScopes: []string{"openid", "pinniped:request-audience"}, + wantGroups: nil, + }, + }, + requestedAudience: "some-workload-cluster", + wantStatus: http.StatusOK, + }, + { + name: "token minting failure", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(authRequest *http.Request) { + authRequest.Form.Set("scope", "openid pinniped:request-audience groups") + }, // Fail to fetch a JWK signing key after the authcode exchange has happened. makeOathHelper: makeOauthHelperWithJWTKeyThatWorksOnlyOnce, want: successfulAuthCodeExchange, @@ -845,7 +876,10 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn require.NoError(t, json.Unmarshal(parsedJWT.UnsafePayloadWithoutVerification(), &tokenClaims)) // Make sure that these are the only fields in the token. - idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "groups", "username"} + idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "username"} + if test.authcodeExchange.want.wantGroups != nil { + idTokenFields = append(idTokenFields, "groups") + } require.ElementsMatch(t, idTokenFields, getMapKeys(tokenClaims)) // Assert that the returned token has expected claims values. @@ -859,7 +893,11 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn require.Equal(t, goodSubject, tokenClaims["sub"]) require.Equal(t, goodIssuer, tokenClaims["iss"]) require.Equal(t, goodUsername, tokenClaims["username"]) - require.Equal(t, toSliceOfInterface(test.authcodeExchange.want.wantGroups), tokenClaims["groups"]) + if test.authcodeExchange.want.wantGroups != nil { + require.Equal(t, toSliceOfInterface(test.authcodeExchange.want.wantGroups), tokenClaims["groups"]) + } else { + require.Nil(t, tokenClaims["groups"]) + } // Also assert that some are the same as the original downstream ID token. requireClaimsAreEqual(t, "iss", claimsOfFirstIDToken, tokenClaims) // issuer @@ -1003,8 +1041,8 @@ func TestRefreshGrant(t *testing.T) { want := tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, - wantRequestedScopes: []string{"openid", "offline_access"}, - wantGrantedScopes: []string{"openid", "offline_access"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantCustomSessionDataStored: wantCustomSessionDataStored, wantGroups: goodGroups, } @@ -1090,7 +1128,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1114,7 +1152,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1142,15 +1180,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCAccessTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCAccessTokenCustomSessionData()), }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: goodGroups, wantUpstreamOIDCValidateTokenCall: &expectedUpstreamValidateTokens{ oidcUpstreamName, @@ -1207,15 +1245,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: goodGroups, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken(), false), @@ -1236,15 +1274,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: []string{"new-group1", "new-group2", "new-group3"}, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), @@ -1265,15 +1303,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: []string{"new-group1", "new-group2", "new-group3"}, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), @@ -1294,15 +1332,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: []string{}, // the user no longer belongs to any groups wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), @@ -1323,15 +1361,15 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: goodGroups, // the same groups as from the initial login wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), @@ -1348,7 +1386,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshGroups: []string{"new-group1", "new-group2", "new-group3"}, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -1358,8 +1396,8 @@ func TestRefreshGrant(t *testing.T) { 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, wantGroups: []string{"new-group1", "new-group2", "new-group3"}, wantUpstreamRefreshCall: happyLDAPUpstreamRefreshCall(), wantCustomSessionDataStored: happyLDAPCustomSessionData, @@ -1375,7 +1413,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshGroups: []string{}, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -1385,9 +1423,120 @@ func TestRefreshGrant(t *testing.T) { want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, + wantGroups: []string{}, + wantUpstreamRefreshCall: happyLDAPUpstreamRefreshCall(), + wantCustomSessionDataStored: happyLDAPCustomSessionData, + }, + }, + }, + { + name: "ldap refresh grant when the upstream refresh when groups scope not requested on original request or refresh", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: ldapUpstreamURL, + PerformRefreshGroups: []string{}, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access"}, wantGrantedScopes: []string{"openid", "offline_access"}, - wantGroups: []string{}, + wantCustomSessionDataStored: happyLDAPCustomSessionData, + wantGroups: nil, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("openid offline_access").ReadCloser() + }, + 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: nil, + wantUpstreamRefreshCall: happyLDAPUpstreamRefreshCall(), + wantCustomSessionDataStored: happyLDAPCustomSessionData, + }, + }, + }, + { + name: "oidc refresh grant when the upstream refresh when groups scope not requested on original request or refresh", + 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{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), + wantGroups: nil, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("openid offline_access").ReadCloser() + }, + 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: nil, + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, + }, + }, + { + // fosite does not look at the scopes provided in refresh requests, although it is a valid parameter. + // even if 'groups' is not sent in the refresh request, we will send groups all the same. + name: "refresh grant when the upstream refresh when groups scope requested on original request but not refresh refresh", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: ldapUpstreamURL, + PerformRefreshGroups: []string{"new-group1", "new-group2", "new-group3"}, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, + customSessionData: happyLDAPCustomSessionData, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, + wantCustomSessionDataStored: happyLDAPCustomSessionData, + wantGroups: goodGroups, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("openid offline_access").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "groups"}, + wantGroups: []string{"new-group1", "new-group2", "new-group3"}, // groups are updated even though the scope was not included wantUpstreamRefreshCall: happyLDAPUpstreamRefreshCall(), wantCustomSessionDataStored: happyLDAPCustomSessionData, }, @@ -1406,7 +1555,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1430,7 +1579,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDTokenWithoutRefreshToken()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1452,7 +1601,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1477,12 +1626,12 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access pinniped:request-audience") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access pinniped:request-audience groups") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience", "groups"}, wantGroups: goodGroups, wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, @@ -1494,8 +1643,8 @@ func TestRefreshGrant(t *testing.T) { want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience", "groups"}, + wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience", "groups"}, wantGroups: goodGroups, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), @@ -1515,7 +1664,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1605,7 +1754,7 @@ func TestRefreshGrant(t *testing.T) { idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: nil, // this should not happen in practice - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(nil), }, refreshRequest: refreshRequestInputs{ @@ -1625,7 +1774,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: "", // this should not happen in practice @@ -1652,7 +1801,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1679,7 +1828,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: "", // this should not happen in practice OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1706,7 +1855,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: "not-an-allowed-provider-type", // this should not happen in practice OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1733,7 +1882,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: nil, // this should not happen in practice }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1763,7 +1912,7 @@ func TestRefreshGrant(t *testing.T) { UpstreamAccessToken: "", }, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1793,7 +1942,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: "this-name-will-not-be-found", // this could happen if the OIDCIdentityProvider was deleted since original login @@ -1825,7 +1974,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamInitialRefreshToken}, }, - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( &psession.CustomSessionData{ // want the initial customSessionData to be unmodified ProviderName: oidcUpstreamName, @@ -1853,7 +2002,7 @@ func TestRefreshGrant(t *testing.T) { WithPerformRefreshError(errors.New("some upstream refresh error")).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1878,7 +2027,7 @@ func TestRefreshGrant(t *testing.T) { Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1910,7 +2059,7 @@ func TestRefreshGrant(t *testing.T) { Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1939,7 +2088,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -1970,7 +2119,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -2001,7 +2150,7 @@ func TestRefreshGrant(t *testing.T) { }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ @@ -2027,7 +2176,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshGroups: goodGroups, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2048,7 +2197,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshGroups: goodGroups, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2068,7 +2217,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: &psession.CustomSessionData{ ProviderUID: ldapUpstreamResourceUID, ProviderName: ldapUpstreamName, @@ -2104,7 +2253,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: &psession.CustomSessionData{ ProviderUID: activeDirectoryUpstreamResourceUID, ProviderName: activeDirectoryUpstreamName, @@ -2140,7 +2289,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: &psession.CustomSessionData{ ProviderUID: ldapUpstreamResourceUID, ProviderName: ldapUpstreamName, @@ -2180,7 +2329,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: &psession.CustomSessionData{ ProviderUID: ldapUpstreamResourceUID, ProviderName: ldapUpstreamName, @@ -2221,7 +2370,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshErr: errors.New("Some error performing upstream refresh"), }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2249,7 +2398,7 @@ func TestRefreshGrant(t *testing.T) { PerformRefreshErr: errors.New("Some error performing upstream refresh"), }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2272,7 +2421,7 @@ func TestRefreshGrant(t *testing.T) { name: "upstream ldap idp not found", idps: oidctestutil.NewUpstreamIDPListerBuilder(), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2294,7 +2443,7 @@ func TestRefreshGrant(t *testing.T) { name: "upstream active directory idp not found", idps: oidctestutil.NewUpstreamIDPListerBuilder(), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2320,7 +2469,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2357,7 +2506,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, //fositeSessionData: &openid.DefaultSession{}, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( @@ -2399,7 +2548,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, //fositeSessionData: &openid.DefaultSession{}, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( @@ -2441,7 +2590,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, //fositeSessionData: &openid.DefaultSession{}, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( @@ -2483,7 +2632,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2509,7 +2658,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2531,7 +2680,7 @@ func TestRefreshGrant(t *testing.T) { name: "upstream ldap idp not found", idps: oidctestutil.NewUpstreamIDPListerBuilder(), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2553,7 +2702,7 @@ func TestRefreshGrant(t *testing.T) { name: "upstream active directory idp not found", idps: oidctestutil.NewUpstreamIDPListerBuilder(), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2579,7 +2728,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2616,7 +2765,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2657,7 +2806,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2696,7 +2845,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2722,7 +2871,7 @@ func TestRefreshGrant(t *testing.T) { URL: ldapUpstreamURL, }), authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access groups") }, customSessionData: happyActiveDirectoryCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyActiveDirectoryCustomSessionData, @@ -2942,7 +3091,7 @@ func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs, idps p requireTokenEndpointBehavior(t, test.want, - goodGroups, // the old groups from the initial login + test.want.wantGroups, // the old groups from the initial login test.customSessionData, // the old custom session data from the initial login wantAtHashClaimInIDToken, wantNonceValueInIDToken, @@ -3174,7 +3323,6 @@ func simulateAuthEndpointHavingAlreadyRun( AuthTime: goodAuthTime, Extra: map[string]interface{}{ oidc.DownstreamUsernameClaim: goodUsername, - oidc.DownstreamGroupsClaim: goodGroups, }, }, Subject: "", // not used, note that callback_handler.go does not set this @@ -3193,6 +3341,10 @@ func simulateAuthEndpointHavingAlreadyRun( if strings.Contains(authRequest.Form.Get("scope"), "pinniped:request-audience") { authRequester.GrantScope("pinniped:request-audience") } + if strings.Contains(authRequest.Form.Get("scope"), "groups") { + authRequester.GrantScope("groups") + session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = goodGroups + } authResponder, err := oauthHelper.NewAuthorizeResponse(ctx, authRequester, session) require.NoError(t, err) return authResponder @@ -3429,10 +3581,13 @@ func requireValidStoredRequest( require.Equal(t, goodSubject, claims.Subject) // Our custom claims from the authorize endpoint should still be set. - require.Equal(t, map[string]interface{}{ + expectedExtra := map[string]interface{}{ "username": goodUsername, - "groups": toSliceOfInterface(wantGroups), - }, claims.Extra) + } + if wantGroups != nil { + expectedExtra["groups"] = toSliceOfInterface(wantGroups) + } + require.Equal(t, expectedExtra, claims.Extra) // We are in charge of setting these fields. For the purpose of testing, we ensure that the // sentinel test value is set correctly. @@ -3551,13 +3706,16 @@ func requireValidIDToken( // Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token // during the initial authcode exchange, but does not prevent `at_hash` from appearing in the refreshed ID token. // We can add a workaround for this later. - idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "groups", "username"} + idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "username"} if wantAtHashClaimInIDToken { idTokenFields = append(idTokenFields, "at_hash") } if wantNonceValueInIDToken { idTokenFields = append(idTokenFields, "nonce") } + if wantGroupsInIDToken != nil { + idTokenFields = append(idTokenFields, "groups") + } // make sure that these are the only fields in the token var m map[string]interface{} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 2fa1679b..b5e22ca1 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -25,6 +25,7 @@ import ( "golang.org/x/oauth2" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/strings/slices" configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" @@ -162,6 +163,7 @@ func TestSupervisorLogin_Browser(t *testing.T) { deleteTestUser func(t *testing.T, username string) requestAuthorization func(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client) createIDP func(t *testing.T) string + downstreamScopes []string wantLocalhostCallbackToNeverHappen bool wantDownstreamIDTokenSubjectToMatch string wantDownstreamIDTokenUsernameToMatch func(username string) string @@ -329,6 +331,55 @@ func TestSupervisorLogin_Browser(t *testing.T) { }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, + { + name: "ldap without requesting groups scope", + maybeSkip: skipLDAPTests, + createIDP: func(t *testing.T) string { + idp, _ := createLDAPIdentityProvider(t, nil) + return idp.Name + }, + downstreamScopes: []string{"openid", "pinniped:request-audience", "offline_access"}, + requestAuthorization: func(t *testing.T, _, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login + env.SupervisorUpstreamLDAP.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute + wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta( + "ldaps://"+env.SupervisorUpstreamLDAP.Host+ + "?base="+url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase)+ + "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), + ) + "$", + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: []string{}, + }, + { + name: "oidc without requesting groups scope", + maybeSkip: skipNever, + createIDP: func(t *testing.T) string { + spec := basicOIDCIdentityProviderSpec() + spec.Claims = idpv1alpha1.OIDCClaims{ + Username: env.SupervisorUpstreamOIDC.UsernameClaim, + Groups: env.SupervisorUpstreamOIDC.GroupsClaim, + } + spec.AuthorizationConfig = idpv1alpha1.OIDCAuthorizationConfig{ + AdditionalScopes: env.SupervisorUpstreamOIDC.AdditionalScopes, + } + return testlib.CreateTestOIDCIdentityProvider(t, spec, idpv1alpha1.PhaseReady).Name + }, + downstreamScopes: []string{"openid", "pinniped:request-audience", "offline_access"}, + requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowOIDC, + wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" }, + wantDownstreamIDTokenGroups: nil, + }, { name: "ldap with browser flow", maybeSkip: skipLDAPTests, @@ -1123,6 +1174,7 @@ func TestSupervisorLogin_Browser(t *testing.T) { tt.breakRefreshSessionData, tt.createTestUser, tt.deleteTestUser, + tt.downstreamScopes, tt.wantLocalhostCallbackToNeverHappen, tt.wantDownstreamIDTokenSubjectToMatch, tt.wantDownstreamIDTokenUsernameToMatch, @@ -1260,6 +1312,7 @@ func testSupervisorLogin( breakRefreshSessionData func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName, username string), createTestUser func(t *testing.T) (string, string), deleteTestUser func(t *testing.T, username string), + downstreamScopes []string, wantLocalhostCallbackToNeverHappen bool, wantDownstreamIDTokenSubjectToMatch string, wantDownstreamIDTokenUsernameToMatch func(username string) string, @@ -1372,6 +1425,10 @@ func testSupervisorLogin( // Start a callback server on localhost. localCallbackServer := startLocalCallbackServer(t) + if downstreamScopes == nil { + downstreamScopes = []string{"openid", "pinniped:request-audience", "offline_access", "groups"} + } + // Form the OAuth2 configuration corresponding to our CLI client. // Note that this is not using response_type=form_post, so the Supervisor will redirect to the callback endpoint // directly, without using the Javascript form_post HTML page to POST back to the callback endpoint. The e2e @@ -1381,7 +1438,7 @@ func testSupervisorLogin( ClientID: "pinniped-cli", Endpoint: discovery.Endpoint(), RedirectURL: localCallbackServer.URL, - Scopes: []string{"openid", "pinniped:request-audience", "offline_access", "groups"}, + Scopes: downstreamScopes, } // Build a valid downstream authorize URL for the supervisor. @@ -1414,9 +1471,9 @@ func testSupervisorLogin( require.NoError(t, err) t.Logf("got callback request: %s", testlib.MaskTokens(callback.URL.String())) - if wantErrorType == "" { + if wantErrorType == "" { // nolint:nestif require.Equal(t, stateParam.String(), callback.URL.Query().Get("state")) - require.ElementsMatch(t, []string{"openid", "pinniped:request-audience", "offline_access", "groups"}, strings.Split(callback.URL.Query().Get("scope"), " ")) + require.ElementsMatch(t, downstreamScopes, strings.Split(callback.URL.Query().Get("scope"), " ")) authcode := callback.URL.Query().Get("code") require.NotEmpty(t, authcode) @@ -1427,7 +1484,10 @@ func testSupervisorLogin( tokenResponse, err := downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier()) require.NoError(t, err) - expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "username", "groups"} + expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "username"} + if slices.Contains(downstreamScopes, "groups") { + expectedIDTokenClaims = append(expectedIDTokenClaims, "groups") + } verifyTokenResponse(t, tokenResponse, discovery, downstreamOAuth2Config, nonceParam, expectedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch(username), wantDownstreamIDTokenGroups) @@ -1464,7 +1524,10 @@ func testSupervisorLogin( require.NoError(t, err) // When refreshing, expect to get an "at_hash" claim, but no "nonce" claim. - expectRefreshedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "rat", "username", "groups", "at_hash"} + expectRefreshedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "rat", "username", "at_hash"} + if slices.Contains(downstreamScopes, "groups") { + expectRefreshedIDTokenClaims = append(expectRefreshedIDTokenClaims, "groups") + } verifyTokenResponse(t, refreshedTokenResponse, discovery, downstreamOAuth2Config, "", expectRefreshedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch(username), refreshedGroups) diff --git a/test/integration/supervisor_warnings_test.go b/test/integration/supervisor_warnings_test.go index 74b5aab0..27c58179 100644 --- a/test/integration/supervisor_warnings_test.go +++ b/test/integration/supervisor_warnings_test.go @@ -119,6 +119,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { "--concierge-authenticator-name", authenticator.Name, "--oidc-session-cache", sessionCachePath, "--credential-cache", credentialCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a cli-based login. @@ -171,7 +172,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { })) // construct the cache key - downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience"} + downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"} sort.Strings(downstreamScopes) sessionCacheKey := oidcclient.SessionCacheKey{ Issuer: downstream.Spec.Issuer,