From 2633d72ce257a4d4189d1350812512e12af14cac Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 13 Jan 2023 14:56:40 -0800 Subject: [PATCH] Change some test variable names related to additional claims Co-authored-by: Ryan Richard Co-authored-by: Joshua Casey --- internal/oidc/auth/auth_handler_test.go | 18 ++++++++-------- .../oidc/callback/callback_handler_test.go | 10 ++++----- .../testutil/oidctestutil/oidctestutil.go | 21 +++++++++---------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index fe36c36f..577bb59d 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package auth @@ -582,7 +582,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantUnnecessaryStoredRecords int wantPasswordGrantCall *expectedPasswordGrant wantDownstreamCustomSessionData *psession.CustomSessionData - wantAdditionalClaims map[string]interface{} + wantDownstreamAdditionalClaims map[string]interface{} } tests := []testCase{ { @@ -721,7 +721,7 @@ func TestAuthorizationEndpoint(t *testing.T) { "downstreamMissingClaim": "upstreamMissingClaim", }). WithIDTokenClaim("upstreamCustomClaim", "i am a claim value"). - WithIDTokenClaim("upstreamOtherClaim", "other claim value"). + WithIDTokenClaim("upstreamOtherClaim", []interface{}{"hello", true}). Build()), method: http.MethodGet, path: happyGetRequestPath, @@ -741,9 +741,9 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, - wantAdditionalClaims: map[string]interface{}{ + wantDownstreamAdditionalClaims: map[string]interface{}{ "downstreamCustomClaim": "i am a claim value", - "downstreamOtherClaim": "other claim value", + "downstreamOtherClaim": []interface{}{"hello", true}, }, }, { @@ -772,7 +772,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, - wantAdditionalClaims: nil, // downstream claims are empty + wantDownstreamAdditionalClaims: nil, // downstream claims are empty }, { name: "LDAP cli upstream happy path using GET", @@ -3189,7 +3189,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test.wantDownstreamClientID, test.wantDownstreamRedirectURI, test.wantDownstreamCustomSessionData, - test.wantAdditionalClaims, + test.wantDownstreamAdditionalClaims, ) default: require.Empty(t, rsp.Header().Values("Location")) @@ -3242,8 +3242,8 @@ func TestAuthorizationEndpoint(t *testing.T) { 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") + if len(test.wantDownstreamAdditionalClaims) > 0 { + require.True(t, len(idps.GetOIDCIdentityProviders()) > 0, "wantDownstreamAdditionalClaims requires at least one OIDC IDP") } subject := NewHandler( diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index a94fed33..a9f185c8 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package callback @@ -189,7 +189,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallenge string wantDownstreamPKCEChallengeMethod string wantDownstreamCustomSessionData *psession.CustomSessionData - wantAdditionalClaims map[string]interface{} + wantDownstreamAdditionalClaims map[string]interface{} wantAuthcodeExchangeCall *expectedAuthcodeExchange }{ @@ -262,7 +262,7 @@ func TestCallbackEndpoint(t *testing.T) { performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, }, - wantAdditionalClaims: map[string]interface{}{ + wantDownstreamAdditionalClaims: map[string]interface{}{ "downstreamCustomClaim": "i am a claim value", "downstreamOtherClaim": "other claim value", }, @@ -1507,7 +1507,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamClientID, downstreamRedirectURI, test.wantDownstreamCustomSessionData, - test.wantAdditionalClaims, + test.wantDownstreamAdditionalClaims, ) // Otherwise, expect an empty response body. @@ -1535,7 +1535,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamClientID, downstreamRedirectURI, test.wantDownstreamCustomSessionData, - test.wantAdditionalClaims, + test.wantDownstreamAdditionalClaims, ) } }) diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 92f470b8..474e5cbc 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidctestutil @@ -28,7 +28,6 @@ import ( kubetesting "k8s.io/client-go/testing" "k8s.io/utils/strings/slices" - oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage/authorizationcode" @@ -947,7 +946,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamClientID string, wantDownstreamRedirectURI string, wantCustomSessionData *psession.CustomSessionData, - wantAdditionalClaims map[string]interface{}, + wantDownstreamAdditionalClaims map[string]interface{}, ) { t.Helper() @@ -986,7 +985,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamClientID, wantDownstreamRedirectURI, wantCustomSessionData, - wantAdditionalClaims, + wantDownstreamAdditionalClaims, ) // One PKCE should have been stored. @@ -1039,7 +1038,7 @@ func validateAuthcodeStorage( wantDownstreamClientID string, wantDownstreamRedirectURI string, wantCustomSessionData *psession.CustomSessionData, - wantAdditionalClaims map[string]interface{}, + wantDownstreamAdditionalClaims map[string]interface{}, ) (*fosite.Request, *psession.PinnipedSession) { t.Helper() @@ -1083,7 +1082,7 @@ func validateAuthcodeStorage( require.Equal(t, wantDownstreamClientID, actualClaims.Extra["azp"]) wantDownstreamIDTokenExtraClaimsCount := 1 // should always have azp claim - if len(wantAdditionalClaims) > 0 { + if len(wantDownstreamAdditionalClaims) > 0 { wantDownstreamIDTokenExtraClaimsCount++ } @@ -1106,12 +1105,12 @@ func validateAuthcodeStorage( actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] require.Nil(t, actualDownstreamIDTokenGroups) } - if len(wantAdditionalClaims) > 0 { - actualAdditionalClaims, ok := actualClaims.Get(oidcapi.IDTokenClaimAdditionalClaims).(map[string]interface{}) - require.True(t, ok, "expected %s to be a map[string]interface{}", oidcapi.IDTokenClaimAdditionalClaims) - require.Equal(t, wantAdditionalClaims, actualAdditionalClaims) + if len(wantDownstreamAdditionalClaims) > 0 { + actualAdditionalClaims, ok := actualClaims.Get("additionalClaims").(map[string]interface{}) + require.True(t, ok, "expected additionalClaims to be a map[string]interface{}") + require.Equal(t, wantDownstreamAdditionalClaims, actualAdditionalClaims) } else { - require.NotContains(t, actualClaims.Extra, oidcapi.IDTokenClaimAdditionalClaims, "%s must not be present when there are no wanted additional claims", oidcapi.IDTokenClaimAdditionalClaims) + require.NotContains(t, actualClaims.Extra, "additionalClaims", "additionalClaims must not be present when there are no wanted additional claims") } // Make sure that we asserted on every extra claim.