From 4d0c2e16f4b89a65a9ca1f7ee543a56d3a905a95 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 15 Jun 2022 08:00:17 -0700 Subject: [PATCH] require groups scope to get groups back from supervisor Signed-off-by: Margo Crawford --- internal/oidc/auth/auth_handler.go | 6 +- internal/oidc/auth/auth_handler_test.go | 10 +-- internal/oidc/callback/callback_handler.go | 5 +- .../oidc/callback/callback_handler_test.go | 75 +++++++++++++++++-- .../oidc/clientregistry/clientregistry.go | 3 +- .../clientregistry/clientregistry_test.go | 7 +- .../downstreamsession/downstream_session.go | 17 +++-- internal/oidc/login/post_login_handler.go | 8 +- .../oidc/login/post_login_handler_test.go | 33 +++++++- internal/oidc/oidc.go | 8 ++ .../testutil/oidctestutil/oidctestutil.go | 16 +++- test/integration/e2e_test.go | 28 +++++-- test/integration/supervisor_login_test.go | 4 +- 13 files changed, 172 insertions(+), 48 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 698ea7f3..67b1581b 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -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..d5783e5e 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -10,7 +10,8 @@ import ( "net/url" "time" - coreosoidc "github.com/coreos/go-oidc/v3/oidc" + "k8s.io/utils/strings/slices" + "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" @@ -40,7 +41,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 +58,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 +150,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..fdc480c0 100644 --- a/internal/oidc/login/post_login_handler.go +++ b/internal/oidc/login/post_login_handler.go @@ -7,6 +7,8 @@ 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 +46,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) @@ -80,7 +82,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 79380df7..0b8df785 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/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index c408ada9..f5598edc 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -14,6 +14,8 @@ import ( "testing" "time" + "k8s.io/utils/strings/slices" + coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/gorilla/securecookie" "github.com/ory/fosite" @@ -1063,10 +1065,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/test/integration/e2e_test.go b/test/integration/e2e_test.go index 9bafffde..b2305da8 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. @@ -256,6 +257,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. @@ -381,6 +383,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. @@ -514,6 +517,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. @@ -594,6 +598,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 +660,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. @@ -715,6 +721,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. @@ -787,6 +794,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. @@ -847,6 +855,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. @@ -924,6 +933,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. @@ -980,6 +990,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. @@ -1036,6 +1047,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. @@ -1311,7 +1323,7 @@ func requireUserCanUseKubectlWithoutAuthenticatingAgain( require.NoError(t, err) })) - downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience"} + downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience", "groups"} sort.Strings(downstreamScopes) token := cache.GetToken(oidcclient.SessionCacheKey{ Issuer: downstream.Spec.Issuer, @@ -1326,12 +1338,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/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 47587db7..2fa1679b 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -1381,7 +1381,7 @@ func testSupervisorLogin( ClientID: "pinniped-cli", Endpoint: discovery.Endpoint(), RedirectURL: localCallbackServer.URL, - Scopes: []string{"openid", "pinniped:request-audience", "offline_access"}, + Scopes: []string{"openid", "pinniped:request-audience", "offline_access", "groups"}, } // Build a valid downstream authorize URL for the supervisor. @@ -1416,7 +1416,7 @@ func testSupervisorLogin( t.Logf("got callback request: %s", testlib.MaskTokens(callback.URL.String())) if wantErrorType == "" { 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, []string{"openid", "pinniped:request-audience", "offline_access", "groups"}, strings.Split(callback.URL.Query().Get("scope"), " ")) authcode := callback.URL.Query().Get("code") require.NotEmpty(t, authcode)