diff --git a/internal/authenticators/authenticators.go b/internal/authenticators/authenticators.go index e343ecd1..9d675c2a 100644 --- a/internal/authenticators/authenticators.go +++ b/internal/authenticators/authenticators.go @@ -31,7 +31,7 @@ import ( // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator // interface, as well as the Response type. type UserAuthenticator interface { - AuthenticateUser(ctx context.Context, username, password string) (*Response, bool, error) + AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*Response, bool, error) } type Response struct { diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 5fd198ea..105f1ed7 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -338,7 +338,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){ "objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"), }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ pwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute), userAccountControlAttribute: validUserAccountControl, userAccountControlComputedAttribute: validComputedUserAccountControl, @@ -437,7 +437,7 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { return strings.Join(domainComponents[1:], "."), nil } -func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { +func validUserAccountControl(entry *ldap.Entry, _ provider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) if err != nil { return err @@ -450,7 +450,7 @@ func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttribut return nil } -func validComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { +func validComputedUserAccountControl(entry *ldap.Entry, _ provider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute)) if err != nil { return err diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index ce69b4af..dc5fa693 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -222,7 +222,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -564,7 +564,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -633,7 +633,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -705,7 +705,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -784,7 +784,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -847,7 +847,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -997,7 +997,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1146,7 +1146,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1217,7 +1217,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1483,7 +1483,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1542,7 +1542,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1605,7 +1605,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1668,7 +1668,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1879,7 +1879,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1941,7 +1941,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { SkipGroupRefresh: true, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -2083,8 +2083,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { expectedRefreshAttributeChecks := copyOfExpectedValueForResultingCache.RefreshAttributeChecks actualRefreshAttributeChecks := actualConfig.RefreshAttributeChecks - copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{} - actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{} + copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} + actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks)) for k, v := range expectedRefreshAttributeChecks { require.NotNil(t, actualRefreshAttributeChecks[k]) @@ -2333,7 +2333,7 @@ func TestValidUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validUserAccountControl(tt.entry, provider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) @@ -2394,7 +2394,7 @@ func TestValidComputedUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validComputedUserAccountControl(tt.entry, provider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 698ea7f3..adbbec7c 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -131,7 +131,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( return nil } - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password) + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") @@ -146,7 +146,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( username = authenticateResponse.User.GetName() groups := authenticateResponse.User.GetGroups() customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse) - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true) return nil @@ -243,7 +243,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( return nil } - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true) @@ -334,7 +334,7 @@ func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fos // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. - downstreamsession.GrantScopesIfRequested(authorizeRequester) + downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope}) return authorizeRequester, true } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 11431a0b..8847d8c4 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -375,8 +375,8 @@ func TestAuthorizationEndpoint(t *testing.T) { return urlToReturn } - happyDownstreamScopesRequested := []string{"openid", "profile", "email"} - happyDownstreamScopesGranted := []string{"openid"} + happyDownstreamScopesRequested := []string{"openid", "profile", "email", "groups"} + happyDownstreamScopesGranted := []string{"openid", "groups"} happyGetRequestQueryMap := map[string]string{ "response_type": "code", @@ -495,7 +495,7 @@ func TestAuthorizationEndpoint(t *testing.T) { } // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it - happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyState + happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState incomingCookieCSRFValue := "csrf-value-from-cookie" encodedIncomingCookieCSRFValue, err := happyCookieEncoder.Encode("csrf", incomingCookieCSRFValue) @@ -957,7 +957,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, wantStatus: http.StatusFound, wantContentType: htmlContentType, - wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid&state=` + happyState, + wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState, wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, wantDownstreamIDTokenUsername: oidcUpstreamUsername, wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, @@ -980,7 +980,7 @@ func TestAuthorizationEndpoint(t *testing.T) { customPasswordHeader: pointer.StringPtr(happyLDAPPassword), wantStatus: http.StatusFound, wantContentType: htmlContentType, - wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid&state=` + happyState, + wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState, wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index bcb8bf1b..683de017 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" + coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/ory/fosite" "go.pinniped.dev/internal/httputil/httperr" @@ -52,7 +53,7 @@ func NewHandler( } // Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested. - downstreamsession.GrantScopesIfRequested(authorizeRequester) + downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope}) token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens( r.Context(), @@ -76,7 +77,7 @@ func NewHandler( return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index d8f08822..8230e4c8 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -62,8 +62,8 @@ const ( var ( oidcUpstreamGroupMembership = []string{"test-pinniped-group-0", "test-pinniped-group-1"} - happyDownstreamScopesRequested = []string{"openid"} - happyDownstreamScopesGranted = []string{"openid"} + happyDownstreamScopesRequested = []string{"openid", "groups"} + happyDownstreamScopesGranted = []string{"openid", "groups"} happyDownstreamRequestParamsQuery = url.Values{ "response_type": []string{"code"}, @@ -133,7 +133,7 @@ func TestCallbackEndpoint(t *testing.T) { } // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it - happyDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState + happyDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState tests := []struct { name string @@ -236,6 +236,38 @@ func TestCallbackEndpoint(t *testing.T) { args: happyExchangeAndValidateTokensArgs, }, }, + { + name: "form_post happy path with no groups scope requested", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()), + method: http.MethodGet, + path: newRequestPath().WithState( + happyUpstreamStateParam().WithAuthorizeRequestParams( + shallowCopyAndModifyQuery( + happyDownstreamRequestParamsQuery, + map[string]string{ + "response_mode": "form_post", + "scope": "openid", + }, + ).Encode(), + ).Build(t, happyStateCodec), + ).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusOK, + wantContentType: "text/html;charset=UTF-8", + wantBodyFormResponseRegexp: `(.+)`, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamRequestedScopes: []string{"openid"}, + wantDownstreamGrantedScopes: []string{"openid"}, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, { name: "GET with authcode exchange that returns an access token but no refresh token but has a short token lifetime which is stored as a warning in the session", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(1*time.Hour))).WithUserInfoURL().Build()), @@ -683,6 +715,33 @@ func TestCallbackEndpoint(t *testing.T) { name: "state's downstream auth params does not contain openid scope", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()), method: http.MethodGet, + path: newRequestPath(). + WithState( + happyUpstreamStateParam(). + WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "profile email groups"}).Encode()). + Build(t, happyStateCodec), + ).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusSeeOther, + wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=groups&state=` + happyDownstreamState, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamRequestedScopes: []string{"profile", "email", "groups"}, + wantDownstreamGrantedScopes: []string{"groups"}, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, + { + name: "state's downstream auth params does not contain openid or groups scope", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()), + method: http.MethodGet, path: newRequestPath(). WithState( happyUpstreamStateParam(). @@ -695,7 +754,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamIDTokenUsername: oidcUpstreamUsername, wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, wantDownstreamRequestedScopes: []string{"profile", "email"}, - wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamGrantedScopes: []string{}, wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, @@ -712,16 +771,16 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath(). WithState( happyUpstreamStateParam(). - WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access"}).Encode()). + WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access groups"}).Encode()). Build(t, happyStateCodec), ).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusSeeOther, - wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access&state=` + happyDownstreamState, + wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+groups&state=` + happyDownstreamState, wantDownstreamIDTokenUsername: oidcUpstreamUsername, wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, - wantDownstreamRequestedScopes: []string{"openid", "offline_access"}, - wantDownstreamGrantedScopes: []string{"openid", "offline_access"}, + wantDownstreamRequestedScopes: []string{"openid", "offline_access", "groups"}, + wantDownstreamGrantedScopes: []string{"openid", "offline_access", "groups"}, wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, diff --git a/internal/oidc/clientregistry/clientregistry.go b/internal/oidc/clientregistry/clientregistry.go index c01caa7d..123a3d3a 100644 --- a/internal/oidc/clientregistry/clientregistry.go +++ b/internal/oidc/clientregistry/clientregistry.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package clientregistry defines Pinniped's OAuth2/OIDC clients. @@ -85,6 +85,7 @@ func PinnipedCLI() *Client { "profile", "email", "pinniped:request-audience", + "groups", }, Audience: nil, Public: true, diff --git a/internal/oidc/clientregistry/clientregistry_test.go b/internal/oidc/clientregistry/clientregistry_test.go index 5062f629..ac70aa65 100644 --- a/internal/oidc/clientregistry/clientregistry_test.go +++ b/internal/oidc/clientregistry/clientregistry_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package clientregistry @@ -50,7 +50,7 @@ func TestPinnipedCLI(t *testing.T) { require.Equal(t, []string{"http://127.0.0.1/callback"}, c.GetRedirectURIs()) require.Equal(t, fosite.Arguments{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, c.GetGrantTypes()) require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes()) - require.Equal(t, fosite.Arguments{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience"}, c.GetScopes()) + require.Equal(t, fosite.Arguments{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience", "groups"}, c.GetScopes()) require.True(t, c.IsPublic()) require.Nil(t, c.GetAudience()) require.Nil(t, c.GetRequestURIs()) @@ -82,7 +82,8 @@ func TestPinnipedCLI(t *testing.T) { "offline_access", "profile", "email", - "pinniped:request-audience" + "pinniped:request-audience", + "groups" ], "audience": null, "public": true, diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index 2343c833..fbb0ca52 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -10,11 +10,11 @@ import ( "net/url" "time" - coreosoidc "github.com/coreos/go-oidc/v3/oidc" "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" @@ -40,7 +40,7 @@ const ( ) // MakeDownstreamSession creates a downstream OIDC session. -func MakeDownstreamSession(subject string, username string, groups []string, custom *psession.CustomSessionData) *psession.PinnipedSession { +func MakeDownstreamSession(subject string, username string, groups []string, grantedScopes []string, custom *psession.CustomSessionData) *psession.PinnipedSession { now := time.Now().UTC() openIDSession := &psession.PinnipedSession{ Fosite: &openid.DefaultSession{ @@ -57,7 +57,9 @@ func MakeDownstreamSession(subject string, username string, groups []string, cus } openIDSession.IDTokenClaims().Extra = map[string]interface{}{ oidc.DownstreamUsernameClaim: username, - oidc.DownstreamGroupsClaim: groups, + } + if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) { + openIDSession.IDTokenClaims().Extra[oidc.DownstreamGroupsClaim] = groups } return openIDSession } @@ -147,10 +149,10 @@ func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdent } // GrantScopesIfRequested auto-grants the scopes for which we do not require end-user approval, if they were requested. -func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester) { - oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) - oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) - oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience") +func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester, scopes []string) { + for _, scope := range scopes { + oidc.GrantScopeIfRequested(authorizeRequester, scope) + } } // GetDownstreamIdentityFromUpstreamIDToken returns the mapped subject, username, and group names, in that order. diff --git a/internal/oidc/login/post_login_handler.go b/internal/oidc/login/post_login_handler.go index 5eb3a2e0..bc851c54 100644 --- a/internal/oidc/login/post_login_handler.go +++ b/internal/oidc/login/post_login_handler.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" + coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/ory/fosite" "go.pinniped.dev/internal/httputil/httperr" @@ -44,8 +45,8 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider return httperr.New(http.StatusBadRequest, "error using state downstream auth params") } - // Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested. - downstreamsession.GrantScopesIfRequested(authorizeRequester) + // Automatically grant the openid, offline_access, pinniped:request-audience and groups scopes, but only if they were requested. + downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope}) // Get the username and password form params from the POST body. username := r.PostFormValue(usernameParamName) @@ -59,7 +60,7 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider } // Attempt to authenticate the user with the upstream IDP. - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password) + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) // There was some problem during authentication with the upstream, aside from bad username/password. @@ -80,7 +81,7 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider username = authenticateResponse.User.GetName() groups := authenticateResponse.User.GetGroups() customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse) - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false) return nil diff --git a/internal/oidc/login/post_login_handler_test.go b/internal/oidc/login/post_login_handler_test.go index 267c5e08..e6f44cc2 100644 --- a/internal/oidc/login/post_login_handler_test.go +++ b/internal/oidc/login/post_login_handler_test.go @@ -82,8 +82,8 @@ func TestPostLoginEndpoint(t *testing.T) { } ) - happyDownstreamScopesRequested := []string{"openid"} - happyDownstreamScopesGranted := []string{"openid"} + happyDownstreamScopesRequested := []string{"openid", "groups"} + happyDownstreamScopesGranted := []string{"openid", "groups"} happyDownstreamRequestParamsQuery := url.Values{ "response_type": []string{"code"}, @@ -211,7 +211,7 @@ func TestPostLoginEndpoint(t *testing.T) { } // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it - happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState + happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState happyUsernamePasswordFormParams := url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{happyLDAPPassword}} @@ -348,7 +348,7 @@ func TestPostLoginEndpoint(t *testing.T) { wantStatus: http.StatusSeeOther, wantContentType: htmlContentType, wantBodyString: "", - wantRedirectLocationRegexp: "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState, + wantRedirectLocationRegexp: "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState, wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, @@ -410,6 +410,31 @@ func TestPostLoginEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, }, + { + name: "happy LDAP login when groups scope is not requested", + idps: oidctestutil.NewUpstreamIDPListerBuilder(). + WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one + WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider), + decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { + query := copyOfHappyDownstreamRequestParamsQuery() + query["scope"] = []string{"openid"} + data.AuthParams = query.Encode() + }), + formParams: happyUsernamePasswordFormParams, + wantStatus: http.StatusSeeOther, + wantContentType: htmlContentType, + wantBodyString: "", + wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamRequestedScopes: []string{"openid"}, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: []string{"openid"}, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, + }, { name: "bad username LDAP login", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 1c5b7237..f78eaef5 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -76,6 +76,14 @@ const ( // information. DownstreamGroupsClaim = "groups" + // DownstreamGroupsScope is a custom scope that determines whether the + // groups claim will be returned in ID tokens. + DownstreamGroupsScope = "groups" + + // RequestAudienceScope is a custom scope that determines whether a RFC8693 token + // exchange is allowed to request a different audience. + RequestAudienceScope = "pinniped:request-audience" + // CSRFCookieLifespan is the length of time that the CSRF cookie is valid. After this time, the // Supervisor's authorization endpoint should give the browser a new CSRF cookie. We set it to // a week so that it is unlikely to expire during a login. diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 79771943..a5eabea5 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -108,15 +108,18 @@ type UpstreamLDAPIdentityProviderI interface { authenticators.UserAuthenticator // PerformRefresh performs a refresh against the upstream LDAP identity provider - PerformRefresh(ctx context.Context, storedRefreshAttributes StoredRefreshAttributes) (groups []string, err error) + PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes) (groups []string, err error) } -type StoredRefreshAttributes struct { +// RefreshAttributes contains information about the user from the original login request +// and previous refreshes. +type RefreshAttributes struct { Username string Subject string DN string Groups []string AdditionalAttributes map[string]string + GrantedScopes []string } type DynamicUpstreamIDPProvider interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 15f50ec1..c0044fc5 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 { //nolint:nestif + // 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,15 +297,18 @@ 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 } subject := session.Fosite.Claims.Subject - oldGroups, err := getDownstreamGroupsFromPinnipedSession(session) - if err != nil { - return err + var oldGroups []string + if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) { + oldGroups, err = getDownstreamGroupsFromPinnipedSession(session) + if err != nil { + return err + } } s := session.Custom @@ -327,22 +336,26 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } // run PerformRefresh - groups, err := p.PerformRefresh(ctx, provider.StoredRefreshAttributes{ + groups, err := p.PerformRefresh(ctx, provider.RefreshAttributes{ Username: username, Subject: subject, DN: dn, Groups: oldGroups, AdditionalAttributes: additionalAttributes, + GrantedScopes: grantedScopes, }) if err != nil { return errUpstreamRefreshError().WithHint( "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 } diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 1211f7f4..0fc1143d 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, } @@ -753,13 +767,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, }, }, @@ -771,13 +785,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, }, }, @@ -786,11 +800,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, @@ -866,7 +897,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. @@ -880,7 +914,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 @@ -1024,8 +1062,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, } @@ -1111,7 +1149,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{ @@ -1135,7 +1173,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{ @@ -1163,15 +1201,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, @@ -1228,15 +1266,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), @@ -1257,15 +1295,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), @@ -1286,15 +1324,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), @@ -1315,15 +1353,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), @@ -1344,15 +1382,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), @@ -1369,7 +1407,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, @@ -1379,8 +1417,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, @@ -1396,7 +1434,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, @@ -1406,9 +1444,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, }, @@ -1427,7 +1576,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{ @@ -1451,7 +1600,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{ @@ -1473,7 +1622,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{ @@ -1498,12 +1647,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(), }, @@ -1515,8 +1664,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), @@ -1536,7 +1685,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{ @@ -1626,7 +1775,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{ @@ -1646,7 +1795,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 @@ -1673,7 +1822,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, @@ -1700,7 +1849,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, @@ -1727,7 +1876,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, @@ -1754,7 +1903,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, @@ -1784,7 +1933,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, @@ -1814,7 +1963,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 @@ -1846,7 +1995,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, @@ -1874,7 +2023,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{ @@ -1899,7 +2048,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{ @@ -1931,7 +2080,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{ @@ -1960,7 +2109,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{ @@ -1991,7 +2140,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{ @@ -2022,7 +2171,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{ @@ -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: happyLDAPCustomSessionData, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, @@ -2069,7 +2218,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, @@ -2089,7 +2238,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, @@ -2125,7 +2274,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, @@ -2161,7 +2310,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, @@ -2201,7 +2350,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, @@ -2242,7 +2391,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, @@ -2270,7 +2419,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, @@ -2293,7 +2442,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, @@ -2315,7 +2464,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, @@ -2341,7 +2490,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, @@ -2378,7 +2527,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( @@ -2420,7 +2569,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( @@ -2462,7 +2611,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( @@ -2504,7 +2653,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, @@ -2530,7 +2679,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, @@ -2552,7 +2701,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, @@ -2574,7 +2723,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, @@ -2600,7 +2749,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, @@ -2637,7 +2786,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, @@ -2678,7 +2827,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, @@ -2717,7 +2866,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, @@ -2743,7 +2892,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, @@ -2963,7 +3112,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, @@ -3195,7 +3344,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 @@ -3214,6 +3362,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 @@ -3450,10 +3602,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. @@ -3572,13 +3727,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/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index c408ada9..fb1e8a7a 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" v1 "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/utils/strings/slices" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/crud" @@ -91,7 +92,7 @@ type ValidateTokenAndMergeWithUserInfoArgs struct { type ValidateRefreshArgs struct { Ctx context.Context Tok *oauth2.Token - StoredAttributes provider.StoredRefreshAttributes + StoredAttributes provider.RefreshAttributes } type TestUpstreamLDAPIdentityProvider struct { @@ -115,7 +116,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetName() string { return u.Name } -func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { +func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*authenticators.Response, bool, error) { return u.AuthenticateFunc(ctx, username, password) } @@ -123,7 +124,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { return u.URL } -func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) ([]string, error) { +func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { if u.performRefreshArgs == nil { u.performRefreshArgs = make([]*PerformRefreshArgs, 0) } @@ -1063,10 +1064,16 @@ func validateAuthcodeStorage( // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) - require.Len(t, actualClaims.Extra, 2) - actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] - require.NotNil(t, actualDownstreamIDTokenGroups) - require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups) + if slices.Contains(wantDownstreamGrantedScopes, "groups") { + require.Len(t, actualClaims.Extra, 2) + actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] + require.NotNil(t, actualDownstreamIDTokenGroups) + require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups) + } else { + require.Len(t, actualClaims.Extra, 1) + actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] + require.Nil(t, actualDownstreamIDTokenGroups) + } // Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time). testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor) diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index ddee048b..cfbd437f 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -20,11 +20,13 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/utils/strings/slices" "k8s.io/utils/trace" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" + "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/downstreamsession" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" @@ -118,7 +120,7 @@ type ProviderConfig struct { GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error) // RefreshAttributeChecks are extra checks that attributes in a refresh response are as expected. - RefreshAttributeChecks map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error + RefreshAttributeChecks map[string]func(*ldap.Entry, provider.RefreshAttributes) error } // UserSearchConfig contains information about how to search for users in the upstream LDAP IDP. @@ -175,7 +177,7 @@ func (p *Provider) GetConfig() ProviderConfig { return p.c } -func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) ([]string, error) { +func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { t := trace.FromContext(ctx).Nest("slow ldap refresh attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches userDN := storedRefreshAttributes.DN @@ -238,6 +240,10 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p if p.c.GroupSearch.SkipGroupRefresh { return storedRefreshAttributes.Groups, nil } + // if we were not granted the groups scope, we should not search for groups or return any. + if !slices.Contains(storedRefreshAttributes.GrantedScopes, oidc.DownstreamGroupsScope) { + return nil, nil + } mappedGroupNames, err := p.searchGroupsForUserDN(conn, userDN) if err != nil { @@ -398,23 +404,23 @@ func (p *Provider) TestConnection(ctx context.Context) error { // authentication for a given end user's username. It runs the same logic as AuthenticateUser except it does // not bind as that user, so it does not test their password. It returns the same values that a real call to // AuthenticateUser with the correct password would return. -func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) (*authenticators.Response, bool, error) { +func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string, grantedScopes []string) (*authenticators.Response, bool, error) { endUserBindFunc := func(conn Conn, foundUserDN string) error { // Act as if the end user bind always succeeds. return nil } - return p.authenticateUserImpl(ctx, username, endUserBindFunc) + return p.authenticateUserImpl(ctx, username, grantedScopes, endUserBindFunc) } // Authenticate an end user and return their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. -func (p *Provider) AuthenticateUser(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { +func (p *Provider) AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*authenticators.Response, bool, error) { endUserBindFunc := func(conn Conn, foundUserDN string) error { return conn.Bind(foundUserDN, password) } - return p.authenticateUserImpl(ctx, username, endUserBindFunc) + return p.authenticateUserImpl(ctx, username, grantedScopes, endUserBindFunc) } -func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, bool, error) { +func (p *Provider) authenticateUserImpl(ctx context.Context, username string, grantedScopes []string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, bool, error) { t := trace.FromContext(ctx).Nest("slow ldap authenticate user attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches @@ -443,7 +449,7 @@ func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bi return nil, false, fmt.Errorf(`error binding as %q before user search: %w`, p.c.BindUsername, err) } - response, err := p.searchAndBindUser(conn, username, bindFunc) + response, err := p.searchAndBindUser(conn, username, grantedScopes, bindFunc) if err != nil { p.traceAuthFailure(t, err) return nil, false, err @@ -540,7 +546,7 @@ func (p *Provider) SearchForDefaultNamingContext(ctx context.Context) (string, e return searchBase, nil } -func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, error) { +func (p *Provider) searchAndBindUser(conn Conn, username string, grantedScopes []string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, error) { searchResult, err := conn.Search(p.userSearchRequest(username)) if err != nil { plog.All(`error searching for user`, @@ -586,9 +592,12 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c return nil, err } - mappedGroupNames, err := p.searchGroupsForUserDN(conn, userEntry.DN) - if err != nil { - return nil, err + var mappedGroupNames []string + if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) { + mappedGroupNames, err = p.searchGroupsForUserDN(conn, userEntry.DN) + if err != nil { + return nil, err + } } mappedRefreshAttributes := make(map[string]string) @@ -822,8 +831,8 @@ func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) { ) } -func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.StoredRefreshAttributes) error { - return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { +func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error { + return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error { prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] newValues := entry.GetRawAttributeValues(attribute) diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index b4ee6bdf..892c9f25 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -174,6 +174,7 @@ func TestEndUserAuthentication(t *testing.T) { name string username string password string + grantedScopes []string providerConfig *ProviderConfig searchMocks func(conn *mockldapconn.MockConn) bindEndUserMocks func(conn *mockldapconn.MockConn) @@ -286,6 +287,25 @@ func TestEndUserAuthentication(t *testing.T) { info.Groups = []string{} }), }, + { + name: "when groups scope isn't granted, don't do group search", + username: testUpstreamUsername, + password: testUpstreamPassword, + grantedScopes: []string{}, + providerConfig: providerConfig(nil), + searchMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) + }, + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.Groups = nil + }), + }, { name: "when the UsernameAttribute is dn and there is a user search filter provided", username: testUpstreamUsername, @@ -638,8 +658,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { return nil }, } @@ -676,8 +696,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { return nil }, } @@ -1167,7 +1187,11 @@ func TestEndUserAuthentication(t *testing.T) { ldapProvider := New(*tt.providerConfig) - authResponse, authenticated, err := ldapProvider.AuthenticateUser(context.Background(), tt.username, tt.password) + if tt.grantedScopes == nil { + tt.grantedScopes = []string{"groups"} + } + + authResponse, authenticated, err := ldapProvider.AuthenticateUser(context.Background(), tt.username, tt.password, tt.grantedScopes) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1199,7 +1223,7 @@ func TestEndUserAuthentication(t *testing.T) { } // Skip tt.bindEndUserMocks since DryRunAuthenticateUser() never binds as the end user. - authResponse, authenticated, err = ldapProvider.DryRunAuthenticateUser(context.Background(), tt.username) + authResponse, authenticated, err = ldapProvider.DryRunAuthenticateUser(context.Background(), tt.username, tt.grantedScopes) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1318,7 +1342,7 @@ func TestUpstreamRefresh(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupSearchGroupNameAttribute, }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute), }, } @@ -1331,6 +1355,7 @@ func TestUpstreamRefresh(t *testing.T) { tests := []struct { name string providerConfig *ProviderConfig + grantedScopes []string setupMocks func(conn *mockldapconn.MockConn) refreshUserDN string dialError error @@ -1465,6 +1490,17 @@ func TestUpstreamRefresh(t *testing.T) { }, wantGroups: nil, // do not update groups }, + { + name: "happy path where group search is configured but groups scope isn't included", + providerConfig: providerConfig(nil), + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(nil)).Return(happyPathUserSearchResult, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + grantedScopes: []string{}, + wantGroups: nil, + }, { name: "error where dial fails", providerConfig: providerConfig(nil), @@ -1769,14 +1805,18 @@ func TestUpstreamRefresh(t *testing.T) { tt.refreshUserDN = testUserSearchResultDNValue // default for all tests } + if tt.grantedScopes == nil { + tt.grantedScopes = []string{"groups"} + } initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000")) ldapProvider := New(*tt.providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" - groups, err := ldapProvider.PerformRefresh(context.Background(), provider.StoredRefreshAttributes{ + groups, err := ldapProvider.PerformRefresh(context.Background(), provider.RefreshAttributes{ Username: testUserSearchResultUsernameAttributeValue, Subject: subject, DN: tt.refreshUserDN, AdditionalAttributes: map[string]string{pwdLastSetAttribute: initialPwdLastSetEncoded}, + GrantedScopes: tt.grantedScopes, }) if tt.wantErr != "" { require.Error(t, err) @@ -2149,7 +2189,7 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { tt := test t.Run(tt.name, func(t *testing.T) { initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal)) - err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.StoredRefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}}) + err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.RefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}}) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 9bafffde..9bbf1589 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -170,6 +170,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-skip-browser", "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser login via the plugin. @@ -192,14 +193,85 @@ func TestE2EFullIntegration_Browser(t *testing.T) { requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) + }) + + // If scopes aren't specified, we don't request the groups scope, which means we won't get any groups back in our token. + t.Run("with Supervisor OIDC upstream IDP and browser flow, scopes not specified", func(t *testing.T) { + testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + t.Cleanup(cancel) + + tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests + + // Start a fresh browser driver because we don't want to share cookies between the various tests in this file. + page := browsertest.Open(t) + + expectedUsername := env.SupervisorUpstreamOIDC.Username + + // Create a ClusterRoleBinding to give our test user from the upstream read-only access to the cluster. + testlib.CreateTestClusterRoleBinding(t, + rbacv1.Subject{Kind: rbacv1.UserKind, APIGroup: rbacv1.GroupName, Name: expectedUsername}, + rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "view"}, ) + testlib.WaitForUserToHaveAccess(t, expectedUsername, []string{}, &authorizationv1.ResourceAttributes{ + Verb: "get", + Group: "", + Version: "v1", + Resource: "namespaces", + }) + + // Create upstream OIDC provider and wait for it to become ready. + testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: env.SupervisorUpstreamOIDC.Issuer, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), + }, + AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ + AdditionalScopes: env.SupervisorUpstreamOIDC.AdditionalScopes, + }, + Claims: idpv1alpha1.OIDCClaims{ + Username: env.SupervisorUpstreamOIDC.UsernameClaim, + Groups: env.SupervisorUpstreamOIDC.GroupsClaim, + }, + Client: idpv1alpha1.OIDCClient{ + SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, + }, + }, idpv1alpha1.PhaseReady) + + // Use a specific session cache for this test. + sessionCachePath := tempDir + "/test-sessions.yaml" + + kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{ + "get", "kubeconfig", + "--concierge-api-group-suffix", env.APIGroupSuffix, + "--concierge-authenticator-type", "jwt", + "--concierge-authenticator-name", authenticator.Name, + "--oidc-skip-browser", + "--oidc-ca-bundle", testCABundlePath, + "--oidc-session-cache", sessionCachePath, + }) + + // Run "kubectl get namespaces" which should trigger a browser login via the plugin. + kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6") + kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...) + + // Run the kubectl command, wait for the Pinniped CLI to print the authorization URL, and open it in the browser. + kubectlOutputChan := startKubectlAndOpenAuthorizationURLInBrowser(testCtx, t, kubectlCmd, page) + + // Confirm that we got to the upstream IDP's login page, fill out the form, and submit the form. + browsertest.LoginToUpstreamOIDC(t, page, env.SupervisorUpstreamOIDC) + + // Expect to be redirected to the downstream callback which is serving the form_post HTML. + t.Logf("waiting for response page %s", downstream.Spec.Issuer) + browsertest.WaitForURL(t, page, regexp.MustCompile(regexp.QuoteMeta(downstream.Spec.Issuer))) + + // The response page should have done the background fetch() and POST'ed to the CLI's callback. + // It should now be in the "success" state. + formpostExpectSuccessState(t, page) + + requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan)) + + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, []string{}, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience"}) }) t.Run("with Supervisor OIDC upstream IDP and manual authcode copy-paste from browser flow", func(t *testing.T) { @@ -256,6 +328,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-skip-listen", "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser login via the plugin. @@ -309,14 +382,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) t.Run("access token based refresh with Supervisor OIDC upstream IDP and manual authcode copy-paste from browser flow", func(t *testing.T) { @@ -381,6 +447,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-skip-listen", "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser login via the plugin. @@ -451,14 +518,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) t.Run("with Supervisor OIDC upstream IDP and CLI password flow without web browser", func(t *testing.T) { @@ -514,6 +574,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--upstream-identity-provider-flow", "cli_password", // create a kubeconfig configured to use the cli_password flow "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser-less CLI prompt login via the plugin. @@ -540,14 +601,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) t.Run("with Supervisor OIDC upstream IDP and CLI password flow when OIDCIdentityProvider disallows it", func(t *testing.T) { @@ -594,6 +648,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--upstream-identity-provider-flow", "cli_password", "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get --raw /healthz" which should trigger a browser-less CLI prompt login via the plugin. @@ -655,6 +710,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--concierge-authenticator-type", "jwt", "--concierge-authenticator-name", authenticator.Name, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger an LDAP-style login CLI prompt via the plugin. @@ -681,14 +737,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an LDAP upstream IDP and try using it to authenticate during kubectl commands @@ -715,6 +764,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--concierge-authenticator-type", "jwt", "--concierge-authenticator-name", authenticator.Name, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Set up the username and password env vars to avoid the interactive prompts. @@ -753,14 +803,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { require.NoError(t, os.Unsetenv(usernameEnvVar)) require.NoError(t, os.Unsetenv(passwordEnvVar)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an Active Directory upstream IDP and try using it to authenticate during kubectl commands @@ -787,6 +830,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--concierge-authenticator-type", "jwt", "--concierge-authenticator-name", authenticator.Name, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger an LDAP-style login CLI prompt via the plugin. @@ -813,14 +857,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an ActiveDirectory upstream IDP and try using it to authenticate during kubectl commands @@ -847,6 +884,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--concierge-authenticator-type", "jwt", "--concierge-authenticator-name", authenticator.Name, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Set up the username and password env vars to avoid the interactive prompts. @@ -885,14 +923,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { require.NoError(t, os.Unsetenv(usernameEnvVar)) require.NoError(t, os.Unsetenv(passwordEnvVar)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the browser flow. @@ -924,6 +955,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-ca-bundle", testCABundlePath, "--upstream-identity-provider-flow", "browser_authcode", "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser login via the plugin. @@ -941,14 +973,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an Active Directory upstream IDP and try using it to authenticate during kubectl commands, using the browser flow. @@ -980,6 +1005,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-ca-bundle", testCABundlePath, "--upstream-identity-provider-flow", "browser_authcode", "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Run "kubectl get namespaces" which should trigger a browser login via the plugin. @@ -997,14 +1023,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) // Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the env var to choose the browser flow. @@ -1036,6 +1055,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { "--oidc-ca-bundle", testCABundlePath, "--upstream-identity-provider-flow", "cli_password", // put cli_password in the kubeconfig, so we can override it with the env var "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", }) // Override the --upstream-identity-provider-flow flag from the kubeconfig using the env var. @@ -1059,14 +1079,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan)) - requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, - downstream, - kubeconfigPath, - sessionCachePath, - pinnipedExe, - expectedUsername, - expectedGroups, - ) + requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env, downstream, kubeconfigPath, sessionCachePath, pinnipedExe, expectedUsername, expectedGroups, []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"}) }) } @@ -1296,6 +1309,7 @@ func requireUserCanUseKubectlWithoutAuthenticatingAgain( pinnipedExe string, expectedUsername string, expectedGroups []string, + downstreamScopes []string, ) { // Run kubectl, which should work without any prompting for authentication. kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath) @@ -1311,7 +1325,6 @@ func requireUserCanUseKubectlWithoutAuthenticatingAgain( require.NoError(t, err) })) - downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience"} sort.Strings(downstreamScopes) token := cache.GetToken(oidcclient.SessionCacheKey{ Issuer: downstream.Spec.Issuer, @@ -1326,12 +1339,16 @@ func requireUserCanUseKubectlWithoutAuthenticatingAgain( idTokenClaims := token.IDToken.Claims require.Equal(t, expectedUsername, idTokenClaims[oidc.DownstreamUsernameClaim]) - // The groups claim in the file ends up as an []interface{}, so adjust our expectation to match. - expectedGroupsAsEmptyInterfaces := make([]interface{}, 0, len(expectedGroups)) - for _, g := range expectedGroups { - expectedGroupsAsEmptyInterfaces = append(expectedGroupsAsEmptyInterfaces, g) + if expectedGroups == nil { + require.Nil(t, idTokenClaims[oidc.DownstreamGroupsClaim]) + } else { + // The groups claim in the file ends up as an []interface{}, so adjust our expectation to match. + expectedGroupsAsEmptyInterfaces := make([]interface{}, 0, len(expectedGroups)) + for _, g := range expectedGroups { + expectedGroupsAsEmptyInterfaces = append(expectedGroupsAsEmptyInterfaces, g) + } + require.ElementsMatch(t, expectedGroupsAsEmptyInterfaces, idTokenClaims[oidc.DownstreamGroupsClaim]) } - require.ElementsMatch(t, expectedGroupsAsEmptyInterfaces, idTokenClaims[oidc.DownstreamGroupsClaim]) expectedGroupsPlusAuthenticated := append([]string{}, expectedGroups...) expectedGroupsPlusAuthenticated = append(expectedGroupsPlusAuthenticated, "system:authenticated") diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index 584b68f5..3541dfa0 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -73,6 +73,7 @@ func TestLDAPSearch_Parallel(t *testing.T) { name string username string password string + grantedScopes []string provider *upstreamldap.Provider wantError string wantAuthResponse *authenticators.Response @@ -114,6 +115,18 @@ func TestLDAPSearch_Parallel(t *testing.T) { ExtraRefreshAttributes: map[string]string{}, }, }, + { + name: "groups scope not in granted scopes", + username: "pinny", + password: pinnyPassword, + grantedScopes: []string{}, + provider: upstreamldap.New(*providerConfig(nil)), + wantAuthResponse: &authenticators.Response{ + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: nil}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, + }, + }, { name: "when the user search filter is already wrapped by parenthesis", username: "pinny", @@ -636,7 +649,10 @@ func TestLDAPSearch_Parallel(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - authResponse, authenticated, err := tt.provider.AuthenticateUser(ctx, tt.username, tt.password) + if tt.grantedScopes == nil { + tt.grantedScopes = []string{"groups"} + } + authResponse, authenticated, err := tt.provider.AuthenticateUser(ctx, tt.username, tt.password, tt.grantedScopes) switch { case tt.wantError != "": @@ -694,9 +710,7 @@ func TestSimultaneousLDAPRequestsOnSingleProvider(t *testing.T) { authUserCtx, authUserCtxCancelFunc := context.WithTimeout(context.Background(), 2*time.Minute) defer authUserCtxCancelFunc() - authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, - env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword, - ) + authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword, []string{"groups"}) resultCh <- authUserResult{ response: authResponse, authenticated: authenticated, diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index b376e9e4..1d43cdd0 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" @@ -163,6 +164,7 @@ func TestSupervisorLogin_Browser(t *testing.T) { requestAuthorization func(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client) createIDP func(t *testing.T) string requestTokenExchangeAud string + downstreamScopes []string wantLocalhostCallbackToNeverHappen bool wantDownstreamIDTokenSubjectToMatch string wantDownstreamIDTokenUsernameToMatch func(username string) string @@ -331,6 +333,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, @@ -1188,6 +1239,7 @@ func TestSupervisorLogin_Browser(t *testing.T) { tt.breakRefreshSessionData, tt.createTestUser, tt.deleteTestUser, + tt.downstreamScopes, tt.requestTokenExchangeAud, tt.wantLocalhostCallbackToNeverHappen, tt.wantDownstreamIDTokenSubjectToMatch, @@ -1327,6 +1379,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, requestTokenExchangeAud string, wantLocalhostCallbackToNeverHappen bool, wantDownstreamIDTokenSubjectToMatch string, @@ -1441,6 +1494,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 @@ -1450,7 +1507,7 @@ func testSupervisorLogin( ClientID: "pinniped-cli", Endpoint: discovery.Endpoint(), RedirectURL: localCallbackServer.URL, - Scopes: []string{"openid", "pinniped:request-audience", "offline_access"}, + Scopes: downstreamScopes, } // Build a valid downstream authorize URL for the supervisor. @@ -1483,9 +1540,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"}, 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) @@ -1496,7 +1553,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) @@ -1536,7 +1596,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..04f77b25 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, @@ -262,6 +263,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. @@ -405,6 +407,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { "--oidc-skip-listen", "--oidc-ca-bundle", testCABundlePath, "--oidc-session-cache", sessionCachePath, + "--oidc-scopes", "offline_access,openid,pinniped:request-audience,groups", "--credential-cache", credentialCachePath, })