From 96098841dd3314644252aabd10ac45a66f8d97a3 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 13 Jun 2023 12:26:59 -0700 Subject: [PATCH] Get tests to compile again and fix lint errors --- cmd/pinniped/cmd/login_oidc.go | 2 +- internal/celtransformer/celformer.go | 2 +- .../impersonator_config_test.go | 2 +- .../controller/kubecertagent/kubecertagent.go | 2 +- .../federation_domain_watcher.go | 4 +- .../oidc_upstream_watcher_test.go | 2 +- .../garbage_collector_test.go | 24 +- .../controllermanager/prepare_controllers.go | 12 +- internal/oidc/auth/auth_handler_test.go | 156 +++++----- .../oidc/callback/callback_handler_test.go | 14 +- .../idp_discovery_handler_test.go | 38 ++- .../oidc/login/post_login_handler_test.go | 124 ++++---- ...ration_domain_identity_providers_lister.go | 5 + .../provider/federation_domain_issuer_test.go | 10 +- .../oidc/provider/manager/manager_test.go | 46 ++- internal/oidc/token/token_handler.go | 3 +- internal/oidc/token/token_handler_test.go | 263 ++++++++-------- internal/supervisor/server/server.go | 2 +- .../testutil/oidctestutil/oidctestutil.go | 288 ++++++++++++++++-- pkg/oidcclient/login_test.go | 2 +- 20 files changed, 622 insertions(+), 379 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 69b8951c..ac76cb4f 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -164,7 +164,7 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin // Initialize the login handler. opts := []oidcclient.Option{ oidcclient.WithContext(cmd.Context()), - oidcclient.WithLogger(plog.Logr()), //nolint:staticcheck // old code with lots of log statements + oidcclient.WithLogger(plog.Logr()), //nolint:staticcheck // old code with lots of log statements oidcclient.WithScopes(flags.scopes), oidcclient.WithSessionCache(sessionCache), } diff --git a/internal/celtransformer/celformer.go b/internal/celtransformer/celformer.go index 133555a1..2c1bf409 100644 --- a/internal/celtransformer/celformer.go +++ b/internal/celtransformer/celformer.go @@ -59,7 +59,7 @@ type TransformationConstants struct { StringListConstants map[string][]string } -// Valid identifiers in CEL expressions are defined by the CEL language spec as: [_a-zA-Z][_a-zA-Z0-9]* +// Valid identifiers in CEL expressions are defined as [_a-zA-Z][_a-zA-Z0-9]* by the CEL language spec. var validIdentifiersRegexp = regexp.MustCompile(`^[_a-zA-Z][_a-zA-Z0-9]*$`) func (t *TransformationConstants) validateVariableNames() error { diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 25252afc..8ec62f59 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -92,7 +92,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { nil, caSignerName, nil, - plog.Logr(), //nolint:staticcheck // old test with no log assertions + plog.Logr(), //nolint:staticcheck // old test with no log assertions ) credIssuerInformerFilter = observableWithInformerOption.GetFilterForInformer(credIssuerInformer) servicesInformerFilter = observableWithInformerOption.GetFilterForInformer(servicesInformer) diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index 30faa3b5..d3e41494 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -179,7 +179,7 @@ func NewAgentController( dynamicCertProvider, &clock.RealClock{}, cache.NewExpiring(), - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ) } diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 9df42508..81366746 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -103,7 +103,7 @@ func NewFederationDomainWatcherController( } // Sync implements controllerlib.Syncer. -func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error { +func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error { //nolint:funlen,gocyclo federationDomains, err := c.federationDomainInformer.Lister().List(labels.Everything()) if err != nil { return err @@ -325,7 +325,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro result, _ := pipeline.Evaluate(context.TODO(), e.Username, e.Groups) // TODO: handle err resultWasAuthRejected := !result.AuthenticationAllowed - if e.Expects.Rejected && !resultWasAuthRejected { + if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif // TODO: handle this failed example plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not", "federationDomain", federationDomain.Name, diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 814b425a..c8e392ae 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -93,7 +93,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { nil, pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), secretInformer, - plog.Logr(), //nolint:staticcheck // old test with no log assertions + plog.Logr(), //nolint:staticcheck // old test with no log assertions withInformer.WithInformer, ) diff --git a/internal/controller/supervisorstorage/garbage_collector_test.go b/internal/controller/supervisorstorage/garbage_collector_test.go index 4eabbad5..1e23b143 100644 --- a/internal/controller/supervisorstorage/garbage_collector_test.go +++ b/internal/controller/supervisorstorage/garbage_collector_test.go @@ -361,7 +361,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is only revoked for the active authcode session. @@ -485,7 +485,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is only revoked for the active authcode session. @@ -562,7 +562,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Nothing to revoke since we couldn't read the invalid secret. @@ -633,7 +633,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Nothing to revoke since we couldn't find the upstream in the cache. @@ -704,7 +704,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Nothing to revoke since we couldn't find the upstream in the cache. @@ -777,7 +777,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(provider.NewRetryableRevocationError(errors.New("some retryable upstream revocation error"))) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Tried to revoke it, although this revocation will fail. @@ -802,7 +802,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(errors.New("some upstream revocation error not worth retrying")) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Tried to revoke it, although this revocation will fail. @@ -881,7 +881,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(errors.New("some upstream revocation error")) // the upstream revocation will fail idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // Tried to revoke it, although this revocation will fail. @@ -1004,7 +1004,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is only revoked for the downstream session which had offline_access granted. @@ -1128,7 +1128,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is only revoked for the downstream session which had offline_access granted. @@ -1206,7 +1206,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is revoked. @@ -1283,7 +1283,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { WithRevokeTokenError(nil) idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build()) - startInformersAndController(idpListerBuilder.Build()) + startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider()) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) // The upstream refresh token is revoked. diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 041ebe32..92dc1e1e 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.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 controllermanager provides an entrypoint into running all of the controllers that run as @@ -222,7 +222,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol agentConfig, client, informers.installationNamespaceK8s.Core().V1().Pods(), - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ), singletonWorker, ). @@ -232,7 +232,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol webhookcachefiller.New( c.AuthenticatorCache, informers.pinniped.Authentication().V1alpha1().WebhookAuthenticators(), - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ), singletonWorker, ). @@ -240,7 +240,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol jwtcachefiller.New( c.AuthenticatorCache, informers.pinniped.Authentication().V1alpha1().JWTAuthenticators(), - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ), singletonWorker, ). @@ -249,7 +249,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol c.AuthenticatorCache, informers.pinniped.Authentication().V1alpha1().WebhookAuthenticators(), informers.pinniped.Authentication().V1alpha1().JWTAuthenticators(), - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ), singletonWorker, ). @@ -275,7 +275,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol impersonator.New, c.NamesConfig.ImpersonationSignerSecret, c.ImpersonationSigningCertProvider, - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements ), singletonWorker, ). diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 308adcc4..f7ff3a88 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -35,7 +35,6 @@ import ( "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/oidcclientvalidator" - "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" @@ -323,27 +322,26 @@ func TestAuthorizationEndpoint(t *testing.T) { return nil, false, nil } - upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: parsedUpstreamLDAPURL, - AuthenticateFunc: ldapAuthenticateFunc, - } + upstreamLDAPIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(parsedUpstreamLDAPURL). + WithAuthenticateFunc(ldapAuthenticateFunc). + Build() - upstreamActiveDirectoryIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: parsedUpstreamLDAPURL, - AuthenticateFunc: ldapAuthenticateFunc, - } + upstreamActiveDirectoryIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(parsedUpstreamLDAPURL). + WithAuthenticateFunc(ldapAuthenticateFunc). + Build() - erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { + erroringUpstreamLDAPIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithAuthenticateFunc(func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { return nil, false, fmt.Errorf("some ldap upstream auth error") - }, - } + }).Build() happyCSRF := "test-csrf" happyPKCE := "test-pkce" @@ -622,7 +620,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream browser flow happy path using GET without a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -639,7 +637,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream browser flow happy path using GET without a CSRF cookie using a dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -657,7 +655,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory upstream browser flow happy path using GET without a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -674,7 +672,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory upstream browser flow happy path using GET without a CSRF cookie using a dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -776,7 +774,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP cli upstream happy path using GET", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -797,7 +795,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "ActiveDirectory cli upstream happy path using GET", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -835,7 +833,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream browser flow happy path using GET with a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -852,7 +850,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory upstream browser flow happy path using GET with a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -908,7 +906,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream browser flow happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -927,7 +925,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream browser flow happy path using POST with a dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -947,7 +945,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory upstream browser flow happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -966,7 +964,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory upstream browser flow happy path using POST with a dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -1010,7 +1008,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP cli upstream happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodPost, path: "/some/path", contentType: formContentType, @@ -1033,7 +1031,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "Active Directory cli upstream happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodPost, path: "/some/path", contentType: formContentType, @@ -1213,7 +1211,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "LDAP upstream happy path when downstream redirect uri matches what is configured for client except for the port number", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ "redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client @@ -1332,7 +1330,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error during upstream LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(erroringUpstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1343,7 +1341,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error during upstream Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(erroringUpstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1377,7 +1375,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream password for LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1389,7 +1387,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream password for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1401,7 +1399,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream username for LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To("wrong-username"), @@ -1413,7 +1411,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream username for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To("wrong-username"), @@ -1437,7 +1435,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream username but has password on request for LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: nil, // do not send header @@ -1449,7 +1447,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream username on request for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: nil, // do not send header @@ -1461,7 +1459,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream password on request for LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1473,7 +1471,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream password on request for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1600,7 +1598,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "dynamic clients are not allowed to use LDAP CLI-flow authentication because we don't want them to handle user credentials", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}), @@ -1613,7 +1611,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "dynamic clients are not allowed to use Active Directory CLI-flow authentication because we don't want them to handle user credentials", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}), @@ -1674,7 +1672,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client when using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ "redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-pinniped-cli-client", @@ -1687,7 +1685,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client when using active directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ "redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-pinniped-cli-client", @@ -1725,7 +1723,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream client does not exist when using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}), wantStatus: http.StatusUnauthorized, @@ -1734,7 +1732,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream client does not exist when using active directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}), wantStatus: http.StatusUnauthorized, @@ -1790,7 +1788,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using LDAP cli upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1802,7 +1800,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using LDAP browser upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), wantStatus: http.StatusSeeOther, @@ -1812,7 +1810,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using LDAP browser upstream with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ @@ -1827,7 +1825,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using active directory cli upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), customUsernameHeader: ptr.To(oidcUpstreamUsername), @@ -1839,7 +1837,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using active directory browser upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), wantStatus: http.StatusSeeOther, @@ -1849,7 +1847,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using active directory browser upstream with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ @@ -1936,7 +1934,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid tuna"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -1948,7 +1946,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client using Active Directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid tuna"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2003,7 +2001,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using LDAP cli upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), customUsernameHeader: ptr.To(oidcUpstreamUsername), @@ -2015,7 +2013,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using LDAP browser upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), wantStatus: http.StatusSeeOther, @@ -2025,7 +2023,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using LDAP browser upstream with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "response_type": ""}), @@ -2036,7 +2034,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using Active Directory cli upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), customUsernameHeader: ptr.To(oidcUpstreamUsername), @@ -2048,7 +2046,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using Active Directory browser upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), wantStatus: http.StatusSeeOther, @@ -2058,7 +2056,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using Active Directory browser upstream with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "response_type": ""}), @@ -2094,7 +2092,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing client id in request using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": ""}), wantStatus: http.StatusUnauthorized, @@ -2148,7 +2146,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing PKCE code_challenge in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"code_challenge": ""}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2206,7 +2204,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "invalid value for PKCE code_challenge_method in request using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2264,7 +2262,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "when PKCE code_challenge_method in request is `plain` using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "plain"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2322,7 +2320,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing PKCE code_challenge_method in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": ""}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2388,7 +2386,7 @@ func TestAuthorizationEndpoint(t *testing.T) { // This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running // through that part of the fosite library when using an LDAP upstream. name: "prompt param is not allowed to have none and another legal value at the same time using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -2465,7 +2463,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "happy path: downstream OIDC validations are skipped when the openid scope was not requested using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, // The following prompt value is illegal when openid is requested, but note that openid is not requested. path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login", "scope": "email"}), @@ -2949,7 +2947,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream state does not have enough entropy using LDAP upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"state": "short"}), customUsernameHeader: ptr.To(happyLDAPUsername), @@ -3049,7 +3047,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: multiple LDAP", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider, &upstreamLDAPIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider, upstreamLDAPIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -3058,7 +3056,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: multiple Active Directory", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider, &upstreamLDAPIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(upstreamLDAPIdentityProvider, upstreamLDAPIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -3067,7 +3065,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: both OIDC and LDAP", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(&upstreamLDAPIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(upstreamLDAPIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -3076,7 +3074,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: OIDC, LDAP and AD", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(&upstreamLDAPIdentityProvider).WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(upstreamLDAPIdentityProvider).WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -3241,7 +3239,7 @@ func TestAuthorizationEndpoint(t *testing.T) { oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient) oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient) - idps := test.idps.Build() + idps := test.idps.BuildFederationDomainIdentityProvidersListerFinder() if len(test.wantDownstreamAdditionalClaims) > 0 { require.True(t, len(idps.GetOIDCIdentityProviders()) > 0, "wantDownstreamAdditionalClaims requires at least one OIDC IDP") } @@ -3268,7 +3266,7 @@ func TestAuthorizationEndpoint(t *testing.T) { oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace") oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient) oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient) - idpLister := test.idps.Build() + idpLister := test.idps.BuildFederationDomainIdentityProvidersListerFinder() subject := NewHandler( downstreamIssuer, idpLister, @@ -3287,7 +3285,7 @@ func TestAuthorizationEndpoint(t *testing.T) { WithScopes([]string{"some-other-new-scope1", "some-other-new-scope2"}). WithAdditionalAuthcodeParams(map[string]string{"prompt": "consent", "abc": "123"}). Build() - idpLister.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{upstreamprovider.UpstreamOIDCIdentityProviderI(newProviderSettings)}) + idpLister.SetOIDCIdentityProviders([]*oidctestutil.TestUpstreamOIDCIdentityProvider{newProviderSettings}) // Update the expectations of the test case to match the new upstream IDP settings. test.wantLocationHeader = urlWithQuery(upstreamAuthURL.String(), diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index a9f185c8..5670ef3f 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -122,11 +122,11 @@ var ( func TestCallbackEndpoint(t *testing.T) { require.Len(t, happyDownstreamState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case") - otherUpstreamOIDCIdentityProvider := oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: "other-upstream-idp-name", - ClientID: "other-some-client-id", - Scopes: []string{"other-scope1", "other-scope2"}, - } + otherUpstreamOIDCIdentityProvider := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). + WithName("other-upstream-idp-name"). + WithClientID("other-some-client-id"). + WithScopes([]string{"other-scope1", "other-scope2"}). + Build() var stateEncoderHashKey = []byte("fake-hash-secret") var stateEncoderBlockKey = []byte("0123456789ABCDEF") // block encryption requires 16/24/32 bytes for AES @@ -1160,7 +1160,7 @@ func TestCallbackEndpoint(t *testing.T) { }, { name: "the OIDCIdentityProvider CRD has been deleted", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&otherUpstreamOIDCIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(otherUpstreamOIDCIdentityProvider), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, @@ -1457,7 +1457,7 @@ func TestCallbackEndpoint(t *testing.T) { jwksProviderIsUnused := jwks.NewDynamicJWKSProvider() oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration) - subject := NewHandler(test.idps.Build(), oauthHelper, happyStateCodec, happyCookieCodec, happyUpstreamRedirectURI) + subject := NewHandler(test.idps.BuildFederationDomainIdentityProvidersListerFinder(), oauthHelper, happyStateCodec, happyCookieCodec, happyUpstreamRedirectURI) reqContext := context.WithValue(context.Background(), struct{ name string }{name: "test"}, "request-context") req := httptest.NewRequest(test.method, test.path, nil).WithContext(reqContext) if test.csrfCookie != "" { diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go index 7ab52d07..9a7fedf2 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go @@ -12,7 +12,6 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/oidc" - "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil/oidctestutil" ) @@ -71,15 +70,15 @@ func TestIDPDiscovery(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { idpLister := oidctestutil.NewUpstreamIDPListerBuilder(). - WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "z-some-oidc-idp", AllowPasswordGrant: true}). - WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "x-some-idp"}). - WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "a-some-ldap-idp"}). - WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "a-some-oidc-idp"}). - WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "z-some-ldap-idp"}). - WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "x-some-idp"}). - WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "z-some-ad-idp"}). - WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "y-some-ad-idp"}). - Build() + WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("z-some-oidc-idp").WithAllowPasswordGrant(true).Build()). + WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("x-some-idp").Build()). + WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("a-some-ldap-idp").Build()). + WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("a-some-oidc-idp").Build()). + WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("z-some-ldap-idp").Build()). + WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("x-some-idp").Build()). + WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("z-some-ad-idp").Build()). + WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("y-some-ad-idp").Build()). + BuildFederationDomainIdentityProvidersListerFinder() handler := NewHandler(idpLister) req := httptest.NewRequest(test.method, test.path, nil) @@ -99,18 +98,17 @@ func TestIDPDiscovery(t *testing.T) { } // Change the list of IDPs in the cache. - idpLister.SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ - &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-1"}, - &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-2"}, + idpLister.SetLDAPIdentityProviders([]*oidctestutil.TestUpstreamLDAPIdentityProvider{ + oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("some-other-ldap-idp-1").Build(), + oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("some-other-ldap-idp-2").Build(), }) - idpLister.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{ - &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-1", AllowPasswordGrant: true}, - &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-2"}, + idpLister.SetOIDCIdentityProviders([]*oidctestutil.TestUpstreamOIDCIdentityProvider{ + oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("some-other-oidc-idp-1").WithAllowPasswordGrant(true).Build(), + oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("some-other-oidc-idp-2").Build(), }) - - idpLister.SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ - &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ad-idp-2"}, - &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ad-idp-1"}, + idpLister.SetActiveDirectoryIdentityProviders([]*oidctestutil.TestUpstreamLDAPIdentityProvider{ + oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("some-other-ad-idp-2").Build(), + oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("some-other-ad-idp-1").Build(), }) // Make the same request to the same handler instance again, and expect different results. diff --git a/internal/oidc/login/post_login_handler_test.go b/internal/oidc/login/post_login_handler_test.go index cd950adc..0ac3fe51 100644 --- a/internal/oidc/login/post_login_handler_test.go +++ b/internal/oidc/login/post_login_handler_test.go @@ -171,27 +171,27 @@ func TestPostLoginEndpoint(t *testing.T) { return nil, false, nil } - upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: parsedUpstreamLDAPURL, - AuthenticateFunc: ldapAuthenticateFunc, - } + upstreamLDAPIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(parsedUpstreamLDAPURL). + WithAuthenticateFunc(ldapAuthenticateFunc). + Build() - upstreamActiveDirectoryIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: parsedUpstreamLDAPURL, - AuthenticateFunc: ldapAuthenticateFunc, - } + upstreamActiveDirectoryIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(parsedUpstreamLDAPURL). + WithAuthenticateFunc(ldapAuthenticateFunc). + Build() - erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { + erroringUpstreamLDAPIdentityProvider := oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithAuthenticateFunc(func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { return nil, false, fmt.Errorf("some ldap upstream auth error") - }, - } + }). + Build() expectedHappyActiveDirectoryUpstreamCustomSession := &psession.CustomSessionData{ Username: happyLDAPUsernameFromAuthenticator, @@ -290,8 +290,8 @@ func TestPostLoginEndpoint(t *testing.T) { { name: "happy LDAP login", idps: oidctestutil.NewUpstreamIDPListerBuilder(). - WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one - WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider), + WithLDAP(upstreamLDAPIdentityProvider). // should pick this one + WithActiveDirectory(erroringUpstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: happyUsernamePasswordFormParams, wantStatus: http.StatusSeeOther, @@ -313,8 +313,8 @@ func TestPostLoginEndpoint(t *testing.T) { { name: "happy LDAP login with dynamic client", idps: oidctestutil.NewUpstreamIDPListerBuilder(). - WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one - WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider), + WithLDAP(upstreamLDAPIdentityProvider). // should pick this one + WithActiveDirectory(erroringUpstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: happyLDAPDecodedStateForDynamicClient, formParams: happyUsernamePasswordFormParams, @@ -337,8 +337,8 @@ func TestPostLoginEndpoint(t *testing.T) { { name: "happy AD login", idps: oidctestutil.NewUpstreamIDPListerBuilder(). - WithLDAP(&erroringUpstreamLDAPIdentityProvider). - WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // should pick this one + WithLDAP(erroringUpstreamLDAPIdentityProvider). + WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), // should pick this one decodedState: happyActiveDirectoryDecodedState, formParams: happyUsernamePasswordFormParams, wantStatus: http.StatusSeeOther, @@ -360,8 +360,8 @@ func TestPostLoginEndpoint(t *testing.T) { { name: "happy AD login with dynamic client", idps: oidctestutil.NewUpstreamIDPListerBuilder(). - WithLDAP(&erroringUpstreamLDAPIdentityProvider). - WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // should pick this one + WithLDAP(erroringUpstreamLDAPIdentityProvider). + WithActiveDirectory(upstreamActiveDirectoryIdentityProvider), // should pick this one kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: happyActiveDirectoryDecodedStateForDynamicClient, formParams: happyUsernamePasswordFormParams, @@ -383,7 +383,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when downstream response_mode=form_post returns 200 with HTML+JS form", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"response_mode": "form_post"}, @@ -408,7 +408,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when downstream redirect uri matches what is configured for client except for the port number", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"redirect_uri": "http://127.0.0.1:4242/callback"}, @@ -433,7 +433,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when downstream redirect uri matches what is configured for client except for the port number with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -459,7 +459,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when there are additional allowed downstream requested scopes", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access pinniped:request-audience"}, @@ -485,7 +485,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is allowed to request username and groups but does not request them", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -511,7 +511,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is not allowed to request username and does not request username", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) { oidcClient, secret := testutil.OIDCClientAndStorageSecret(t, "some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, @@ -545,7 +545,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is not allowed to request groups and does not request groups", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) { oidcClient, secret := testutil.OIDCClientAndStorageSecret(t, "some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, @@ -579,7 +579,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "happy LDAP when downstream OIDC validations are skipped because the openid scope was not requested", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{ @@ -610,8 +610,8 @@ func TestPostLoginEndpoint(t *testing.T) { { name: "happy LDAP login when username and groups scopes are not requested", idps: oidctestutil.NewUpstreamIDPListerBuilder(). - WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one - WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider), + WithLDAP(upstreamLDAPIdentityProvider). // should pick this one + WithActiveDirectory(erroringUpstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid"}, @@ -637,7 +637,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "bad username LDAP login", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: url.Values{userParam: []string{"wrong!"}, passParam: []string{happyLDAPPassword}}, wantStatus: http.StatusSeeOther, @@ -647,7 +647,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "bad password LDAP login", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{"wrong!"}}, wantStatus: http.StatusSeeOther, @@ -657,7 +657,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "blank username LDAP login", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: url.Values{userParam: []string{""}, passParam: []string{happyLDAPPassword}}, wantStatus: http.StatusSeeOther, @@ -667,7 +667,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "blank password LDAP login", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{""}}, wantStatus: http.StatusSeeOther, @@ -677,7 +677,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "username and password sent as URI query params should be ignored since they are expected in form post body", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, reqURIQuery: happyUsernamePasswordFormParams, wantStatus: http.StatusSeeOther, @@ -687,7 +687,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "error during upstream LDAP authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(erroringUpstreamLDAPIdentityProvider), decodedState: happyLDAPDecodedState, formParams: happyUsernamePasswordFormParams, wantStatus: http.StatusSeeOther, @@ -697,7 +697,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"redirect_uri": "http://127.0.0.1/wrong_callback"}, @@ -708,7 +708,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -720,7 +720,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream client does not exist", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": "wrong_client_id"}, @@ -731,7 +731,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream client is missing", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": ""}, @@ -742,7 +742,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "response type is unsupported", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"response_type": "unsupported"}, @@ -753,7 +753,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "response type form_post is unsupported for dynamic clients", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -765,7 +765,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "response type is missing", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"response_type": ""}, @@ -776,7 +776,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "PKCE code_challenge is missing", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"code_challenge": ""}, @@ -791,7 +791,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "PKCE code_challenge_method is invalid", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"}, @@ -806,7 +806,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "PKCE code_challenge_method is `plain`", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"code_challenge_method": "plain"}, // plain is not allowed @@ -821,7 +821,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "PKCE code_challenge_method is missing", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"code_challenge_method": ""}, @@ -836,7 +836,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "PKCE code_challenge_method is missing with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -852,7 +852,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "prompt param is not allowed to have none and another legal value at the same time", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"prompt": "none login"}, @@ -867,7 +867,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream state does not have enough entropy", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"state": "short"}, @@ -878,7 +878,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access pinniped:request-audience scope_not_allowed"}, @@ -889,7 +889,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "using dynamic client which is not allowed to request username scope in authorize request but requests it anyway", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) { oidcClient, secret := testutil.OIDCClientAndStorageSecret(t, "some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, @@ -909,7 +909,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "using dynamic client which is not allowed to request groups scope in authorize request but requests it anyway", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) { oidcClient, secret := testutil.OIDCClientAndStorageSecret(t, "some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, @@ -929,7 +929,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client with dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) { data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient, @@ -948,7 +948,7 @@ func TestPostLoginEndpoint(t *testing.T) { }, { name: "upstream provider cannot be found by name and type", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(upstreamLDAPIdentityProvider), decodedState: happyActiveDirectoryDecodedState, // correct upstream IDP name, but wrong upstream IDP type formParams: happyUsernamePasswordFormParams, wantErr: "error finding upstream provider: provider not found", @@ -988,7 +988,7 @@ func TestPostLoginEndpoint(t *testing.T) { rsp := httptest.NewRecorder() - subject := NewPostHandler(downstreamIssuer, tt.idps.Build(), oauthHelper) + subject := NewPostHandler(downstreamIssuer, tt.idps.BuildFederationDomainIdentityProvidersListerFinder(), oauthHelper) err := subject(rsp, req, happyEncodedUpstreamState, tt.decodedState) if tt.wantErr != "" { diff --git a/internal/oidc/provider/federation_domain_identity_providers_lister.go b/internal/oidc/provider/federation_domain_identity_providers_lister.go index ce333d3c..855023c1 100644 --- a/internal/oidc/provider/federation_domain_identity_providers_lister.go +++ b/internal/oidc/provider/federation_domain_identity_providers_lister.go @@ -64,6 +64,11 @@ type FederationDomainIdentityProvidersListerI interface { GetActiveDirectoryIdentityProviders() []*FederationDomainResolvedLDAPIdentityProvider } +type FederationDomainIdentityProvidersListerFinderI interface { + FederationDomainIdentityProvidersListerI + FederationDomainIdentityProvidersFinderI +} + // FederationDomainIdentityProvidersLister wraps an UpstreamIdentityProvidersLister. The lister which is being // wrapped should contain all valid upstream providers that are currently defined in the Supervisor. // FederationDomainIdentityProvidersLister provides a lookup method which only looks up IDPs within those which diff --git a/internal/oidc/provider/federation_domain_issuer_test.go b/internal/oidc/provider/federation_domain_issuer_test.go index b80bff19..08a3d851 100644 --- a/internal/oidc/provider/federation_domain_issuer_test.go +++ b/internal/oidc/provider/federation_domain_issuer_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/idtransform" ) func TestFederationDomainIssuerValidations(t *testing.T) { @@ -82,7 +84,7 @@ func TestFederationDomainIssuerValidations(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - _, err := NewFederationDomainIssuer(tt.issuer, nil) + _, err := NewFederationDomainIssuer(tt.issuer, []*FederationDomainIdentityProvider{}) if tt.wantError != "" { require.EqualError(t, err, tt.wantError) } else { @@ -90,7 +92,11 @@ func TestFederationDomainIssuerValidations(t *testing.T) { } // This alternate constructor should perform all the same validations on the issuer string. - _, err = NewFederationDomainIssuerWithDefaultIDP(tt.issuer, nil) + _, err = NewFederationDomainIssuerWithDefaultIDP(tt.issuer, &FederationDomainIdentityProvider{ + DisplayName: "foobar", + UID: "foo-123", + Transforms: idtransform.NewTransformationPipeline(), + }) if tt.wantError != "" { require.EqualError(t, err, tt.wantError) } else { diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 9f407b25..45319220 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -1,10 +1,9 @@ -// 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 manager import ( - "context" "crypto/ecdsa" "encoding/json" "fmt" @@ -29,9 +28,6 @@ import ( "go.pinniped.dev/internal/secret" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" - "go.pinniped.dev/pkg/oidcclient/nonce" - "go.pinniped.dev/pkg/oidcclient/oidctypes" - "go.pinniped.dev/pkg/oidcclient/pkce" ) func TestManager(t *testing.T) { @@ -249,25 +245,19 @@ func TestManager(t *testing.T) { parsedUpstreamIDPAuthorizationURL, err := url.Parse(upstreamIDPAuthorizationURL) r.NoError(err) - idpLister := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: upstreamIDPName, - ClientID: "test-client-id", - AuthorizationURL: *parsedUpstreamIDPAuthorizationURL, - Scopes: []string{"test-scope"}, - ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { - return &oidctypes.Token{ - IDToken: &oidctypes.IDToken{ - Claims: map[string]interface{}{ - "iss": "https://some-issuer.com", - "sub": "some-subject", - "username": "test-username", - "groups": "test-group1", - }, - }, - RefreshToken: &oidctypes.RefreshToken{Token: "some-opaque-token"}, - }, nil - }, - }).Build() + idpLister := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). + WithName(upstreamIDPName). + WithClientID("test-client-id"). + WithAuthorizationURL(*parsedUpstreamIDPAuthorizationURL). + WithScopes([]string{"test-scope"}). + WithIDTokenClaim("iss", "https://some-issuer.com"). + WithIDTokenClaim("sub", "some-subject"). + WithIDTokenClaim("username", "test-username"). + WithIDTokenClaim("groups", "test-group1"). + WithRefreshToken("some-opaque-token"). + WithoutAccessToken(). + Build(), + ).BuildDynamicUpstreamIDPProvider() kubeClient = fake.NewSimpleClientset() secretsClient := kubeClient.CoreV1().Secrets("some-namespace") @@ -387,9 +377,9 @@ func TestManager(t *testing.T) { when("given some valid providers via SetProviders()", func() { it.Before(func() { - p1, err := provider.NewFederationDomainIssuer(issuer1) + p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - p2, err := provider.NewFederationDomainIssuer(issuer2) + p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) subject.SetProviders(p1, p2) @@ -430,9 +420,9 @@ func TestManager(t *testing.T) { when("given the same valid providers as arguments to SetProviders() in reverse order", func() { it.Before(func() { - p1, err := provider.NewFederationDomainIssuer(issuer1) + p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - p2, err := provider.NewFederationDomainIssuer(issuer2) + p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) subject.SetProviders(p2, p1) diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 4d2bf708..09cb1596 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -125,6 +125,7 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, } } +//nolint:funlen func upstreamOIDCRefresh( ctx context.Context, session *psession.PinnipedSession, @@ -201,7 +202,7 @@ func upstreamOIDCRefresh( var refreshedUntransformedGroups []string groupsScope := slices.Contains(grantedScopes, oidcapi.ScopeGroups) - if groupsScope { //nolint:nestif + if groupsScope { // If possible, update the user's group memberships. The configured groups claim name (if there is one) may or // may not be included in the newly fetched and merged claims. It could be missing due to a misconfiguration of the // claim name. It could also be missing because the claim was originally found in the ID token during login, but diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index bf463ef6..494346d2 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -881,7 +881,7 @@ func TestTokenEndpointAuthcodeExchange(t *testing.T) { // Authcode exchange doesn't use the upstream provider cache, so just pass an empty cache. exchangeAuthcodeForTokens(t, - test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().Build(), test.kubeResources) + test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().BuildFederationDomainIdentityProvidersListerFinder(), test.kubeResources) }) } } @@ -916,7 +916,7 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { // First call - should be successful. // Authcode exchange doesn't use the upstream provider cache, so just pass an empty cache. subject, rsp, authCode, _, secrets, oauthStore := exchangeAuthcodeForTokens(t, - test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().Build(), test.kubeResources) + test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().BuildFederationDomainIdentityProvidersListerFinder(), test.kubeResources) var parsedResponseBody map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody)) @@ -1577,7 +1577,7 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn // Authcode exchange doesn't use the upstream provider cache, so just pass an empty cache. subject, rsp, _, _, secrets, storage := exchangeAuthcodeForTokens(t, - test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().Build(), test.kubeResources) + test.authcodeExchange, oidctestutil.NewUpstreamIDPListerBuilder().BuildFederationDomainIdentityProvidersListerFinder(), test.kubeResources) var parsedAuthcodeExchangeResponseBody map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedAuthcodeExchangeResponseBody)) @@ -2466,12 +2466,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "happy path refresh grant when the upstream refresh returns new group memberships from LDAP, it updates groups", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: []string{"new-group1", "new-group2", "new-group3"}, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups([]string{"new-group1", "new-group2", "new-group3"}). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ @@ -2493,12 +2494,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "happy path refresh grant when the upstream refresh returns new group memberships from LDAP, it updates groups, using dynamic client - updates groups without outputting warnings", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: []string{"new-group1", "new-group2", "new-group3"}, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups([]string{"new-group1", "new-group2", "new-group3"}). + Build(), + ), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, authcodeExchange: authcodeExchangeInputs{ customSessionData: happyLDAPCustomSessionData, @@ -2527,12 +2529,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "happy path refresh grant when the upstream refresh returns empty list of group memberships from LDAP, it updates groups to an empty list", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: []string{}, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups([]string{}). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ @@ -2553,12 +2556,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "ldap refresh grant when the upstream refresh when username and groups scopes are not requested on original request or refresh", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: []string{"new-group1", "new-group2", "new-group3"}, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups([]string{"new-group1", "new-group2", "new-group3"}). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, customSessionData: happyLDAPCustomSessionData, @@ -2694,12 +2698,13 @@ func TestRefreshGrant(t *testing.T) { // 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"}, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups([]string{"new-group1", "new-group2", "new-group3"}). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyLDAPCustomSessionData, @@ -3469,12 +3474,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh happy path", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: goodGroups, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups(goodGroups). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, refreshRequest: refreshRequestInputs{ want: happyRefreshTokenResponseForLDAP( @@ -3484,12 +3490,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh happy path using dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: goodGroups, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups(goodGroups). + Build(), + ), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { @@ -3507,12 +3514,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh happy path without downstream username scope granted, using dynamic client", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: goodGroups, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups(goodGroups). + Build(), + ), kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources, authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { @@ -3549,12 +3557,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream active directory refresh happy path", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshGroups: goodGroups, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshGroups(goodGroups). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyActiveDirectoryCustomSessionData, @@ -3570,11 +3579,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh when the LDAP session data is nil", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: &psession.CustomSessionData{ @@ -3606,11 +3616,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream active directory refresh when the ad session data is nil", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: &psession.CustomSessionData{ @@ -3642,11 +3653,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh when the LDAP session data does not contain dn", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: &psession.CustomSessionData{ @@ -3682,11 +3694,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream active directory refresh when the active directory session data does not contain dn", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: &psession.CustomSessionData{ @@ -3722,12 +3735,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream ldap refresh returns an error", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshErr: errors.New("Some error performing upstream refresh"), - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshErr(errors.New("Some error performing upstream refresh")). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ @@ -3744,12 +3758,13 @@ func TestRefreshGrant(t *testing.T) { }, { name: "upstream active directory refresh returns an error", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: activeDirectoryUpstreamResourceUID, - URL: ldapUpstreamURL, - PerformRefreshErr: errors.New("Some error performing upstream refresh"), - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID(activeDirectoryUpstreamResourceUID). + WithURL(ldapUpstreamURL). + WithPerformRefreshErr(errors.New("Some error performing upstream refresh")). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyActiveDirectoryCustomSessionData, @@ -3810,11 +3825,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "fosite session is empty", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, modifyRefreshTokenStorage: func(t *testing.T, oauthStore *oidc.KubeStorage, secrets v1.SecretInterface, refreshToken string) { refreshTokenSignature := getFositeDataSignature(t, refreshToken) @@ -3841,11 +3857,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "groups not found in extra field when the groups scope was granted", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyLDAPCustomSessionData, @@ -3878,11 +3895,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "username in custom session is empty string during refresh", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyLDAPCustomSessionData, @@ -3915,11 +3933,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "when the ldap provider in the session storage is found but has the wrong resource UID during the refresh request", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: "the-wrong-uid", - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID("the-wrong-uid"). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ @@ -3935,11 +3954,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "when the active directory provider in the session storage is found but has the wrong resource UID during the refresh request", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: activeDirectoryUpstreamName, - ResourceUID: "the-wrong-uid", - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(activeDirectoryUpstreamName). + WithResourceUID("the-wrong-uid"). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, customSessionData: happyActiveDirectoryCustomSessionData, @@ -3961,11 +3981,12 @@ func TestRefreshGrant(t *testing.T) { }, { name: "auth time is the zero value", // time.Times can never be nil, but it is possible that it would be the zero value which would mean something's wrong - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: ldapUpstreamName, - ResourceUID: ldapUpstreamResourceUID, - URL: ldapUpstreamURL, - }), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder(). + WithName(ldapUpstreamName). + WithResourceUID(ldapUpstreamResourceUID). + WithURL(ldapUpstreamURL). + Build(), + ), authcodeExchange: happyAuthcodeExchangeInputsForLDAPUpstream, modifyRefreshTokenStorage: func(t *testing.T, oauthStore *oidc.KubeStorage, secrets v1.SecretInterface, refreshToken string) { refreshTokenSignature := getFositeDataSignature(t, refreshToken) @@ -4002,7 +4023,7 @@ func TestRefreshGrant(t *testing.T) { // its actually fine to use this function even when simulating ldap (which uses a different flow) because it's // just populating a secret in storage. subject, rsp, authCode, jwtSigningKey, secrets, oauthStore := exchangeAuthcodeForTokens(t, - test.authcodeExchange, test.idps.Build(), test.kubeResources) + test.authcodeExchange, test.idps.BuildFederationDomainIdentityProvidersListerFinder(), test.kubeResources) var parsedAuthcodeExchangeResponseBody map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedAuthcodeExchangeResponseBody)) @@ -4149,7 +4170,7 @@ func requireClaimsAreEqual(t *testing.T, claimName string, claimsOfTokenA map[st func exchangeAuthcodeForTokens( t *testing.T, test authcodeExchangeInputs, - idps provider.DynamicUpstreamIDPProvider, + idps provider.FederationDomainIdentityProvidersListerFinderI, kubeResources func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset), ) ( subject http.Handler, diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 11b5c988..5d67c2c3 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -296,7 +296,7 @@ func prepareControllers( pinnipedClient, pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), secretInformer, - plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements controllerlib.WithInformer, ), singletonWorker). diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index ee566c1f..7f989b09 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -34,6 +34,7 @@ import ( "go.pinniped.dev/internal/fositestorage/openidconnect" pkce2 "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestoragei" + "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/psession" @@ -97,15 +98,107 @@ type ValidateRefreshArgs struct { StoredAttributes upstreamprovider.RefreshAttributes } +func NewTestUpstreamLDAPIdentityProviderBuilder() *TestUpstreamLDAPIdentityProviderBuilder { + return &TestUpstreamLDAPIdentityProviderBuilder{} +} + +type TestUpstreamLDAPIdentityProviderBuilder struct { + name string + resourceUID types.UID + url *url.URL + authenticateFunc func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) + performRefreshCallCount int + performRefreshArgs []*PerformRefreshArgs + performRefreshErr error + performRefreshGroups []string + displayNameForFederationDomain string + transformsForFederationDomain *idtransform.TransformationPipeline +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithName(name string) *TestUpstreamLDAPIdentityProviderBuilder { + t.name = name + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithResourceUID(uid types.UID) *TestUpstreamLDAPIdentityProviderBuilder { + t.resourceUID = uid + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithURL(url *url.URL) *TestUpstreamLDAPIdentityProviderBuilder { + t.url = url + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithAuthenticateFunc(f func(ctx context.Context, username, password string) (*authenticators.Response, bool, error)) *TestUpstreamLDAPIdentityProviderBuilder { + t.authenticateFunc = f + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithPerformRefreshCallCount(count int) *TestUpstreamLDAPIdentityProviderBuilder { + t.performRefreshCallCount = count + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithPerformRefreshArgs(args []*PerformRefreshArgs) *TestUpstreamLDAPIdentityProviderBuilder { + t.performRefreshArgs = args + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithPerformRefreshErr(err error) *TestUpstreamLDAPIdentityProviderBuilder { + t.performRefreshErr = err + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithPerformRefreshGroups(groups []string) *TestUpstreamLDAPIdentityProviderBuilder { + t.performRefreshGroups = groups + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithDisplayNameForFederationDomain(displayName string) *TestUpstreamLDAPIdentityProviderBuilder { + t.displayNameForFederationDomain = displayName + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) WithTransformsForFederationDomain(transforms *idtransform.TransformationPipeline) *TestUpstreamLDAPIdentityProviderBuilder { + t.transformsForFederationDomain = transforms + return t +} + +func (t *TestUpstreamLDAPIdentityProviderBuilder) Build() *TestUpstreamLDAPIdentityProvider { + if t.displayNameForFederationDomain == "" { + // default it to the CR name + t.displayNameForFederationDomain = t.name + } + if t.transformsForFederationDomain == nil { + // default to an empty pipeline + t.transformsForFederationDomain = idtransform.NewTransformationPipeline() + } + return &TestUpstreamLDAPIdentityProvider{ + Name: t.name, + ResourceUID: t.resourceUID, + URL: t.url, + AuthenticateFunc: t.authenticateFunc, + performRefreshCallCount: t.performRefreshCallCount, + performRefreshArgs: t.performRefreshArgs, + PerformRefreshErr: t.performRefreshErr, + PerformRefreshGroups: t.performRefreshGroups, + DisplayNameForFederationDomain: t.displayNameForFederationDomain, + TransformsForFederationDomain: t.transformsForFederationDomain, + } +} + type TestUpstreamLDAPIdentityProvider struct { - Name string - ResourceUID types.UID - URL *url.URL - AuthenticateFunc func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) - performRefreshCallCount int - performRefreshArgs []*PerformRefreshArgs - PerformRefreshErr error - PerformRefreshGroups []string + Name string + ResourceUID types.UID + URL *url.URL + AuthenticateFunc func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) + performRefreshCallCount int + performRefreshArgs []*PerformRefreshArgs + PerformRefreshErr error + PerformRefreshGroups []string + DisplayNameForFederationDomain string + TransformsForFederationDomain *idtransform.TransformationPipeline } var _ upstreamprovider.UpstreamLDAPIdentityProviderI = &TestUpstreamLDAPIdentityProvider{} @@ -155,18 +248,20 @@ func (u *TestUpstreamLDAPIdentityProvider) PerformRefreshArgs(call int) *Perform } type TestUpstreamOIDCIdentityProvider struct { - Name string - ClientID string - ResourceUID types.UID - AuthorizationURL url.URL - UserInfoURL bool - RevocationURL *url.URL - UsernameClaim string - GroupsClaim string - Scopes []string - AdditionalAuthcodeParams map[string]string - AdditionalClaimMappings map[string]string - AllowPasswordGrant bool + Name string + ClientID string + ResourceUID types.UID + AuthorizationURL url.URL + UserInfoURL bool + RevocationURL *url.URL + UsernameClaim string + GroupsClaim string + Scopes []string + AdditionalAuthcodeParams map[string]string + AdditionalClaimMappings map[string]string + AllowPasswordGrant bool + DisplayNameForFederationDomain string + TransformsForFederationDomain *idtransform.TransformationPipeline ExchangeAuthcodeAndValidateTokensFunc func( ctx context.Context, @@ -364,6 +459,104 @@ func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenAndMergeWithUserInfoArgs return u.validateTokenAndMergeWithUserInfoArgs[call] } +type TestFederationDomainIdentityProvidersListerFinder struct { + upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider + upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider + upstreamActiveDirectoryIdentityProviders []*TestUpstreamLDAPIdentityProvider +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) GetOIDCIdentityProviders() []*provider.FederationDomainResolvedOIDCIdentityProvider { + fdIDPs := make([]*provider.FederationDomainResolvedOIDCIdentityProvider, len(t.upstreamOIDCIdentityProviders)) + for i, testIDP := range t.upstreamOIDCIdentityProviders { + fdIDP := &provider.FederationDomainResolvedOIDCIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeOIDC, + Transforms: testIDP.TransformsForFederationDomain, + } + fdIDPs[i] = fdIDP + } + return fdIDPs +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) GetLDAPIdentityProviders() []*provider.FederationDomainResolvedLDAPIdentityProvider { + fdIDPs := make([]*provider.FederationDomainResolvedLDAPIdentityProvider, len(t.upstreamLDAPIdentityProviders)) + for i, testIDP := range t.upstreamLDAPIdentityProviders { + fdIDP := &provider.FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeLDAP, + Transforms: testIDP.TransformsForFederationDomain, + } + fdIDPs[i] = fdIDP + } + return fdIDPs +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) GetActiveDirectoryIdentityProviders() []*provider.FederationDomainResolvedLDAPIdentityProvider { + fdIDPs := make([]*provider.FederationDomainResolvedLDAPIdentityProvider, len(t.upstreamActiveDirectoryIdentityProviders)) + for i, testIDP := range t.upstreamActiveDirectoryIdentityProviders { + fdIDP := &provider.FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeActiveDirectory, + Transforms: testIDP.TransformsForFederationDomain, + } + fdIDPs[i] = fdIDP + } + return fdIDPs +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) FindDefaultIDP() (*provider.FederationDomainResolvedOIDCIdentityProvider, *provider.FederationDomainResolvedLDAPIdentityProvider, error) { + return nil, nil, fmt.Errorf("TODO: implement me") // TODO +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) FindUpstreamIDPByDisplayName(upstreamIDPDisplayName string) (*provider.FederationDomainResolvedOIDCIdentityProvider, *provider.FederationDomainResolvedLDAPIdentityProvider, error) { + for _, testIDP := range t.upstreamOIDCIdentityProviders { + if upstreamIDPDisplayName == testIDP.DisplayNameForFederationDomain { + return &provider.FederationDomainResolvedOIDCIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeOIDC, + Transforms: testIDP.TransformsForFederationDomain, + }, nil, nil + } + } + for _, testIDP := range t.upstreamLDAPIdentityProviders { + if upstreamIDPDisplayName == testIDP.DisplayNameForFederationDomain { + return nil, &provider.FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeLDAP, + Transforms: testIDP.TransformsForFederationDomain, + }, nil + } + } + for _, testIDP := range t.upstreamActiveDirectoryIdentityProviders { + if upstreamIDPDisplayName == testIDP.DisplayNameForFederationDomain { + return nil, &provider.FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeActiveDirectory, + Transforms: testIDP.TransformsForFederationDomain, + }, nil + } + } + return nil, nil, fmt.Errorf("did not find IDP with name %q", upstreamIDPDisplayName) +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) SetOIDCIdentityProviders(providers []*TestUpstreamOIDCIdentityProvider) { + t.upstreamOIDCIdentityProviders = providers +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) SetLDAPIdentityProviders(providers []*TestUpstreamLDAPIdentityProvider) { + t.upstreamLDAPIdentityProviders = providers +} + +func (t *TestFederationDomainIdentityProvidersListerFinder) SetActiveDirectoryIdentityProviders(providers []*TestUpstreamLDAPIdentityProvider) { + t.upstreamActiveDirectoryIdentityProviders = providers +} + type UpstreamIDPListerBuilder struct { upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider @@ -385,7 +578,15 @@ func (b *UpstreamIDPListerBuilder) WithActiveDirectory(upstreamActiveDirectoryId return b } -func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { +func (b *UpstreamIDPListerBuilder) BuildFederationDomainIdentityProvidersListerFinder() *TestFederationDomainIdentityProvidersListerFinder { + return &TestFederationDomainIdentityProvidersListerFinder{ + upstreamOIDCIdentityProviders: b.upstreamOIDCIdentityProviders, + upstreamLDAPIdentityProviders: b.upstreamLDAPIdentityProviders, + upstreamActiveDirectoryIdentityProviders: b.upstreamActiveDirectoryIdentityProviders, + } +} + +func (b *UpstreamIDPListerBuilder) BuildDynamicUpstreamIDPProvider() provider.DynamicUpstreamIDPProvider { idpProvider := provider.NewDynamicUpstreamIDPProvider() oidcUpstreams := make([]upstreamprovider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders)) @@ -643,6 +844,8 @@ type TestUpstreamOIDCIdentityProviderBuilder struct { performRefreshErr error revokeTokenErr error validateTokenAndMergeWithUserInfoErr error + displayNameForFederationDomain string + transformsForFederationDomain *idtransform.TransformationPipeline } func (u *TestUpstreamOIDCIdentityProviderBuilder) WithName(value string) *TestUpstreamOIDCIdentityProviderBuilder { @@ -792,19 +995,40 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithRevokeTokenError(err error return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithDisplayNameForFederationDomain(displayName string) *TestUpstreamOIDCIdentityProviderBuilder { + u.displayNameForFederationDomain = displayName + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithTransformsForFederationDomain(transforms *idtransform.TransformationPipeline) *TestUpstreamOIDCIdentityProviderBuilder { + u.transformsForFederationDomain = transforms + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdentityProvider { + if u.displayNameForFederationDomain == "" { + // default it to the CR name + u.displayNameForFederationDomain = u.name + } + if u.transformsForFederationDomain == nil { + // default to an empty pipeline + u.transformsForFederationDomain = idtransform.NewTransformationPipeline() + } + return &TestUpstreamOIDCIdentityProvider{ - Name: u.name, - ClientID: u.clientID, - ResourceUID: u.resourceUID, - UsernameClaim: u.usernameClaim, - GroupsClaim: u.groupsClaim, - Scopes: u.scopes, - AllowPasswordGrant: u.allowPasswordGrant, - AuthorizationURL: u.authorizationURL, - UserInfoURL: u.hasUserInfoURL, - AdditionalAuthcodeParams: u.additionalAuthcodeParams, - AdditionalClaimMappings: u.additionalClaimMappings, + Name: u.name, + ClientID: u.clientID, + ResourceUID: u.resourceUID, + UsernameClaim: u.usernameClaim, + GroupsClaim: u.groupsClaim, + Scopes: u.scopes, + AllowPasswordGrant: u.allowPasswordGrant, + AuthorizationURL: u.authorizationURL, + UserInfoURL: u.hasUserInfoURL, + AdditionalAuthcodeParams: u.additionalAuthcodeParams, + AdditionalClaimMappings: u.additionalClaimMappings, + DisplayNameForFederationDomain: u.displayNameForFederationDomain, + TransformsForFederationDomain: u.transformsForFederationDomain, ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { if u.authcodeExchangeErr != nil { return nil, u.authcodeExchangeErr diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index e0b08adc..871d6014 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -2338,7 +2338,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { state: state.State("test-state"), pkce: pkce.Code("test-pkce"), nonce: nonce.Nonce("test-nonce"), - logger: plog.Logr(), //nolint:staticcheck // old test with no log assertions + logger: plog.Logr(), //nolint:staticcheck // old test with no log assertions issuer: "https://valid-issuer.com/with/some/path", } if tt.opt != nil {