From 02ed2d9c95fae00de5790aa103c13a8b37e212cf Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Wed, 4 Jan 2023 11:42:50 -0600 Subject: [PATCH] Backfill tests --- internal/oidc/auth/auth_handler_test.go | 44 ++++++++++++- .../oidc/callback/callback_handler_test.go | 46 +++++++++++++ .../downstream_session_test.go | 66 +++++++++++++++++++ .../oidc/login/post_login_handler_test.go | 2 + .../testutil/oidctestutil/oidctestutil.go | 25 +++++++ internal/upstreamoidc/upstreamoidc_test.go | 10 +++ 6 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 internal/oidc/downstreamsession/downstream_session_test.go diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 0e1af24f..99a00eaa 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -582,6 +582,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantUnnecessaryStoredRecords int wantPasswordGrantCall *expectedPasswordGrant wantDownstreamCustomSessionData *psession.CustomSessionData + wantAdditionalClaims map[string]interface{} } tests := []testCase{ { @@ -711,6 +712,40 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, + { + name: "OIDC upstream password grant happy path using GET with additional claim mappings", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder(). + WithAdditionalClaimMappings(map[string]string{ + "downstreamCustomClaim": "upstreamCustomClaim", + "downstreamOtherClaim": "upstreamOtherClaim", + "downstreamMissingClaim": "upstreamMissingClaim", + }). + WithIDTokenClaim("upstreamCustomClaim", "i am a claim value"). + WithIDTokenClaim("upstreamOtherClaim", "other claim value"). + Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.String(oidcUpstreamUsername), + customPasswordHeader: pointer.String(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, + wantAdditionalClaims: map[string]interface{}{ + "downstreamCustomClaim": "i am a claim value", + "downstreamOtherClaim": "other claim value", + }, + }, { name: "LDAP cli upstream happy path using GET", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), @@ -3126,6 +3161,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test.wantDownstreamClientID, test.wantDownstreamRedirectURI, test.wantDownstreamCustomSessionData, + test.wantAdditionalClaims, ) default: require.Empty(t, rsp.Header().Values("Location")) @@ -3176,9 +3212,15 @@ func TestAuthorizationEndpoint(t *testing.T) { oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace") oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient) oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient) + + idps := test.idps.Build() + if len(test.wantAdditionalClaims) > 0 { + require.True(t, len(idps.GetOIDCIdentityProviders()) > 0, "wantAdditionalClaims requires at least one OIDC IDP") + } + subject := NewHandler( downstreamIssuer, - test.idps.Build(), + idps, oauthHelperWithNullStorage, oauthHelperWithRealStorage, test.generateCSRF, test.generatePKCE, test.generateNonce, test.stateEncoder, test.cookieEncoder, diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 44794ea5..a94fed33 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -189,6 +189,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallenge string wantDownstreamPKCEChallengeMethod string wantDownstreamCustomSessionData *psession.CustomSessionData + wantAdditionalClaims map[string]interface{} wantAuthcodeExchangeCall *expectedAuthcodeExchange }{ @@ -223,6 +224,49 @@ func TestCallbackEndpoint(t *testing.T) { args: happyExchangeAndValidateTokensArgs, }, }, + { + name: "GET with good state and cookie with additional params", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream(). + WithAdditionalClaimMappings(map[string]string{ + "downstreamCustomClaim": "upstreamCustomClaim", + "downstreamOtherClaim": "upstreamOtherClaim", + "downstreamMissingClaim": "upstreamMissingClaim", + }). + WithIDTokenClaim("upstreamCustomClaim", "i am a claim value"). + WithIDTokenClaim("upstreamOtherClaim", "other claim value"). + Build()), + method: http.MethodGet, + path: newRequestPath().WithState( + happyUpstreamStateParam().WithAuthorizeRequestParams( + shallowCopyAndModifyQuery( + happyDownstreamRequestParamsQuery, + map[string]string{"response_mode": "form_post"}, + ).Encode(), + ).Build(t, happyStateCodec), + ).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusOK, + wantContentType: "text/html;charset=UTF-8", + wantBodyFormResponseRegexp: `(.+)`, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamClientID: downstreamPinnipedClientID, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + wantAdditionalClaims: map[string]interface{}{ + "downstreamCustomClaim": "i am a claim value", + "downstreamOtherClaim": "other claim value", + }, + }, { name: "GET with good state and cookie and successful upstream token exchange returns 303 to downstream client callback with its state and code", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()), @@ -1463,6 +1507,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamClientID, downstreamRedirectURI, test.wantDownstreamCustomSessionData, + test.wantAdditionalClaims, ) // Otherwise, expect an empty response body. @@ -1490,6 +1535,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamClientID, downstreamRedirectURI, test.wantDownstreamCustomSessionData, + test.wantAdditionalClaims, ) } }) diff --git a/internal/oidc/downstreamsession/downstream_session_test.go b/internal/oidc/downstreamsession/downstream_session_test.go new file mode 100644 index 00000000..e4e8af3d --- /dev/null +++ b/internal/oidc/downstreamsession/downstream_session_test.go @@ -0,0 +1,66 @@ +package downstreamsession + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/testutil/oidctestutil" +) + +func TestMapAdditionalClaimsFromUpstreamIDToken(t *testing.T) { + tests := []struct { + name string + additionalClaimMappings map[string]string + upstreamClaims map[string]interface{} + wantClaims map[string]interface{} + }{ + { + name: "happy path", + additionalClaimMappings: map[string]string{ + "email": "notification_email", + }, + upstreamClaims: map[string]interface{}{ + "notification_email": "test@example.com", + }, + wantClaims: map[string]interface{}{ + "email": "test@example.com", + }, + }, + { + name: "missing", + additionalClaimMappings: map[string]string{ + "email": "email", + }, + upstreamClaims: map[string]interface{}{}, + wantClaims: map[string]interface{}{}, + }, + { + name: "complex", + additionalClaimMappings: map[string]string{ + "complex": "complex", + }, + upstreamClaims: map[string]interface{}{ + "complex": map[string]string{ + "subClaim": "subValue", + }, + }, + wantClaims: map[string]interface{}{ + "complex": map[string]string{ + "subClaim": "subValue", + }, + }, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + idp := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). + WithAdditionalClaimMappings(test.additionalClaimMappings). + Build() + actual := MapAdditionalClaimsFromUpstreamIDToken(idp, test.upstreamClaims) + + require.Equal(t, test.wantClaims, actual) + }) + } +} diff --git a/internal/oidc/login/post_login_handler_test.go b/internal/oidc/login/post_login_handler_test.go index 72bce69a..cd401dc0 100644 --- a/internal/oidc/login/post_login_handler_test.go +++ b/internal/oidc/login/post_login_handler_test.go @@ -1027,6 +1027,7 @@ func TestPostLoginEndpoint(t *testing.T) { tt.wantDownstreamClient, tt.wantDownstreamRedirectURI, tt.wantDownstreamCustomSessionData, + map[string]interface{}{}, ) case tt.wantRedirectToLoginPageError != "": // Expecting an error redirect to the login UI page. @@ -1062,6 +1063,7 @@ func TestPostLoginEndpoint(t *testing.T) { tt.wantDownstreamClient, tt.wantDownstreamRedirectURI, tt.wantDownstreamCustomSessionData, + map[string]interface{}{}, ) default: require.Failf(t, "test should have expected a redirect or form body", diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index f5de96e7..6e8ff494 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -164,6 +164,7 @@ type TestUpstreamOIDCIdentityProvider struct { GroupsClaim string Scopes []string AdditionalAuthcodeParams map[string]string + AdditionalClaimMappings map[string]string AllowPasswordGrant bool ExchangeAuthcodeAndValidateTokensFunc func( @@ -207,6 +208,10 @@ func (u *TestUpstreamOIDCIdentityProvider) GetAdditionalAuthcodeParams() map[str return u.AdditionalAuthcodeParams } +func (u *TestUpstreamOIDCIdentityProvider) GetAdditionalClaimMappings() map[string]string { + return u.AdditionalClaimMappings +} + func (u *TestUpstreamOIDCIdentityProvider) GetName() string { return u.Name } @@ -630,6 +635,7 @@ type TestUpstreamOIDCIdentityProviderBuilder struct { authorizationURL url.URL hasUserInfoURL bool additionalAuthcodeParams map[string]string + additionalClaimMappings map[string]string allowPasswordGrant bool authcodeExchangeErr error passwordGrantErr error @@ -716,6 +722,11 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAdditionalAuthcodeParams(p return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAdditionalClaimMappings(m map[string]string) *TestUpstreamOIDCIdentityProviderBuilder { + u.additionalClaimMappings = m + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) WithRefreshToken(token string) *TestUpstreamOIDCIdentityProviderBuilder { u.refreshToken = &oidctypes.RefreshToken{Token: token} return u @@ -792,6 +803,7 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent AuthorizationURL: u.authorizationURL, UserInfoURL: u.hasUserInfoURL, AdditionalAuthcodeParams: u.additionalAuthcodeParams, + AdditionalClaimMappings: u.additionalClaimMappings, ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { if u.authcodeExchangeErr != nil { return nil, u.authcodeExchangeErr @@ -934,6 +946,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamClientID string, wantDownstreamRedirectURI string, wantCustomSessionData *psession.CustomSessionData, + wantAdditionalClaims map[string]interface{}, ) { t.Helper() @@ -972,6 +985,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamClientID, wantDownstreamRedirectURI, wantCustomSessionData, + wantAdditionalClaims, ) // One PKCE should have been stored. @@ -1023,6 +1037,7 @@ func validateAuthcodeStorage( wantDownstreamClientID string, wantDownstreamRedirectURI string, wantCustomSessionData *psession.CustomSessionData, + wantAdditionalClaims map[string]interface{}, ) (*fosite.Request, *psession.PinnipedSession) { t.Helper() @@ -1066,6 +1081,10 @@ func validateAuthcodeStorage( require.Equal(t, wantDownstreamClientID, actualClaims.Extra["azp"]) wantDownstreamIDTokenExtraClaimsCount := 1 // should always have azp claim + if len(wantAdditionalClaims) > 0 { + wantDownstreamIDTokenExtraClaimsCount++ + } + // 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) if wantDownstreamIDTokenUsername == "" { @@ -1085,6 +1104,12 @@ func validateAuthcodeStorage( actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] require.Nil(t, actualDownstreamIDTokenGroups) } + if len(wantAdditionalClaims) > 0 { + actualAdditionalClaims, ok := actualClaims.Get("additionalClaims").(map[string]interface{}) + require.True(t, ok, "expected additionalClaims to be a map[string]interface{}") + require.Equal(t, wantAdditionalClaims, actualAdditionalClaims) + } + // Make sure that we asserted on every extra claim. require.Len(t, actualClaims.Extra, wantDownstreamIDTokenExtraClaimsCount) diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go index 79ae3c55..974048c2 100644 --- a/internal/upstreamoidc/upstreamoidc_test.go +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -68,6 +68,16 @@ func TestProviderConfig(t *testing.T) { rawClaims: []byte(`{`), } require.False(t, p.HasUserInfoURL()) + + // AdditionalAuthcodeParams defaults to empty + require.Empty(t, p.AdditionalAuthcodeParams) + p.AdditionalAuthcodeParams = map[string]string{"additional": "authcodeParams"} + require.Equal(t, p.GetAdditionalAuthcodeParams(), map[string]string{"additional": "authcodeParams"}) + + // AdditionalClaimMappings defaults to empty + require.Empty(t, p.AdditionalClaimMappings) + p.AdditionalClaimMappings = map[string]string{"additional": "claimMappings"} + require.Equal(t, p.GetAdditionalClaimMappings(), map[string]string{"additional": "claimMappings"}) }) const (