Implement upstream LDAP support in auth_handler.go

- When the upstream IDP is an LDAP IDP and the user's LDAP username and
  password are received as new custom headers, then authenticate the
  user and, if authentication was successful, return a redirect with
  an authcode. Handle errors according to the OAuth/OIDC specs.
- Still does not support having multiple upstream IDPs defined at the
  same time, which was an existing limitation of this endpoint.
- Does not yet include the actual LDAP authentication, which is
  hidden behind an interface from the point of view of auth_handler.go
- Move the oidctestutil package to the testutil directory.
- Add an interface for Fosite storage to avoid a cyclical test
  dependency.
- Add GetURL() to the UpstreamLDAPIdentityProviderI interface.
- Extract test helpers to be shared between callback_handler_test.go
  and auth_handler_test.go because the authcode and fosite storage
  assertions should be identical.
- Backfill Content-Type assertions in callback_handler_test.go.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go

@@ -24,9 +24,9 @@ import (
 	pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
 	"go.pinniped.dev/internal/controllerlib"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
 	"go.pinniped.dev/internal/testutil/testlogger"
 	"go.pinniped.dev/internal/upstreamoidc"
 )

internal/fositestoragei/fosite_sotrage_interface.go

@@ -0,0 +1,21 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package fositestoragei
+
+import (
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/handler/oauth2"
+	"github.com/ory/fosite/handler/openid"
+	"github.com/ory/fosite/handler/pkce"
+)
+
+// This interface seems to be missing from Fosite.
+// Not having this interface makes it a pain to avoid cyclical test dependencies, so we'll define it.
+type AllFositeStorage interface {
+	fosite.ClientManager
+	oauth2.CoreStorage
+	oauth2.TokenRevocationStorage
+	openid.OpenIDConnectRequestStorage
+	pkce.PKCERequestStorage
+}

internal/ldap/ldap.go

@@ -13,6 +13,21 @@ import (
 // This interface is similar to the k8s token authenticator, but works with username/passwords instead
 // of a single token string.
 //
+// The return values should be as follows.
+// 1. For a successful authentication:
+//    - A response which includes the username, uid, and groups in the userInfo. The username and uid must not be blank.
+//    - true
+//    - nil error
+// 2. For an unsuccessful authentication, e.g. bad username or password:
+//    - nil response
+//    - false
+//    - nil error
+// 3. For an unexpected error, e.g. a network problem:
+//    - nil response
+//    - false
+//    - an error
+// Other combinations of return values must be avoided.
+//
 // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator
 // interface, as well as the Response type.
 type UserAuthenticator interface {

internal/oidc/auth/auth_handler.go

@@ -13,6 +13,7 @@ import (
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/ory/fosite/token/jwt"
+	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
 
 	"go.pinniped.dev/internal/httputil/httperr"
@@ -25,11 +26,16 @@ import (
 	"go.pinniped.dev/pkg/oidcclient/pkce"
 )
 
+const (
+	CustomUsernameHeaderName = "X-Pinniped-Upstream-Username"
+	CustomPasswordHeaderName = "X-Pinniped-Upstream-Password" //nolint:gosec // this is not a credential
+)
+
 func NewHandler(
 	downstreamIssuer string,
 	idpLister oidc.UpstreamIdentityProvidersLister,
-	oauthHelperWithNullStorage fosite.OAuth2Provider,
+	oauthHelperWithoutStorage fosite.OAuth2Provider,
-	oauthHelperWithRealStorage fosite.OAuth2Provider,
+	oauthHelperWithStorage fosite.OAuth2Provider,
 	generateCSRF func() (csrftoken.CSRFToken, error),
 	generatePKCE func() (pkce.Code, error),
 	generateNonce func() (nonce.Nonce, error),
@@ -44,109 +50,203 @@ func NewHandler(
 			return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method)
 		}
 
-		csrfFromCookie := readCSRFCookie(r, cookieCodec)
+		oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpLister)
-
-		authorizeRequester, err := oauthHelperWithNullStorage.NewAuthorizeRequest(r.Context(), r)
-		if err != nil {
-			plog.Info("authorize request error", oidc.FositeErrorForLog(err)...)
-			oauthHelperWithNullStorage.WriteAuthorizeError(w, authorizeRequester, err)
-			return nil
-		}
-
-		upstreamIDP, err := chooseUpstreamIDP(idpLister)
 		if err != nil {
 			plog.WarningErr("authorize upstream config", err)
 			return err
 		}
 
-		// Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
+		if oidcUpstream != nil {
-		oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
+			return handleAuthRequestForOIDCUpstream(r, w,
-		// There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope
+				oauthHelperWithoutStorage,
-		// at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite.
+				generateCSRF, generateNonce, generatePKCE,
-		oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
+				oidcUpstream,
-
+				downstreamIssuer,
-		// Grant the pinniped:request-audience scope if requested.
+				upstreamStateEncoder,
-		oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
+				cookieCodec,
-
+			)
-		now := time.Now()
-		_, err = oauthHelperWithNullStorage.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{
-			Claims: &jwt.IDTokenClaims{
-				// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
-				Subject:     "none",
-				AuthTime:    now,
-				RequestedAt: now,
-			},
-		})
-		if err != nil {
-			plog.Info("authorize response error", oidc.FositeErrorForLog(err)...)
-			oauthHelperWithNullStorage.WriteAuthorizeError(w, authorizeRequester, err)
-			return nil
 		}
-
+		return handleAuthRequestForLDAPUpstream(r, w,
-		csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
+			oauthHelperWithStorage,
-		if err != nil {
+			ldapUpstream,
-			plog.Error("authorize generate error", err)
-			return err
-		}
-		if csrfFromCookie != "" {
-			csrfValue = csrfFromCookie
-		}
-
-		upstreamOAuthConfig := oauth2.Config{
-			ClientID: upstreamIDP.GetClientID(),
-			Endpoint: oauth2.Endpoint{
-				AuthURL: upstreamIDP.GetAuthorizationURL().String(),
-			},
-			RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer),
-			Scopes:      upstreamIDP.GetScopes(),
-		}
-
-		encodedStateParamValue, err := upstreamStateParam(
-			authorizeRequester,
-			upstreamIDP.GetName(),
-			nonceValue,
-			csrfValue,
-			pkceValue,
-			upstreamStateEncoder,
 		)
-		if err != nil {
-			plog.Error("authorize upstream state param error", err)
-			return err
-		}
-
-		if csrfFromCookie == "" {
-			// We did not receive an incoming CSRF cookie, so write a new one.
-			err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec)
-			if err != nil {
-				plog.Error("error setting CSRF cookie", err)
-				return err
-			}
-		}
-
-		authCodeOptions := []oauth2.AuthCodeOption{
-			oauth2.AccessTypeOffline,
-			nonceValue.Param(),
-			pkceValue.Challenge(),
-			pkceValue.Method(),
-		}
-
-		promptParam := r.Form.Get("prompt")
-		if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
-			authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam))
-		}
-
-		http.Redirect(w, r,
-			upstreamOAuthConfig.AuthCodeURL(
-				encodedStateParamValue,
-				authCodeOptions...,
-			),
-			302,
-		)
-
-		return nil
 	}))
 }
 
+func handleAuthRequestForLDAPUpstream(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	ldapUpstream provider.UpstreamLDAPIdentityProviderI,
+) error {
+	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper)
+	if !created {
+		return nil
+	}
+
+	username := r.Header.Get(CustomUsernameHeaderName)
+	password := r.Header.Get(CustomPasswordHeaderName)
+	if username == "" || password == "" {
+		// Return an error according to OIDC spec 3.1.2.6 (second paragraph).
+		err := errors.WithStack(fosite.ErrAccessDenied.WithHintf("Missing or blank username or password."))
+		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+		return nil
+	}
+
+	authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password)
+	if err != nil {
+		plog.WarningErr("unexpected error during upstream authentication", err, "upstreamName", ldapUpstream.GetName())
+		return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication")
+	}
+	if !authenticated {
+		// Return an error according to OIDC spec 3.1.2.6 (second paragraph).
+		err = errors.WithStack(fosite.ErrAccessDenied.WithHintf("Username/password not accepted by LDAP provider."))
+		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+		return nil
+	}
+
+	subject := fmt.Sprintf("%s?%s=%s", ldapUpstream.GetURL(), oidc.IDTokenSubjectClaim, authenticateResponse.User.GetUID())
+	now := time.Now().UTC()
+	openIDSession := &openid.DefaultSession{
+		Claims: &jwt.IDTokenClaims{
+			Subject:     subject,
+			RequestedAt: now,
+			AuthTime:    now,
+		},
+	}
+	openIDSession.Claims.Extra = map[string]interface{}{
+		oidc.DownstreamUsernameClaim: authenticateResponse.User.GetName(),
+		oidc.DownstreamGroupsClaim:   authenticateResponse.User.GetGroups(),
+	}
+
+	authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
+	if err != nil {
+		plog.Info("authorize response error", oidc.FositeErrorForLog(err)...)
+		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+		return nil
+	}
+
+	oauthHelper.WriteAuthorizeResponse(w, authorizeRequester, authorizeResponder)
+	return nil
+}
+
+func handleAuthRequestForOIDCUpstream(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	generateCSRF func() (csrftoken.CSRFToken, error),
+	generateNonce func() (nonce.Nonce, error),
+	generatePKCE func() (pkce.Code, error),
+	oidcUpstream provider.UpstreamOIDCIdentityProviderI,
+	downstreamIssuer string,
+	upstreamStateEncoder oidc.Encoder,
+	cookieCodec oidc.Codec,
+) error {
+	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper)
+	if !created {
+		return nil
+	}
+
+	now := time.Now()
+	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{
+		Claims: &jwt.IDTokenClaims{
+			// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
+			Subject:     "none",
+			AuthTime:    now,
+			RequestedAt: now,
+		},
+	})
+	if err != nil {
+		plog.Info("authorize response error", oidc.FositeErrorForLog(err)...)
+		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+		return nil
+	}
+
+	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
+	if err != nil {
+		plog.Error("authorize generate error", err)
+		return err
+	}
+	csrfFromCookie := readCSRFCookie(r, cookieCodec)
+	if csrfFromCookie != "" {
+		csrfValue = csrfFromCookie
+	}
+
+	upstreamOAuthConfig := oauth2.Config{
+		ClientID: oidcUpstream.GetClientID(),
+		Endpoint: oauth2.Endpoint{
+			AuthURL: oidcUpstream.GetAuthorizationURL().String(),
+		},
+		RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer),
+		Scopes:      oidcUpstream.GetScopes(),
+	}
+
+	encodedStateParamValue, err := upstreamStateParam(
+		authorizeRequester,
+		oidcUpstream.GetName(),
+		nonceValue,
+		csrfValue,
+		pkceValue,
+		upstreamStateEncoder,
+	)
+	if err != nil {
+		plog.Error("authorize upstream state param error", err)
+		return err
+	}
+
+	if csrfFromCookie == "" {
+		// We did not receive an incoming CSRF cookie, so write a new one.
+		err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec)
+		if err != nil {
+			plog.Error("error setting CSRF cookie", err)
+			return err
+		}
+	}
+
+	authCodeOptions := []oauth2.AuthCodeOption{
+		oauth2.AccessTypeOffline,
+		nonceValue.Param(),
+		pkceValue.Challenge(),
+		pkceValue.Method(),
+	}
+
+	promptParam := r.Form.Get("prompt")
+	if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
+		authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam))
+	}
+
+	http.Redirect(w, r,
+		upstreamOAuthConfig.AuthCodeURL(
+			encodedStateParamValue,
+			authCodeOptions...,
+		),
+		302,
+	)
+
+	return nil
+}
+
+func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider) (fosite.AuthorizeRequester, bool) {
+	authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r)
+	if err != nil {
+		plog.Info("authorize request error", oidc.FositeErrorForLog(err)...)
+		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+		return nil, false
+	}
+	grantScopes(authorizeRequester)
+	return authorizeRequester, true
+}
+
+func grantScopes(authorizeRequester fosite.AuthorizeRequester) {
+	// Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
+	oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
+	// There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope
+	// at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite.
+	oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
+	// Grant the pinniped:request-audience scope if requested.
+	oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
+}
+
 func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {
 	receivedCSRFCookie, err := r.Cookie(oidc.CSRFCookieName)
 	if err != nil {
@@ -166,27 +266,34 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {
 	return csrfFromCookie
 }
 
-func chooseUpstreamIDP(idpLister oidc.UpstreamOIDCIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, error) {
+// Select either an OIDC or an LDAP IDP, or return an error.
-	allUpstreamIDPs := idpLister.GetOIDCIdentityProviders()
+func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, error) {
-	if len(allUpstreamIDPs) == 0 {
+	oidcUpstreams := idpLister.GetOIDCIdentityProviders()
-		return nil, httperr.New(
+	ldapUpstreams := idpLister.GetLDAPIdentityProviders()
+	switch {
+	case len(oidcUpstreams)+len(ldapUpstreams) == 0:
+		return nil, nil, httperr.New(
 			http.StatusUnprocessableEntity,
 			"No upstream providers are configured",
 		)
-	} else if len(allUpstreamIDPs) > 1 {
+	case len(oidcUpstreams)+len(ldapUpstreams) > 1:
 		var upstreamIDPNames []string
-		for _, idp := range allUpstreamIDPs {
+		for _, idp := range oidcUpstreams {
+			upstreamIDPNames = append(upstreamIDPNames, idp.GetName())
+		}
+		for _, idp := range ldapUpstreams {
 			upstreamIDPNames = append(upstreamIDPNames, idp.GetName())
 		}
-
 		plog.Warning("Too many upstream providers are configured (found: %s)", upstreamIDPNames)
-
+		return nil, nil, httperr.New(
-		return nil, httperr.New(
 			http.StatusUnprocessableEntity,
 			"Too many upstream providers are configured (support for multiple upstreams is not yet implemented)",
 		)
+	case len(oidcUpstreams) == 1:
+		return oidcUpstreams[0], nil, nil
+	default:
+		return nil, ldapUpstreams[0], nil
 	}
-	return allUpstreamIDPs[0], nil
 }
 
 func generateValues(

internal/oidc/auth/auth_handler_test.go

@@ -4,6 +4,7 @@
 package auth
 
 import (
+	"context"
 	"fmt"
 	"html"
 	"net/http"
@@ -13,18 +14,21 @@ import (
 	"strings"
 	"testing"
 
-	"k8s.io/client-go/kubernetes/fake"
-
 	"github.com/gorilla/securecookie"
+	"github.com/ory/fosite"
 	"github.com/stretchr/testify/require"
+	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/client-go/kubernetes/fake"
+	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/jwks"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
 	"go.pinniped.dev/pkg/oidcclient/pkce"
 )
@@ -34,7 +38,13 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		downstreamIssuer                       = "https://my-downstream-issuer.com/some-path"
 		downstreamRedirectURI                  = "http://127.0.0.1/callback"
 		downstreamRedirectURIWithDifferentPort = "http://127.0.0.1:42/callback"
+		downstreamNonce                        = "some-nonce-value"
+		downstreamPKCEChallenge                = "some-challenge"
+		downstreamPKCEChallengeMethod          = "S256"
 		happyState                             = "8b-state"
+		downstreamClientID                     = "pinniped-cli"
+		upstreamLDAPURL                        = "ldaps://some-ldap-host:123"
+		htmlContentType                        = "text/html; charset=utf-8"
 	)
 
 	require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
@@ -101,20 +111,31 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"error_description": "The authorization server does not support obtaining a token using this method. `The request is missing the 'response_type' parameter.",
 			"state":             happyState,
 		}
-	)
 
-	kubeClient := fake.NewSimpleClientset()
+		fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery = map[string]string{
-	secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+			"error":             "access_denied",
+			"error_description": "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.",
+			"state":             happyState,
+		}
+
+		fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery = map[string]string{
+			"error":             "access_denied",
+			"error_description": "The resource owner or authorization server denied the request. Missing or blank username or password.",
+			"state":             happyState,
+		}
+	)
 
 	hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") }
 	require.GreaterOrEqual(t, len(hmacSecretFunc()), 32, "fosite requires that hmac secrets have at least 32 bytes")
 	jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
-
-	// Configure fosite the same way that the production code would when using Kube storage.
-	// Inject this into our test subject at the last second so we get a fresh storage for every test.
 	timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
-	kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration)
+
-	oauthHelperWithRealStorage := oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
+	createOauthHelperWithRealStorage := func(secretsClient v1.SecretInterface) (fosite.OAuth2Provider, *oidc.KubeStorage) {
+		// Configure fosite the same way that the production code would when using Kube storage.
+		// Inject this into our test subject at the last second so we get a fresh storage for every test.
+		kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration)
+		return oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), kubeOauthStore
+	}
 
 	// Configure fosite the same way that the production code would, using NullStorage to turn off storage.
 	nullOauthStore := oidc.NullStorage{}
@@ -124,12 +145,45 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	require.NoError(t, err)
 
 	upstreamOIDCIdentityProvider := oidctestutil.TestUpstreamOIDCIdentityProvider{
-		Name:             "some-idp",
+		Name:             "some-oidc-idp",
 		ClientID:         "some-client-id",
 		AuthorizationURL: *upstreamAuthURL,
 		Scopes:           []string{"scope1", "scope2"}, // the scopes to request when starting the upstream authorization flow
 	}
 
+	happyLDAPUsername := "some-ldap-user"
+	happyLDAPUsernameFromAuthenticator := "some-mapped-ldap-username"
+	happyLDAPPassword := "some-ldap-password" //nolint:gosec
+	happyLDAPUID := "some-ldap-uid"
+	happyLDAPGroups := []string{"group1", "group2", "group3"}
+
+	upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{
+		Name: "some-ldap-idp",
+		URL:  upstreamLDAPURL,
+		AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) {
+			if username == "" || password == "" {
+				return nil, false, fmt.Errorf("should not have passed empty username or password to the authenticator")
+			}
+			if username == happyLDAPUsername && password == happyLDAPPassword {
+				return &authenticator.Response{
+					User: &user.DefaultInfo{
+						Name:   happyLDAPUsernameFromAuthenticator,
+						UID:    happyLDAPUID,
+						Groups: happyLDAPGroups,
+					},
+				}, true, nil
+			}
+			return nil, false, nil
+		},
+	}
+
+	erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{
+		Name: "some-ldap-idp",
+		AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) {
+			return nil, false, fmt.Errorf("some ldap upstream auth error")
+		},
+	}
+
 	happyCSRF := "test-csrf"
 	happyPKCE := "test-pkce"
 	happyNonce := "test-nonce"
@@ -177,14 +231,17 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		return urlToReturn
 	}
 
+	happyDownstreamScopesRequested := []string{"openid", "profile", "email"}
+	happyDownstreamScopesGranted := []string{"openid"}
+
 	happyGetRequestQueryMap := map[string]string{
 		"response_type":         "code",
-		"scope":                 "openid profile email",
+		"scope":                 strings.Join(happyDownstreamScopesRequested, " "),
-		"client_id":             "pinniped-cli",
+		"client_id":             downstreamClientID,
 		"state":                 happyState,
-		"nonce":                 "some-nonce-value",
+		"nonce":                 downstreamNonce,
-		"code_challenge":        "some-challenge",
+		"code_challenge":        downstreamPKCEChallenge,
-		"code_challenge_method": "S256",
+		"code_challenge_method": downstreamPKCEChallengeMethod,
 		"redirect_uri":          downstreamRedirectURI,
 	}
 
@@ -242,7 +299,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"state":                 expectedUpstreamState,
 			"nonce":                 happyNonce,
 			"code_challenge":        expectedUpstreamCodeChallenge,
-			"code_challenge_method": "S256",
+			"code_challenge_method": downstreamPKCEChallengeMethod,
 			"redirect_uri":          downstreamIssuer + "/callback",
 		}
 		if expectedPrompt != "" {
@@ -251,6 +308,9 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		return urlWithQuery(upstreamAuthURL.String(), query)
 	}
 
+	// Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it
+	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyState
+
 	incomingCookieCSRFValue := "csrf-value-from-cookie"
 	encodedIncomingCookieCSRFValue, err := happyCookieEncoder.Encode("csrf", incomingCookieCSRFValue)
 	require.NoError(t, err)
@@ -258,27 +318,41 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	type testCase struct {
 		name string
 
-		idpLister     provider.DynamicUpstreamIDPProvider
+		idpLister            provider.DynamicUpstreamIDPProvider
-		generateCSRF  func() (csrftoken.CSRFToken, error)
+		generateCSRF         func() (csrftoken.CSRFToken, error)
-		generatePKCE  func() (pkce.Code, error)
+		generatePKCE         func() (pkce.Code, error)
-		generateNonce func() (nonce.Nonce, error)
+		generateNonce        func() (nonce.Nonce, error)
-		stateEncoder  oidc.Codec
+		stateEncoder         oidc.Codec
-		cookieEncoder oidc.Codec
+		cookieEncoder        oidc.Codec
-		method        string
+		method               string
-		path          string
+		path                 string
-		contentType   string
+		contentType          string
-		body          string
+		body                 string
-		csrfCookie    string
+		csrfCookie           string
+		customUsernameHeader *string // nil means do not send header, empty means send header with empty value
+		customPasswordHeader *string // nil means do not send header, empty means send header with empty value
 
-		wantStatus                  int
+		wantStatus                             int
-		wantContentType             string
+		wantContentType                        string
-		wantBodyString              string
+		wantBodyString                         string
-		wantBodyJSON                string
+		wantBodyJSON                           string
-		wantLocationHeader          string
+		wantCSRFValueInCookieHeader            string
-		wantCSRFValueInCookieHeader string
-
-		wantUpstreamStateParamInLocationHeader bool
 		wantBodyStringWithLocationInHref       bool
+		wantLocationHeader                     string
+		wantUpstreamStateParamInLocationHeader bool
+
+		// For when the request was authenticated by an upstream LDAP provider and an authcode is being returned.
+		wantRedirectLocationRegexp        string
+		wantDownstreamRedirectURI         string
+		wantDownstreamGrantedScopes       []string
+		wantDownstreamIDTokenSubject      string
+		wantDownstreamIDTokenUsername     string
+		wantDownstreamIDTokenGroups       []string
+		wantDownstreamRequestedScopes     []string
+		wantDownstreamPKCEChallenge       string
+		wantDownstreamPKCEChallengeMethod string
+		wantDownstreamNonce               string
+		wantUnnecessaryStoredRecords      int
 	}
 	tests := []testCase{
 		{
@@ -292,12 +366,33 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:                                 http.MethodGet,
 			path:                                   happyGetRequestPath,
 			wantStatus:                             http.StatusFound,
-			wantContentType:                        "text/html; charset=utf-8",
+			wantContentType:                        htmlContentType,
 			wantCSRFValueInCookieHeader:            happyCSRF,
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:                              "LDAP upstream happy path using GET",
+			idpLister:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                            http.MethodGet,
+			path:                              happyGetRequestPath,
+			customUsernameHeader:              stringPtr(happyLDAPUsername),
+			customPasswordHeader:              stringPtr(happyLDAPPassword),
+			wantStatus:                        http.StatusFound,
+			wantContentType:                   htmlContentType,
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantBodyStringWithLocationInHref:  false,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "?sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+		},
 		{
 			name:                                   "OIDC upstream happy path using GET with a CSRF cookie",
 			idpLister:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
@@ -310,7 +405,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			path:                                   happyGetRequestPath,
 			csrfCookie:                             "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
 			wantStatus:                             http.StatusFound,
-			wantContentType:                        "text/html; charset=utf-8",
+			wantContentType:                        htmlContentType,
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
@@ -334,6 +429,29 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
+		{
+			name:                              "LDAP upstream happy path using POST",
+			idpLister:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                            http.MethodPost,
+			path:                              "/some/path",
+			contentType:                       "application/x-www-form-urlencoded",
+			body:                              encodeQuery(happyGetRequestQueryMap),
+			customUsernameHeader:              stringPtr(happyLDAPUsername),
+			customPasswordHeader:              stringPtr(happyLDAPPassword),
+			wantStatus:                        http.StatusFound,
+			wantContentType:                   htmlContentType,
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantBodyStringWithLocationInHref:  false,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "?sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+		},
 		{
 			name:                                   "OIDC upstream happy path with prompt param login passed through to redirect uri",
 			idpLister:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
@@ -347,14 +465,14 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			contentType:                            "application/x-www-form-urlencoded",
 			body:                                   encodeQuery(happyGetRequestQueryMap),
 			wantStatus:                             http.StatusFound,
-			wantContentType:                        "text/html; charset=utf-8",
+			wantContentType:                        htmlContentType,
 			wantBodyStringWithLocationInHref:       true,
 			wantCSRFValueInCookieHeader:            happyCSRF,
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), "login"),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
 		{
-			name:            "error while decoding CSRF cookie just generates a new cookie and succeeds as usual",
+			name:            "OIDC upstream with error while decoding CSRF cookie just generates a new cookie and succeeds as usual",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -365,7 +483,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			path:            happyGetRequestPath,
 			csrfCookie:      "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
 			wantStatus:      http.StatusFound,
-			wantContentType: "text/html; charset=utf-8",
+			wantContentType: htmlContentType,
 			// Generated a new CSRF cookie and set it in the response.
 			wantCSRFValueInCookieHeader:            happyCSRF,
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""),
@@ -385,7 +503,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
 			}),
 			wantStatus:                  http.StatusFound,
-			wantContentType:             "text/html; charset=utf-8",
+			wantContentType:             htmlContentType,
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{
 				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
@@ -393,6 +511,29 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:      "LDAP upstream happy path when downstream redirect uri matches what is configured for client except for the port number",
+			idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:    http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
+			}),
+			customUsernameHeader:              stringPtr(happyLDAPUsername),
+			customPasswordHeader:              stringPtr(happyLDAPPassword),
+			wantStatus:                        http.StatusFound,
+			wantContentType:                   htmlContentType,
+			wantRedirectLocationRegexp:        downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid&state=` + happyState,
+			wantBodyStringWithLocationInHref:  false,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "?sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURIWithDifferentPort,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+		},
 		{
 			name:                        "OIDC upstream happy path when downstream requested scopes include offline_access",
 			idpLister:                   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
@@ -404,7 +545,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:                      http.MethodGet,
 			path:                        modifiedHappyGetRequestPath(map[string]string{"scope": "openid offline_access"}),
 			wantStatus:                  http.StatusFound,
-			wantContentType:             "text/html; charset=utf-8",
+			wantContentType:             htmlContentType,
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{
 				"scope": "openid offline_access",
@@ -413,7 +554,66 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyStringWithLocationInHref:       true,
 		},
 		{
-			name:          "downstream redirect uri does not match what is configured for client",
+			name:                 "error during upstream LDAP authentication",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusBadGateway,
+			wantContentType:      htmlContentType,
+			wantBodyString:       "Bad Gateway: unexpected error during upstream authentication\n",
+		},
+		{
+			name:                 "wrong upstream password for LDAP authentication",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: stringPtr("wrong-password"),
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "wrong upstream username for LDAP authentication",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: stringPtr("wrong-username"),
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "missing upstream username on request for LDAP authentication",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: nil, // do not send header
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "missing upstream password on request for LDAP authentication",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: nil, // do not send header
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:          "downstream redirect uri does not match what is configured for client when using OIDC upstream",
 			idpLister:     oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:  happyCSRFGenerator,
 			generatePKCE:  happyPKCEGenerator,
@@ -429,7 +629,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyJSON:    fositeInvalidRedirectURIErrorBody,
 		},
 		{
-			name:            "downstream client does not exist",
+			name:      "downstream redirect uri does not match what is configured for client when using LDAP upstream",
+			idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			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",
+			}),
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusBadRequest,
+			wantContentType:      "application/json; charset=utf-8",
+			wantBodyJSON:         fositeInvalidRedirectURIErrorBody,
+		},
+		{
+			name:            "downstream client does not exist when using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -443,7 +656,16 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
-			name:               "response type is unsupported",
+			name:            "downstream client does not exist when using LDAP upstream",
+			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:          http.MethodGet,
+			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}),
+			wantStatus:      http.StatusUnauthorized,
+			wantContentType: "application/json; charset=utf-8",
+			wantBodyJSON:    fositeInvalidClientErrorBody,
+		},
+		{
+			name:               "response type is unsupported when using OIDC upstream",
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -458,7 +680,17 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:               "downstream scopes do not match what is configured for client",
+			name:               "response type is unsupported when using LDAP upstream",
+			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
+			wantStatus:         http.StatusFound,
+			wantContentType:    "application/json; charset=utf-8",
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
+		{
+			name:               "downstream scopes do not match what is configured for client using OIDC upstream",
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -473,7 +705,19 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:               "missing response type in request",
+			name:                 "downstream scopes do not match what is configured for client using LDAP upstream",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"scope": "openid tuna"}),
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:               "missing response type in request using OIDC upstream",
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -488,7 +732,17 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:            "missing client id in request",
+			name:               "missing response type in request using LDAP upstream",
+			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
+			wantStatus:         http.StatusFound,
+			wantContentType:    "application/json; charset=utf-8",
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
+		{
+			name:            "missing client id in request using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -502,7 +756,16 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
-			name:               "missing PKCE code_challenge in request", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			name:            "missing client id in request using LDAP upstream",
+			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:          http.MethodGet,
+			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": ""}),
+			wantStatus:      http.StatusUnauthorized,
+			wantContentType: "application/json; charset=utf-8",
+			wantBodyJSON:    fositeInvalidClientErrorBody,
+		},
+		{
+			name:               "missing PKCE code_challenge in request using OIDC upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -517,7 +780,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:               "invalid value for PKCE code_challenge_method in request", // https://tools.ietf.org/html/rfc7636#section-4.3
+			name:                         "missing PKCE code_challenge in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			idpLister:                    oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                       http.MethodGet,
+			path:                         modifiedHappyGetRequestPath(map[string]string{"code_challenge": ""}),
+			customUsernameHeader:         stringPtr(happyLDAPUsername),
+			customPasswordHeader:         stringPtr(happyLDAPPassword),
+			wantStatus:                   http.StatusFound,
+			wantContentType:              "application/json; charset=utf-8",
+			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
+			wantBodyString:               "",
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name:               "invalid value for PKCE code_challenge_method in request using OIDC upstream", // https://tools.ietf.org/html/rfc7636#section-4.3
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -532,7 +808,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:               "when PKCE code_challenge_method in request is `plain`", // https://tools.ietf.org/html/rfc7636#section-4.3
+			name:                         "invalid value for PKCE code_challenge_method in request using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3
+			idpLister:                    oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                       http.MethodGet,
+			path:                         modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"}),
+			customUsernameHeader:         stringPtr(happyLDAPUsername),
+			customPasswordHeader:         stringPtr(happyLDAPPassword),
+			wantStatus:                   http.StatusFound,
+			wantContentType:              "application/json; charset=utf-8",
+			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
+			wantBodyString:               "",
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name:               "when PKCE code_challenge_method in request is `plain` using OIDC upstream", // https://tools.ietf.org/html/rfc7636#section-4.3
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -547,7 +836,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:               "missing PKCE code_challenge_method in request", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			name:                         "when PKCE code_challenge_method in request is `plain` using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3
+			idpLister:                    oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                       http.MethodGet,
+			path:                         modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "plain"}),
+			customUsernameHeader:         stringPtr(happyLDAPUsername),
+			customPasswordHeader:         stringPtr(happyLDAPPassword),
+			wantStatus:                   http.StatusFound,
+			wantContentType:              "application/json; charset=utf-8",
+			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantBodyString:               "",
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name:               "missing PKCE code_challenge_method in request using OIDC upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -561,10 +863,23 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:                         "missing PKCE code_challenge_method in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			idpLister:                    oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                       http.MethodGet,
+			path:                         modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": ""}),
+			customUsernameHeader:         stringPtr(happyLDAPUsername),
+			customPasswordHeader:         stringPtr(happyLDAPPassword),
+			wantStatus:                   http.StatusFound,
+			wantContentType:              "application/json; charset=utf-8",
+			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantBodyString:               "",
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
 		{
 			// 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.
+			// through that part of the fosite library when using an OIDC upstream.
-			name:               "prompt param is not allowed to have none and another legal value at the same time",
+			name:               "prompt param is not allowed to have none and another legal value at the same time using OIDC upstream",
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -579,7 +894,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:          "OIDC validations are skipped when the openid scope was not requested",
+			// 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",
+			idpLister:                    oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:                       http.MethodGet,
+			path:                         modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}),
+			customUsernameHeader:         stringPtr(happyLDAPUsername),
+			customPasswordHeader:         stringPtr(happyLDAPPassword),
+			wantStatus:                   http.StatusFound,
+			wantContentType:              "application/json; charset=utf-8",
+			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
+			wantBodyString:               "",
+			wantUnnecessaryStoredRecords: 1, // fosite already stored the authcode before it noticed the error
+		},
+		{
+			name:          "happy path: downstream OIDC validations are skipped when the openid scope was not requested using OIDC upstream",
 			idpLister:     oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:  happyCSRFGenerator,
 			generatePKCE:  happyPKCEGenerator,
@@ -590,7 +920,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			// 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"}),
 			wantStatus:                  http.StatusFound,
-			wantContentType:             "text/html; charset=utf-8",
+			wantContentType:             htmlContentType,
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(
 				map[string]string{"prompt": "none login", "scope": "email"}, "", "",
@@ -599,7 +929,29 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyStringWithLocationInHref:       true,
 		},
 		{
-			name:               "state does not have enough entropy",
+			name:      "happy path: downstream OIDC validations are skipped when the openid scope was not requested using LDAP upstream",
+			idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			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"}),
+			customUsernameHeader:              stringPtr(happyLDAPUsername),
+			customPasswordHeader:              stringPtr(happyLDAPPassword),
+			wantStatus:                        http.StatusFound,
+			wantContentType:                   htmlContentType,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyState, // no scopes granted
+			wantBodyStringWithLocationInHref:  false,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "?sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     []string{"email"}, // only email was requested
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       []string{}, // no scopes granted
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+		},
+		{
+			name:               "downstream state does not have enough entropy using OIDC upstream",
 			idpLister:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:       happyCSRFGenerator,
 			generatePKCE:       happyPKCEGenerator,
@@ -614,7 +966,19 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:     "",
 		},
 		{
-			name:            "error while encoding upstream state param",
+			name:                 "downstream state does not have enough entropy using LDAP upstream",
+			idpLister:            oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"state": "short"}),
+			customUsernameHeader: stringPtr(happyLDAPUsername),
+			customPasswordHeader: stringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      "application/json; charset=utf-8",
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:            "error while encoding upstream state param using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -628,7 +992,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:  "Internal Server Error: error encoding upstream state param\n",
 		},
 		{
-			name:            "error while encoding CSRF cookie value for new cookie",
+			name:            "error while encoding CSRF cookie value for new cookie using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -642,7 +1006,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:  "Internal Server Error: error encoding CSRF cookie\n",
 		},
 		{
-			name:            "error while generating CSRF token",
+			name:            "error while generating CSRF token using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    sadCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -656,7 +1020,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:  "Internal Server Error: error generating CSRF token\n",
 		},
 		{
-			name:            "error while generating nonce",
+			name:            "error while generating nonce using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    happyPKCEGenerator,
@@ -670,7 +1034,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:  "Internal Server Error: error generating nonce param\n",
 		},
 		{
-			name:            "error while generating PKCE",
+			name:            "error while generating PKCE using OIDC upstream",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
 			generateCSRF:    happyCSRFGenerator,
 			generatePKCE:    sadPKCEGenerator,
@@ -693,7 +1057,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:  "Unprocessable Entity: No upstream providers are configured\n",
 		},
 		{
-			name:            "too many upstream providers are configured",
+			name:            "too many upstream providers are configured: multiple OIDC",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider, &upstreamOIDCIdentityProvider).Build(), // more than one not allowed
 			method:          http.MethodGet,
 			path:            happyGetRequestPath,
@@ -701,6 +1065,24 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType: "text/plain; charset=utf-8",
 			wantBodyString:  "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n",
 		},
+		{
+			name:            "too many upstream providers are configured: multiple LDAP",
+			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider, &upstreamLDAPIdentityProvider).Build(), // more than one not allowed
+			method:          http.MethodGet,
+			path:            happyGetRequestPath,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n",
+		},
+		{
+			name:            "too many upstream providers are configured: both OIDC and LDAP",
+			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).WithLDAP(&upstreamLDAPIdentityProvider).Build(), // more than one not allowed
+			method:          http.MethodGet,
+			path:            happyGetRequestPath,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n",
+		},
 		{
 			name:            "PUT is a bad method",
 			idpLister:       oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(),
@@ -730,12 +1112,18 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		},
 	}
 
-	runOneTestCase := func(t *testing.T, test testCase, subject http.Handler) {
+	runOneTestCase := func(t *testing.T, test testCase, subject http.Handler, kubeOauthStore *oidc.KubeStorage, kubeClient *fake.Clientset, secretsClient v1.SecretInterface) {
 		req := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body))
 		req.Header.Set("Content-Type", test.contentType)
 		if test.csrfCookie != "" {
 			req.Header.Set("Cookie", test.csrfCookie)
 		}
+		if test.customUsernameHeader != nil {
+			req.Header.Set("X-Pinniped-Upstream-Username", *test.customUsernameHeader)
+		}
+		if test.customPasswordHeader != nil {
+			req.Header.Set("X-Pinniped-Upstream-Password", *test.customPasswordHeader)
+		}
 		rsp := httptest.NewRecorder()
 		subject.ServeHTTP(rsp, req)
 		t.Logf("response: %#v", rsp)
@@ -746,7 +1134,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		testutil.RequireSecurityHeaders(t, rsp)
 
 		actualLocation := rsp.Header().Get("Location")
-		if test.wantLocationHeader != "" {
+		switch {
+		case test.wantLocationHeader != "":
 			if test.wantUpstreamStateParamInLocationHeader {
 				requireEqualDecodedStateParams(t, actualLocation, test.wantLocationHeader, test.stateEncoder)
 			}
@@ -754,7 +1143,34 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			// compare those states since they may be different, but we do want to compare the downstream
 			// state param that should be exactly the same.
 			requireEqualURLs(t, actualLocation, test.wantLocationHeader, test.wantUpstreamStateParamInLocationHeader)
-		} else {
+
+			// Authorization requests for either a successful OIDC upstream or for an error with any upstream
+			// should never use Kube storage. There is only one exception to this rule, which is that certain
+			// OIDC validations are checked in fosite after the OAuth authcode (and sometimes the OIDC session)
+			// is stored, so it is possible with an LDAP upstream to store objects and then return an error to
+			// the client anyway (which makes the stored objects useless, but oh well).
+			require.Len(t, kubeClient.Actions(), test.wantUnnecessaryStoredRecords)
+		case test.wantRedirectLocationRegexp != "":
+			require.Len(t, rsp.Header().Values("Location"), 1)
+			oidctestutil.RequireAuthcodeRedirectLocation(
+				t,
+				rsp.Header().Get("Location"),
+				test.wantRedirectLocationRegexp,
+				kubeClient,
+				secretsClient,
+				kubeOauthStore,
+				test.wantDownstreamGrantedScopes,
+				test.wantDownstreamIDTokenSubject,
+				test.wantDownstreamIDTokenUsername,
+				test.wantDownstreamIDTokenGroups,
+				test.wantDownstreamRequestedScopes,
+				test.wantDownstreamPKCEChallenge,
+				test.wantDownstreamPKCEChallengeMethod,
+				test.wantDownstreamNonce,
+				downstreamClientID,
+				test.wantDownstreamRedirectURI,
+			)
+		default:
 			require.Empty(t, rsp.Header().Values("Location"))
 		}
 
@@ -782,14 +1198,14 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		} else {
 			require.Empty(t, rsp.Header().Values("Set-Cookie"))
 		}
-
-		// Authorization requests for an OIDC upstream should never use Kube storage.
-		require.Len(t, kubeClient.Actions(), 0)
 	}
 
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
+			kubeClient := fake.NewSimpleClientset()
+			secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+			oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient)
 			subject := NewHandler(
 				downstreamIssuer,
 				test.idpLister,
@@ -797,7 +1213,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				test.generateCSRF, test.generatePKCE, test.generateNonce,
 				test.stateEncoder, test.cookieEncoder,
 			)
-			runOneTestCase(t, test, subject)
+			runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
 		})
 	}
 
@@ -805,6 +1221,9 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		test := tests[0]
 		require.Equal(t, "OIDC upstream happy path using GET without a CSRF cookie", test.name) // re-use the happy path test case
 
+		kubeClient := fake.NewSimpleClientset()
+		secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+		oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient)
 		subject := NewHandler(
 			downstreamIssuer,
 			test.idpLister,
@@ -813,7 +1232,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			test.stateEncoder, test.cookieEncoder,
 		)
 
-		runOneTestCase(t, test, subject)
+		runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
 
 		// Call the setter to change the upstream IDP settings.
 		newProviderSettings := oidctestutil.TestUpstreamOIDCIdentityProvider{
@@ -834,7 +1253,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				"state":                 expectedUpstreamStateParam(nil, "", newProviderSettings.Name),
 				"nonce":                 happyNonce,
 				"code_challenge":        expectedUpstreamCodeChallenge,
-				"code_challenge_method": "S256",
+				"code_challenge_method": downstreamPKCEChallengeMethod,
 				"redirect_uri":          downstreamIssuer + "/callback",
 			},
 		)
@@ -847,7 +1266,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		// modified expectations. This should ensure that the implementation is using the in-memory cache
 		// of upstream IDP settings appropriately in terms of always getting the values from the cache
 		// on every request.
-		runOneTestCase(t, test, subject)
+		runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
 	})
 }
 
@@ -912,3 +1331,7 @@ func requireEqualURLs(t *testing.T, actualURL string, expectedURL string, ignore
 	}
 	require.Equal(t, expectedLocationQuery, actualLocationQuery)
 }
+
+func stringPtr(s string) *string {
+	return &s
+}

internal/oidc/callback/callback_handler_test.go

@@ -9,26 +9,17 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"regexp"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/gorilla/securecookie"
-	"github.com/ory/fosite"
-	"github.com/ory/fosite/handler/openid"
 	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/kubernetes/fake"
 
-	"go.pinniped.dev/internal/crud"
-	"go.pinniped.dev/internal/fositestorage/authorizationcode"
-	"go.pinniped.dev/internal/fositestorage/openidconnect"
-	"go.pinniped.dev/internal/fositestorage/pkce"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
 	"go.pinniped.dev/pkg/oidcclient/oidctypes"
 	oidcpkce "go.pinniped.dev/pkg/oidcclient/pkce"
@@ -44,8 +35,7 @@ const (
 	upstreamUsernameClaim = "the-user-claim"
 	upstreamGroupsClaim   = "the-groups-claim"
 
-	happyUpstreamAuthcode = "upstream-auth-code"
+	happyUpstreamAuthcode    = "upstream-auth-code"
-
 	happyUpstreamRedirectURI = "https://example.com/callback"
 
 	happyDownstreamState        = "8b-state"
@@ -61,8 +51,7 @@ const (
 	downstreamPKCEChallenge       = "some-challenge"
 	downstreamPKCEChallengeMethod = "S256"
 
-	authCodeExpirationSeconds = 10 * 60 // Current, we set our auth code expiration to 10 minutes
+	htmlContentType = "text/html; charset=utf-8"
-	timeComparisonFudgeFactor = time.Second * 15
 )
 
 var (
@@ -129,6 +118,7 @@ func TestCallbackEndpoint(t *testing.T) {
 		csrfCookie string
 
 		wantStatus                        int
+		wantContentType                   string
 		wantBody                          string
 		wantRedirectLocationRegexp        string
 		wantDownstreamGrantedScopes       []string
@@ -252,6 +242,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: email_verified claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -264,6 +255,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: email_verified claim in upstream ID token has false value\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -327,57 +319,64 @@ func TestCallbackEndpoint(t *testing.T) {
 
 		// Pre-upstream-exchange verification
 		{
-			name:       "PUT method is invalid",
+			name:            "PUT method is invalid",
-			method:     http.MethodPut,
+			method:          http.MethodPut,
-			path:       newRequestPath().String(),
+			path:            newRequestPath().String(),
-			wantStatus: http.StatusMethodNotAllowed,
+			wantStatus:      http.StatusMethodNotAllowed,
-			wantBody:   "Method Not Allowed: PUT (try GET)\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: PUT (try GET)\n",
 		},
 		{
-			name:       "POST method is invalid",
+			name:            "POST method is invalid",
-			method:     http.MethodPost,
+			method:          http.MethodPost,
-			path:       newRequestPath().String(),
+			path:            newRequestPath().String(),
-			wantStatus: http.StatusMethodNotAllowed,
+			wantStatus:      http.StatusMethodNotAllowed,
-			wantBody:   "Method Not Allowed: POST (try GET)\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: POST (try GET)\n",
 		},
 		{
-			name:       "PATCH method is invalid",
+			name:            "PATCH method is invalid",
-			method:     http.MethodPatch,
+			method:          http.MethodPatch,
-			path:       newRequestPath().String(),
+			path:            newRequestPath().String(),
-			wantStatus: http.StatusMethodNotAllowed,
+			wantStatus:      http.StatusMethodNotAllowed,
-			wantBody:   "Method Not Allowed: PATCH (try GET)\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: PATCH (try GET)\n",
 		},
 		{
-			name:       "DELETE method is invalid",
+			name:            "DELETE method is invalid",
-			method:     http.MethodDelete,
+			method:          http.MethodDelete,
-			path:       newRequestPath().String(),
+			path:            newRequestPath().String(),
-			wantStatus: http.StatusMethodNotAllowed,
+			wantStatus:      http.StatusMethodNotAllowed,
-			wantBody:   "Method Not Allowed: DELETE (try GET)\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: DELETE (try GET)\n",
 		},
 		{
-			name:       "code param was not included on request",
+			name:            "code param was not included on request",
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyState).WithoutCode().String(),
+			path:            newRequestPath().WithState(happyState).WithoutCode().String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusBadRequest,
+			wantStatus:      http.StatusBadRequest,
-			wantBody:   "Bad Request: code param not found\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: code param not found\n",
 		},
 		{
-			name:       "state param was not included on request",
+			name:            "state param was not included on request",
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithoutState().String(),
+			path:            newRequestPath().WithoutState().String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusBadRequest,
+			wantStatus:      http.StatusBadRequest,
-			wantBody:   "Bad Request: state param not found\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: state param not found\n",
 		},
 		{
-			name:       "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason",
+			name:            "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason",
-			idp:        happyUpstream().Build(),
+			idp:             happyUpstream().Build(),
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState("this-will-not-decode").String(),
+			path:            newRequestPath().WithState("this-will-not-decode").String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusBadRequest,
+			wantStatus:      http.StatusBadRequest,
-			wantBody:   "Bad Request: error reading state\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error reading state\n",
 		},
 		{
 			// This shouldn't happen in practice because the authorize endpoint should have already run the same
@@ -393,16 +392,18 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:                        happyCSRFCookie,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 			wantStatus:                        http.StatusInternalServerError,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Internal Server Error: error while generating and saving authcode\n",
 		},
 		{
-			name:       "state's internal version does not match what we want",
+			name:            "state's internal version does not match what we want",
-			idp:        happyUpstream().Build(),
+			idp:             happyUpstream().Build(),
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyUpstreamStateParam().WithStateVersion("wrong-state-version").Build(t, happyStateCodec)).String(),
+			path:            newRequestPath().WithState(happyUpstreamStateParam().WithStateVersion("wrong-state-version").Build(t, happyStateCodec)).String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusUnprocessableEntity,
+			wantStatus:      http.StatusUnprocessableEntity,
-			wantBody:   "Unprocessable Entity: state format version is invalid\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: state format version is invalid\n",
 		},
 		{
 			name:   "state's downstream auth params element is invalid",
@@ -411,9 +412,10 @@ func TestCallbackEndpoint(t *testing.T) {
 			path: newRequestPath().WithState(happyUpstreamStateParam().
 				WithAuthorizeRequestParams("the following is an invalid url encoding token, and therefore this is an invalid param: %z").
 				Build(t, happyStateCodec)).String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusBadRequest,
+			wantStatus:      http.StatusBadRequest,
-			wantBody:   "Bad Request: error reading state downstream auth params\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error reading state downstream auth params\n",
 		},
 		{
 			name:   "state's downstream auth params are missing required value (e.g., client_id)",
@@ -424,9 +426,10 @@ func TestCallbackEndpoint(t *testing.T) {
 					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": ""}).Encode()).
 					Build(t, happyStateCodec),
 			).String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusBadRequest,
+			wantStatus:      http.StatusBadRequest,
-			wantBody:   "Bad Request: error using state downstream auth params\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error using state downstream auth params\n",
 		},
 		{
 			name:   "state's downstream auth params does not contain openid scope",
@@ -474,39 +477,43 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
-			name:       "the OIDCIdentityProvider CRD has been deleted",
+			name:            "the OIDCIdentityProvider CRD has been deleted",
-			idp:        otherUpstreamOIDCIdentityProvider,
+			idp:             otherUpstreamOIDCIdentityProvider,
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyState).String(),
+			path:            newRequestPath().WithState(happyState).String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusUnprocessableEntity,
+			wantStatus:      http.StatusUnprocessableEntity,
-			wantBody:   "Unprocessable Entity: upstream provider not found\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: upstream provider not found\n",
 		},
 		{
-			name:       "the CSRF cookie does not exist on request",
+			name:            "the CSRF cookie does not exist on request",
-			idp:        happyUpstream().Build(),
+			idp:             happyUpstream().Build(),
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyState).String(),
+			path:            newRequestPath().WithState(happyState).String(),
-			wantStatus: http.StatusForbidden,
+			wantStatus:      http.StatusForbidden,
-			wantBody:   "Forbidden: CSRF cookie is missing\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF cookie is missing\n",
 		},
 		{
-			name:       "cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason",
+			name:            "cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason",
-			idp:        happyUpstream().Build(),
+			idp:             happyUpstream().Build(),
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyState).String(),
+			path:            newRequestPath().WithState(happyState).String(),
-			csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
+			csrfCookie:      "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
-			wantStatus: http.StatusForbidden,
+			wantStatus:      http.StatusForbidden,
-			wantBody:   "Forbidden: error reading CSRF cookie\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: error reading CSRF cookie\n",
 		},
 		{
-			name:       "cookie csrf value does not match state csrf value",
+			name:            "cookie csrf value does not match state csrf value",
-			idp:        happyUpstream().Build(),
+			idp:             happyUpstream().Build(),
-			method:     http.MethodGet,
+			method:          http.MethodGet,
-			path:       newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(),
+			path:            newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(),
-			csrfCookie: happyCSRFCookie,
+			csrfCookie:      happyCSRFCookie,
-			wantStatus: http.StatusForbidden,
+			wantStatus:      http.StatusForbidden,
-			wantBody:   "Forbidden: CSRF value does not match\n",
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF value does not match\n",
 		},
 
 		// Upstream exchange
@@ -518,6 +525,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusBadGateway,
 			wantBody:                          "Bad Gateway: error exchanging and validating upstream tokens\n",
+			wantContentType:                   htmlContentType,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
@@ -528,6 +536,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
 			wantBody:                          "Unprocessable Entity: no username claim in upstream ID token\n",
+			wantContentType:                   htmlContentType,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
@@ -556,6 +565,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: username claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -566,6 +576,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: issuer claim in upstream ID token missing\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -576,6 +587,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: issuer claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -586,6 +598,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: groups claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -596,6 +609,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: groups claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -606,6 +620,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusUnprocessableEntity,
+			wantContentType:                   htmlContentType,
 			wantBody:                          "Unprocessable Entity: groups claim in upstream ID token has invalid format\n",
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
@@ -648,6 +663,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			}
 
 			require.Equal(t, test.wantStatus, rsp.Code)
+			testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType)
 
 			if test.wantBody != "" {
 				require.Equal(t, test.wantBody, rsp.Body.String())
@@ -656,79 +672,30 @@ func TestCallbackEndpoint(t *testing.T) {
 			}
 
 			if test.wantRedirectLocationRegexp != "" { //nolint:nestif // don't mind have several sequential if statements in this test
-				// Assert that Location header matches regular expression.
 				require.Len(t, rsp.Header().Values("Location"), 1)
-				actualLocation := rsp.Header().Get("Location")
+				oidctestutil.RequireAuthcodeRedirectLocation(
-				regex := regexp.MustCompile(test.wantRedirectLocationRegexp)
-				submatches := regex.FindStringSubmatch(actualLocation)
-				require.Lenf(t, submatches, 2, "no regexp match in actualLocation: %q", actualLocation)
-				capturedAuthCode := submatches[1]
-
-				// fosite authcodes are in the format `data.signature`, so grab the signature part, which is the lookup key in the storage interface
-				authcodeDataAndSignature := strings.Split(capturedAuthCode, ".")
-				require.Len(t, authcodeDataAndSignature, 2)
-
-				// Several Secrets should have been created
-				expectedNumberOfCreatedSecrets := 2
-				if includesOpenIDScope(test.wantDownstreamGrantedScopes) {
-					expectedNumberOfCreatedSecrets++
-				}
-				require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets)
-
-				// One authcode should have been stored.
-				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
-
-				storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage(
 					t,
+					rsp.Header().Get("Location"),
+					test.wantRedirectLocationRegexp,
+					client,
+					secrets,
 					oauthStore,
-					authcodeDataAndSignature[1], // Authcode store key is authcode signature
 					test.wantDownstreamGrantedScopes,
 					test.wantDownstreamIDTokenSubject,
 					test.wantDownstreamIDTokenUsername,
 					test.wantDownstreamIDTokenGroups,
 					test.wantDownstreamRequestedScopes,
-				)
-
-				// One PKCE should have been stored.
-				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: pkce.TypeLabelValue}, 1)
-
-				validatePKCEStorage(
-					t,
-					oauthStore,
-					authcodeDataAndSignature[1], // PKCE store key is authcode signature
-					storedRequestFromAuthcode,
-					storedSessionFromAuthcode,
 					test.wantDownstreamPKCEChallenge,
 					test.wantDownstreamPKCEChallengeMethod,
+					test.wantDownstreamNonce,
+					downstreamClientID,
+					downstreamRedirectURI,
 				)
-
-				// One IDSession should have been stored, if the downstream actually requested the "openid" scope
-				if includesOpenIDScope(test.wantDownstreamGrantedScopes) {
-					testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
-
-					validateIDSessionStorage(
-						t,
-						oauthStore,
-						capturedAuthCode, // IDSession store key is full authcode
-						storedRequestFromAuthcode,
-						storedSessionFromAuthcode,
-						test.wantDownstreamNonce,
-					)
-				}
 			}
 		})
 	}
 }
 
-func includesOpenIDScope(scopes []string) bool {
-	for _, scope := range scopes {
-		if scope == "openid" {
-			return true
-		}
-	}
-	return false
-}
-
 type requestPath struct {
 	code, state *string
 }
@@ -898,141 +865,3 @@ func shallowCopyAndModifyQuery(query url.Values, modifications map[string]string
 	}
 	return copied
 }
-
-func validateAuthcodeStorage(
-	t *testing.T,
-	oauthStore *oidc.KubeStorage,
-	storeKey string,
-	wantDownstreamGrantedScopes []string,
-	wantDownstreamIDTokenSubject string,
-	wantDownstreamIDTokenUsername string,
-	wantDownstreamIDTokenGroups []string,
-	wantDownstreamRequestedScopes []string,
-) (*fosite.Request, *openid.DefaultSession) {
-	t.Helper()
-
-	// Get the authcode session back from storage so we can require that it was stored correctly.
-	storedAuthorizeRequestFromAuthcode, err := oauthStore.GetAuthorizeCodeSession(context.Background(), storeKey, nil)
-	require.NoError(t, err)
-
-	// Check that storage returned the expected concrete data types.
-	storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode)
-
-	// Check which scopes were granted.
-	require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes())
-
-	// Check all the other fields of the stored request.
-	require.NotEmpty(t, storedRequestFromAuthcode.ID)
-	require.Equal(t, downstreamClientID, storedRequestFromAuthcode.Client.GetID())
-	require.ElementsMatch(t, wantDownstreamRequestedScopes, storedRequestFromAuthcode.RequestedScope)
-	require.Nil(t, storedRequestFromAuthcode.RequestedAudience)
-	require.Empty(t, storedRequestFromAuthcode.GrantedAudience)
-	require.Equal(t, url.Values{"redirect_uri": []string{downstreamRedirectURI}}, storedRequestFromAuthcode.Form)
-	testutil.RequireTimeInDelta(t, time.Now(), storedRequestFromAuthcode.RequestedAt, timeComparisonFudgeFactor)
-
-	// We're not using these fields yet, so confirm that we did not set them (for now).
-	require.Empty(t, storedSessionFromAuthcode.Subject)
-	require.Empty(t, storedSessionFromAuthcode.Username)
-	require.Empty(t, storedSessionFromAuthcode.Headers)
-
-	// The authcode that we are issuing should be good for the length of time that we declare in the fosite config.
-	testutil.RequireTimeInDelta(t, time.Now().Add(authCodeExpirationSeconds*time.Second), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor)
-	require.Len(t, storedSessionFromAuthcode.ExpiresAt, 1)
-
-	// Now confirm the ID token claims.
-	actualClaims := storedSessionFromAuthcode.Claims
-
-	// Check the user's identity, which are put into the downstream ID token's subject, username and groups claims.
-	require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject)
-	require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"])
-	require.Len(t, actualClaims.Extra, 2)
-	actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
-	require.NotNil(t, actualDownstreamIDTokenGroups)
-	require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups)
-
-	// Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time).
-	testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor)
-	testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.AuthTime, timeComparisonFudgeFactor)
-	requestedAtZone, _ := actualClaims.RequestedAt.Zone()
-	require.Equal(t, "UTC", requestedAtZone)
-	authTimeZone, _ := actualClaims.AuthTime.Zone()
-	require.Equal(t, "UTC", authTimeZone)
-
-	// Fosite will set these fields for us in the token endpoint based on the store session
-	// information. Therefore, we assert that they are empty because we want the library to do the
-	// lifting for us.
-	require.Empty(t, actualClaims.Issuer)
-	require.Nil(t, actualClaims.Audience)
-	require.Empty(t, actualClaims.Nonce)
-	require.Zero(t, actualClaims.ExpiresAt)
-	require.Zero(t, actualClaims.IssuedAt)
-
-	// These are not needed yet.
-	require.Empty(t, actualClaims.JTI)
-	require.Empty(t, actualClaims.CodeHash)
-	require.Empty(t, actualClaims.AccessTokenHash)
-	require.Empty(t, actualClaims.AuthenticationContextClassReference)
-	require.Empty(t, actualClaims.AuthenticationMethodsReference)
-
-	return storedRequestFromAuthcode, storedSessionFromAuthcode
-}
-
-func validatePKCEStorage(
-	t *testing.T,
-	oauthStore *oidc.KubeStorage,
-	storeKey string,
-	storedRequestFromAuthcode *fosite.Request,
-	storedSessionFromAuthcode *openid.DefaultSession,
-	wantDownstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod string,
-) {
-	t.Helper()
-
-	storedAuthorizeRequestFromPKCE, err := oauthStore.GetPKCERequestSession(context.Background(), storeKey, nil)
-	require.NoError(t, err)
-
-	// Check that storage returned the expected concrete data types.
-	storedRequestFromPKCE, storedSessionFromPKCE := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromPKCE)
-
-	// The stored PKCE request should be the same as the stored authcode request.
-	require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromPKCE.ID)
-	require.Equal(t, storedSessionFromAuthcode, storedSessionFromPKCE)
-
-	// The stored PKCE request should also contain the PKCE challenge that the downstream sent us.
-	require.Equal(t, wantDownstreamPKCEChallenge, storedRequestFromPKCE.Form.Get("code_challenge"))
-	require.Equal(t, wantDownstreamPKCEChallengeMethod, storedRequestFromPKCE.Form.Get("code_challenge_method"))
-}
-
-func validateIDSessionStorage(
-	t *testing.T,
-	oauthStore *oidc.KubeStorage,
-	storeKey string,
-	storedRequestFromAuthcode *fosite.Request,
-	storedSessionFromAuthcode *openid.DefaultSession,
-	wantDownstreamNonce string,
-) {
-	t.Helper()
-
-	storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil)
-	require.NoError(t, err)
-
-	// Check that storage returned the expected concrete data types.
-	storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession)
-
-	// The stored IDSession request should be the same as the stored authcode request.
-	require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID)
-	require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession)
-
-	// The stored IDSession request should also contain the nonce that the downstream sent us.
-	require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce"))
-}
-
-func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requester) (*fosite.Request, *openid.DefaultSession) {
-	t.Helper()
-
-	storedRequest, ok := storedAuthorizeRequest.(*fosite.Request)
-	require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest, &fosite.Request{})
-	storedSession, ok := storedAuthorizeRequest.GetSession().(*openid.DefaultSession)
-	require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest.GetSession(), &openid.DefaultSession{})
-
-	return storedRequest, storedSession
-}

internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package oidc
@@ -8,14 +8,13 @@ import (
 	"crypto/ecdsa"
 	"reflect"
 
-	"go.pinniped.dev/internal/constable"
-	"go.pinniped.dev/internal/plog"
-
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
 	"github.com/ory/fosite/handler/openid"
 
+	"go.pinniped.dev/internal/constable"
 	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/plog"
 )
 
 // dynamicOpenIDConnectECDSAStrategy is an openid.OpenIDConnectTokenStrategy that can dynamically

internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package oidc
@@ -21,7 +21,7 @@ import (
 	"gopkg.in/square/go-jose.v2"
 
 	"go.pinniped.dev/internal/oidc/jwks"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
 )
 
 func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) {

internal/oidc/kube_storage.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package oidc
@@ -19,6 +19,7 @@ import (
 	"go.pinniped.dev/internal/fositestorage/openidconnect"
 	"go.pinniped.dev/internal/fositestorage/pkce"
 	"go.pinniped.dev/internal/fositestorage/refreshtoken"
+	"go.pinniped.dev/internal/fositestoragei"
 )
 
 const errKubeStorageNotImplemented = constable.Error("KubeStorage does not implement this method. It should not have been called.")
@@ -31,6 +32,8 @@ type KubeStorage struct {
 	refreshTokenStorage      refreshtoken.RevocationStorage
 }
 
+var _ fositestoragei.AllFositeStorage = &KubeStorage{}
+
 func NewKubeStorage(secrets corev1client.SecretInterface, timeoutsConfiguration TimeoutsConfiguration) *KubeStorage {
 	nowFunc := time.Now
 	return &KubeStorage{

internal/oidc/nullstorage.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package oidc
@@ -10,12 +10,15 @@ import (
 	"github.com/ory/fosite"
 
 	"go.pinniped.dev/internal/constable"
+	"go.pinniped.dev/internal/fositestoragei"
 )
 
 const errNullStorageNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.")
 
 type NullStorage struct{}
 
+var _ fositestoragei.AllFositeStorage = &NullStorage{}
+
 func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error {
 	return errNullStorageNotImplemented
 }

internal/oidc/oidctestutil/oidc.go

@@ -1,187 +0,0 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package oidctestutil
-
-import (
-	"context"
-	"crypto"
-	"crypto/ecdsa"
-	"fmt"
-	"net/url"
-	"testing"
-
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
-	"github.com/stretchr/testify/require"
-	"golang.org/x/oauth2"
-	"gopkg.in/square/go-jose.v2"
-
-	"go.pinniped.dev/internal/oidc/provider"
-	"go.pinniped.dev/pkg/oidcclient/nonce"
-	"go.pinniped.dev/pkg/oidcclient/oidctypes"
-	"go.pinniped.dev/pkg/oidcclient/pkce"
-)
-
-// Test helpers for the OIDC package.
-
-// ExchangeAuthcodeAndValidateTokenArgs is a POGO (plain old go object?) used to spy on calls to
-// TestUpstreamOIDCIdentityProvider.ExchangeAuthcodeAndValidateTokensFunc().
-type ExchangeAuthcodeAndValidateTokenArgs struct {
-	Ctx                  context.Context
-	Authcode             string
-	PKCECodeVerifier     pkce.Code
-	ExpectedIDTokenNonce nonce.Nonce
-	RedirectURI          string
-}
-
-type TestUpstreamOIDCIdentityProvider struct {
-	Name                                  string
-	ClientID                              string
-	AuthorizationURL                      url.URL
-	UsernameClaim                         string
-	GroupsClaim                           string
-	Scopes                                []string
-	ExchangeAuthcodeAndValidateTokensFunc func(
-		ctx context.Context,
-		authcode string,
-		pkceCodeVerifier pkce.Code,
-		expectedIDTokenNonce nonce.Nonce,
-	) (*oidctypes.Token, error)
-
-	exchangeAuthcodeAndValidateTokensCallCount int
-	exchangeAuthcodeAndValidateTokensArgs      []*ExchangeAuthcodeAndValidateTokenArgs
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetName() string {
-	return u.Name
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetClientID() string {
-	return u.ClientID
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL {
-	return &u.AuthorizationURL
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetScopes() []string {
-	return u.Scopes
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetUsernameClaim() string {
-	return u.UsernameClaim
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) GetGroupsClaim() string {
-	return u.GroupsClaim
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokens(
-	ctx context.Context,
-	authcode string,
-	pkceCodeVerifier pkce.Code,
-	expectedIDTokenNonce nonce.Nonce,
-	redirectURI string,
-) (*oidctypes.Token, error) {
-	if u.exchangeAuthcodeAndValidateTokensArgs == nil {
-		u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0)
-	}
-	u.exchangeAuthcodeAndValidateTokensCallCount++
-	u.exchangeAuthcodeAndValidateTokensArgs = append(u.exchangeAuthcodeAndValidateTokensArgs, &ExchangeAuthcodeAndValidateTokenArgs{
-		Ctx:                  ctx,
-		Authcode:             authcode,
-		PKCECodeVerifier:     pkceCodeVerifier,
-		ExpectedIDTokenNonce: expectedIDTokenNonce,
-		RedirectURI:          redirectURI,
-	})
-	return u.ExchangeAuthcodeAndValidateTokensFunc(ctx, authcode, pkceCodeVerifier, expectedIDTokenNonce)
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensCallCount() int {
-	return u.exchangeAuthcodeAndValidateTokensCallCount
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensArgs(call int) *ExchangeAuthcodeAndValidateTokenArgs {
-	if u.exchangeAuthcodeAndValidateTokensArgs == nil {
-		u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0)
-	}
-	return u.exchangeAuthcodeAndValidateTokensArgs[call]
-}
-
-func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(_ context.Context, _ *oauth2.Token, _ nonce.Nonce) (*oidctypes.Token, error) {
-	panic("implement me")
-}
-
-type UpstreamIDPListerBuilder struct {
-	upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider
-}
-
-func (b *UpstreamIDPListerBuilder) WithOIDC(upstreamOIDCIdentityProviders ...*TestUpstreamOIDCIdentityProvider) *UpstreamIDPListerBuilder {
-	b.upstreamOIDCIdentityProviders = append(b.upstreamOIDCIdentityProviders, upstreamOIDCIdentityProviders...)
-	return b
-}
-
-func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider {
-	idpProvider := provider.NewDynamicUpstreamIDPProvider()
-	upstreams := make([]provider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders))
-	for i := range b.upstreamOIDCIdentityProviders {
-		upstreams[i] = provider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i])
-	}
-	idpProvider.SetOIDCIdentityProviders(upstreams)
-	return idpProvider
-}
-
-func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder {
-	return &UpstreamIDPListerBuilder{}
-}
-
-// Declare a separate type from the production code to ensure that the state param's contents was serialized
-// in the format that we expect, with the json keys that we expect, etc. This also ensure that the order of
-// the serialized fields is the same, which doesn't really matter expect that we can make simpler equality
-// assertions about the redirect URL in this test.
-type ExpectedUpstreamStateParamFormat struct {
-	P string `json:"p"`
-	U string `json:"u"`
-	N string `json:"n"`
-	C string `json:"c"`
-	K string `json:"k"`
-	V string `json:"v"`
-}
-
-type staticKeySet struct {
-	publicKey crypto.PublicKey
-}
-
-func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet {
-	return &staticKeySet{publicKey}
-}
-
-func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) {
-	jws, err := jose.ParseSigned(jwt)
-	if err != nil {
-		return nil, fmt.Errorf("oidc: malformed jwt: %w", err)
-	}
-	return jws.Verify(s.publicKey)
-}
-
-// VerifyECDSAIDToken verifies that the provided idToken was issued via the provided jwtSigningKey.
-// It also performs some light validation on the claims, i.e., it makes sure the provided idToken
-// has the provided  issuer and clientID.
-//
-// Further validation can be done via callers via the returned coreosoidc.IDToken.
-func VerifyECDSAIDToken(
-	t *testing.T,
-	issuer, clientID string,
-	jwtSigningKey *ecdsa.PrivateKey,
-	idToken string,
-) *coreosoidc.IDToken {
-	t.Helper()
-
-	keySet := newStaticKeySet(jwtSigningKey.Public())
-	verifyConfig := coreosoidc.Config{ClientID: clientID, SupportedSigningAlgs: []string{coreosoidc.ES256}}
-	verifier := coreosoidc.NewVerifier(issuer, keySet, &verifyConfig)
-	token, err := verifier.Verify(context.Background(), idToken)
-	require.NoError(t, err)
-
-	return token
-}

internal/oidc/provider/dynamic_upstream_idp_provider.go

@@ -53,6 +53,11 @@ type UpstreamLDAPIdentityProviderI interface {
 	// A name for this upstream provider.
 	GetName() string
 
+	// Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234".
+	// This URL is not used for connecting to the provider, but rather is used for creating a globally unique user
+	// identifier by being combined with the user's UID, since user UIDs are only unique within one provider.
+	GetURL() string
+
 	// A method for performing user authentication against the upstream LDAP provider.
 	ldap.UserAuthenticator
 }

internal/oidc/provider/manager/manager_test.go

@@ -25,9 +25,9 @@ import (
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/discovery"
 	"go.pinniped.dev/internal/oidc/jwks"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/oidc/provider"
 	"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"

internal/oidc/token/token_handler_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package token
@@ -40,11 +40,12 @@ import (
 	"go.pinniped.dev/internal/fositestorage/openidconnect"
 	storagepkce "go.pinniped.dev/internal/fositestorage/pkce"
 	"go.pinniped.dev/internal/fositestorage/refreshtoken"
+	"go.pinniped.dev/internal/fositestoragei"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
-	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
 )
 
 const (
@@ -214,25 +215,13 @@ type authcodeExchangeInputs struct {
 	modifyTokenRequest func(tokenRequest *http.Request, authCode string)
 	modifyStorage      func(
 		t *testing.T,
-		s interface {
+		s fositestoragei.AllFositeStorage,
-			oauth2.TokenRevocationStorage
-			oauth2.CoreStorage
-			openid.OpenIDConnectRequestStorage
-			pkce.PKCERequestStorage
-			fosite.ClientManager
-		},
 		authCode string,
 	)
 	makeOathHelper func(
 		t *testing.T,
 		authRequest *http.Request,
-		store interface {
+		store fositestoragei.AllFositeStorage,
-			oauth2.TokenRevocationStorage
-			oauth2.CoreStorage
-			openid.OpenIDConnectRequestStorage
-			pkce.PKCERequestStorage
-			fosite.ClientManager
-		},
 	) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey)
 
 	want tokenEndpointResponseExpectedValues
@@ -1315,13 +1304,7 @@ func getFositeDataSignature(t *testing.T, data string) string {
 func makeHappyOauthHelper(
 	t *testing.T,
 	authRequest *http.Request,
-	store interface {
+	store fositestoragei.AllFositeStorage,
-		oauth2.TokenRevocationStorage
-		oauth2.CoreStorage
-		openid.OpenIDConnectRequestStorage
-		pkce.PKCERequestStorage
-		fosite.ClientManager
-	},
 ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) {
 	t.Helper()
 
@@ -1347,13 +1330,7 @@ func (s *singleUseJWKProvider) GetJWKS(issuerName string) (jwks *jose.JSONWebKey
 func makeOauthHelperWithJWTKeyThatWorksOnlyOnce(
 	t *testing.T,
 	authRequest *http.Request,
-	store interface {
+	store fositestoragei.AllFositeStorage,
-		oauth2.TokenRevocationStorage
-		oauth2.CoreStorage
-		openid.OpenIDConnectRequestStorage
-		pkce.PKCERequestStorage
-		fosite.ClientManager
-	},
 ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) {
 	t.Helper()
 
@@ -1366,13 +1343,7 @@ func makeOauthHelperWithJWTKeyThatWorksOnlyOnce(
 func makeOauthHelperWithNilPrivateJWTSigningKey(
 	t *testing.T,
 	authRequest *http.Request,
-	store interface {
+	store fositestoragei.AllFositeStorage,
-		oauth2.TokenRevocationStorage
-		oauth2.CoreStorage
-		openid.OpenIDConnectRequestStorage
-		pkce.PKCERequestStorage
-		fosite.ClientManager
-	},
 ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) {
 	t.Helper()
 

internal/testutil/oidctestutil/oidctestutil.go

@@ -0,0 +1,469 @@
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package oidctestutil
+
+import (
+	"context"
+	"crypto"
+	"crypto/ecdsa"
+	"fmt"
+	"net/url"
+	"regexp"
+	"strings"
+	"testing"
+	"time"
+
+	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/handler/openid"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2"
+	"gopkg.in/square/go-jose.v2"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/client-go/kubernetes/fake"
+	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
+
+	"go.pinniped.dev/internal/crud"
+	"go.pinniped.dev/internal/fositestorage/authorizationcode"
+	"go.pinniped.dev/internal/fositestorage/openidconnect"
+	pkce2 "go.pinniped.dev/internal/fositestorage/pkce"
+	"go.pinniped.dev/internal/fositestoragei"
+	"go.pinniped.dev/internal/oidc/provider"
+	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/pkg/oidcclient/nonce"
+	"go.pinniped.dev/pkg/oidcclient/oidctypes"
+	"go.pinniped.dev/pkg/oidcclient/pkce"
+)
+
+// Test helpers for the OIDC package.
+
+// ExchangeAuthcodeAndValidateTokenArgs is a POGO (plain old go object?) used to spy on calls to
+// TestUpstreamOIDCIdentityProvider.ExchangeAuthcodeAndValidateTokensFunc().
+type ExchangeAuthcodeAndValidateTokenArgs struct {
+	Ctx                  context.Context
+	Authcode             string
+	PKCECodeVerifier     pkce.Code
+	ExpectedIDTokenNonce nonce.Nonce
+	RedirectURI          string
+}
+
+type TestUpstreamLDAPIdentityProvider struct {
+	Name             string
+	URL              string
+	AuthenticateFunc func(ctx context.Context, username, password string) (*authenticator.Response, bool, error)
+}
+
+func (u *TestUpstreamLDAPIdentityProvider) GetName() string {
+	return u.Name
+}
+
+func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string) (*authenticator.Response, bool, error) {
+	return u.AuthenticateFunc(ctx, username, password)
+}
+
+func (u *TestUpstreamLDAPIdentityProvider) GetURL() string {
+	return u.URL
+}
+
+type TestUpstreamOIDCIdentityProvider struct {
+	Name                                  string
+	ClientID                              string
+	AuthorizationURL                      url.URL
+	UsernameClaim                         string
+	GroupsClaim                           string
+	Scopes                                []string
+	ExchangeAuthcodeAndValidateTokensFunc func(
+		ctx context.Context,
+		authcode string,
+		pkceCodeVerifier pkce.Code,
+		expectedIDTokenNonce nonce.Nonce,
+	) (*oidctypes.Token, error)
+
+	exchangeAuthcodeAndValidateTokensCallCount int
+	exchangeAuthcodeAndValidateTokensArgs      []*ExchangeAuthcodeAndValidateTokenArgs
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetName() string {
+	return u.Name
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetClientID() string {
+	return u.ClientID
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL {
+	return &u.AuthorizationURL
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetScopes() []string {
+	return u.Scopes
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetUsernameClaim() string {
+	return u.UsernameClaim
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) GetGroupsClaim() string {
+	return u.GroupsClaim
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokens(
+	ctx context.Context,
+	authcode string,
+	pkceCodeVerifier pkce.Code,
+	expectedIDTokenNonce nonce.Nonce,
+	redirectURI string,
+) (*oidctypes.Token, error) {
+	if u.exchangeAuthcodeAndValidateTokensArgs == nil {
+		u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0)
+	}
+	u.exchangeAuthcodeAndValidateTokensCallCount++
+	u.exchangeAuthcodeAndValidateTokensArgs = append(u.exchangeAuthcodeAndValidateTokensArgs, &ExchangeAuthcodeAndValidateTokenArgs{
+		Ctx:                  ctx,
+		Authcode:             authcode,
+		PKCECodeVerifier:     pkceCodeVerifier,
+		ExpectedIDTokenNonce: expectedIDTokenNonce,
+		RedirectURI:          redirectURI,
+	})
+	return u.ExchangeAuthcodeAndValidateTokensFunc(ctx, authcode, pkceCodeVerifier, expectedIDTokenNonce)
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensCallCount() int {
+	return u.exchangeAuthcodeAndValidateTokensCallCount
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensArgs(call int) *ExchangeAuthcodeAndValidateTokenArgs {
+	if u.exchangeAuthcodeAndValidateTokensArgs == nil {
+		u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0)
+	}
+	return u.exchangeAuthcodeAndValidateTokensArgs[call]
+}
+
+func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(_ context.Context, _ *oauth2.Token, _ nonce.Nonce) (*oidctypes.Token, error) {
+	panic("implement me")
+}
+
+type UpstreamIDPListerBuilder struct {
+	upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider
+	upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider
+}
+
+func (b *UpstreamIDPListerBuilder) WithOIDC(upstreamOIDCIdentityProviders ...*TestUpstreamOIDCIdentityProvider) *UpstreamIDPListerBuilder {
+	b.upstreamOIDCIdentityProviders = append(b.upstreamOIDCIdentityProviders, upstreamOIDCIdentityProviders...)
+	return b
+}
+
+func (b *UpstreamIDPListerBuilder) WithLDAP(upstreamLDAPIdentityProviders ...*TestUpstreamLDAPIdentityProvider) *UpstreamIDPListerBuilder {
+	b.upstreamLDAPIdentityProviders = append(b.upstreamLDAPIdentityProviders, upstreamLDAPIdentityProviders...)
+	return b
+}
+
+func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider {
+	idpProvider := provider.NewDynamicUpstreamIDPProvider()
+
+	oidcUpstreams := make([]provider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders))
+	for i := range b.upstreamOIDCIdentityProviders {
+		oidcUpstreams[i] = provider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i])
+	}
+	idpProvider.SetOIDCIdentityProviders(oidcUpstreams)
+
+	ldapUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, len(b.upstreamLDAPIdentityProviders))
+	for i := range b.upstreamLDAPIdentityProviders {
+		ldapUpstreams[i] = provider.UpstreamLDAPIdentityProviderI(b.upstreamLDAPIdentityProviders[i])
+	}
+	idpProvider.SetLDAPIdentityProviders(ldapUpstreams)
+
+	return idpProvider
+}
+
+func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder {
+	return &UpstreamIDPListerBuilder{}
+}
+
+// Declare a separate type from the production code to ensure that the state param's contents was serialized
+// in the format that we expect, with the json keys that we expect, etc. This also ensure that the order of
+// the serialized fields is the same, which doesn't really matter expect that we can make simpler equality
+// assertions about the redirect URL in this test.
+type ExpectedUpstreamStateParamFormat struct {
+	P string `json:"p"`
+	U string `json:"u"`
+	N string `json:"n"`
+	C string `json:"c"`
+	K string `json:"k"`
+	V string `json:"v"`
+}
+
+type staticKeySet struct {
+	publicKey crypto.PublicKey
+}
+
+func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet {
+	return &staticKeySet{publicKey}
+}
+
+func (s *staticKeySet) VerifySignature(_ context.Context, jwt string) ([]byte, error) {
+	jws, err := jose.ParseSigned(jwt)
+	if err != nil {
+		return nil, fmt.Errorf("oidc: malformed jwt: %w", err)
+	}
+	return jws.Verify(s.publicKey)
+}
+
+// VerifyECDSAIDToken verifies that the provided idToken was issued via the provided jwtSigningKey.
+// It also performs some light validation on the claims, i.e., it makes sure the provided idToken
+// has the provided  issuer and clientID.
+//
+// Further validation can be done via callers via the returned coreosoidc.IDToken.
+func VerifyECDSAIDToken(
+	t *testing.T,
+	issuer, clientID string,
+	jwtSigningKey *ecdsa.PrivateKey,
+	idToken string,
+) *coreosoidc.IDToken {
+	t.Helper()
+
+	keySet := newStaticKeySet(jwtSigningKey.Public())
+	verifyConfig := coreosoidc.Config{ClientID: clientID, SupportedSigningAlgs: []string{coreosoidc.ES256}}
+	verifier := coreosoidc.NewVerifier(issuer, keySet, &verifyConfig)
+	token, err := verifier.Verify(context.Background(), idToken)
+	require.NoError(t, err)
+
+	return token
+}
+
+func RequireAuthcodeRedirectLocation(
+	t *testing.T,
+	actualRedirectLocation string,
+	wantRedirectLocationRegexp string,
+	kubeClient *fake.Clientset,
+	secretsClient v1.SecretInterface,
+	oauthStore fositestoragei.AllFositeStorage,
+	wantDownstreamGrantedScopes []string,
+	wantDownstreamIDTokenSubject string,
+	wantDownstreamIDTokenUsername string,
+	wantDownstreamIDTokenGroups []string,
+	wantDownstreamRequestedScopes []string,
+	wantDownstreamPKCEChallenge string,
+	wantDownstreamPKCEChallengeMethod string,
+	wantDownstreamNonce string,
+	wantDownstreamClientID string,
+	wantDownstreamRedirectURI string,
+) {
+	t.Helper()
+
+	// Assert that Location header matches regular expression.
+	regex := regexp.MustCompile(wantRedirectLocationRegexp)
+	submatches := regex.FindStringSubmatch(actualRedirectLocation)
+	require.Lenf(t, submatches, 2, "no regexp match in actualRedirectLocation: %q", actualRedirectLocation)
+	capturedAuthCode := submatches[1]
+
+	// fosite authcodes are in the format `data.signature`, so grab the signature part, which is the lookup key in the storage interface
+	authcodeDataAndSignature := strings.Split(capturedAuthCode, ".")
+	require.Len(t, authcodeDataAndSignature, 2)
+
+	// Several Secrets should have been created
+	expectedNumberOfCreatedSecrets := 2
+	if includesOpenIDScope(wantDownstreamGrantedScopes) {
+		expectedNumberOfCreatedSecrets++
+	}
+	require.Len(t, kubeClient.Actions(), expectedNumberOfCreatedSecrets)
+
+	// One authcode should have been stored.
+	testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+
+	storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage(
+		t,
+		oauthStore,
+		authcodeDataAndSignature[1], // Authcode store key is authcode signature
+		wantDownstreamGrantedScopes,
+		wantDownstreamIDTokenSubject,
+		wantDownstreamIDTokenUsername,
+		wantDownstreamIDTokenGroups,
+		wantDownstreamRequestedScopes,
+		wantDownstreamClientID,
+		wantDownstreamRedirectURI,
+	)
+
+	// One PKCE should have been stored.
+	testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: pkce2.TypeLabelValue}, 1)
+
+	validatePKCEStorage(
+		t,
+		oauthStore,
+		authcodeDataAndSignature[1], // PKCE store key is authcode signature
+		storedRequestFromAuthcode,
+		storedSessionFromAuthcode,
+		wantDownstreamPKCEChallenge,
+		wantDownstreamPKCEChallengeMethod,
+	)
+
+	// One IDSession should have been stored, if the downstream actually requested the "openid" scope
+	if includesOpenIDScope(wantDownstreamGrantedScopes) {
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+
+		validateIDSessionStorage(
+			t,
+			oauthStore,
+			capturedAuthCode, // IDSession store key is full authcode
+			storedRequestFromAuthcode,
+			storedSessionFromAuthcode,
+			wantDownstreamNonce,
+		)
+	}
+}
+
+func includesOpenIDScope(scopes []string) bool {
+	for _, scope := range scopes {
+		if scope == "openid" {
+			return true
+		}
+	}
+	return false
+}
+
+func validateAuthcodeStorage(
+	t *testing.T,
+	oauthStore fositestoragei.AllFositeStorage,
+	storeKey string,
+	wantDownstreamGrantedScopes []string,
+	wantDownstreamIDTokenSubject string,
+	wantDownstreamIDTokenUsername string,
+	wantDownstreamIDTokenGroups []string,
+	wantDownstreamRequestedScopes []string,
+	wantDownstreamClientID string,
+	wantDownstreamRedirectURI string,
+) (*fosite.Request, *openid.DefaultSession) {
+	t.Helper()
+
+	const (
+		authCodeExpirationSeconds = 10 * 60 // Currently, we set our auth code expiration to 10 minutes
+		timeComparisonFudgeFactor = time.Second * 15
+	)
+
+	// Get the authcode session back from storage so we can require that it was stored correctly.
+	storedAuthorizeRequestFromAuthcode, err := oauthStore.GetAuthorizeCodeSession(context.Background(), storeKey, nil)
+	require.NoError(t, err)
+
+	// Check that storage returned the expected concrete data types.
+	storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode)
+
+	// Check which scopes were granted.
+	require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes())
+
+	// Check all the other fields of the stored request.
+	require.NotEmpty(t, storedRequestFromAuthcode.ID)
+	require.Equal(t, wantDownstreamClientID, storedRequestFromAuthcode.Client.GetID())
+	require.ElementsMatch(t, wantDownstreamRequestedScopes, storedRequestFromAuthcode.RequestedScope)
+	require.Nil(t, storedRequestFromAuthcode.RequestedAudience)
+	require.Empty(t, storedRequestFromAuthcode.GrantedAudience)
+	require.Equal(t, url.Values{"redirect_uri": []string{wantDownstreamRedirectURI}}, storedRequestFromAuthcode.Form)
+	testutil.RequireTimeInDelta(t, time.Now(), storedRequestFromAuthcode.RequestedAt, timeComparisonFudgeFactor)
+
+	// We're not using these fields yet, so confirm that we did not set them (for now).
+	require.Empty(t, storedSessionFromAuthcode.Subject)
+	require.Empty(t, storedSessionFromAuthcode.Username)
+	require.Empty(t, storedSessionFromAuthcode.Headers)
+
+	// The authcode that we are issuing should be good for the length of time that we declare in the fosite config.
+	testutil.RequireTimeInDelta(t, time.Now().Add(authCodeExpirationSeconds*time.Second), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor)
+	require.Len(t, storedSessionFromAuthcode.ExpiresAt, 1)
+
+	// Now confirm the ID token claims.
+	actualClaims := storedSessionFromAuthcode.Claims
+
+	// Check the user's identity, which are put into the downstream ID token's subject, username and groups claims.
+	require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject)
+	require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"])
+	require.Len(t, actualClaims.Extra, 2)
+	actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
+	require.NotNil(t, actualDownstreamIDTokenGroups)
+	require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups)
+
+	// Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time).
+	testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor)
+	testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.AuthTime, timeComparisonFudgeFactor)
+	requestedAtZone, _ := actualClaims.RequestedAt.Zone()
+	require.Equal(t, "UTC", requestedAtZone)
+	authTimeZone, _ := actualClaims.AuthTime.Zone()
+	require.Equal(t, "UTC", authTimeZone)
+
+	// Fosite will set these fields for us in the token endpoint based on the store session
+	// information. Therefore, we assert that they are empty because we want the library to do the
+	// lifting for us.
+	require.Empty(t, actualClaims.Issuer)
+	require.Nil(t, actualClaims.Audience)
+	require.Empty(t, actualClaims.Nonce)
+	require.Zero(t, actualClaims.ExpiresAt)
+	require.Zero(t, actualClaims.IssuedAt)
+
+	// These are not needed yet.
+	require.Empty(t, actualClaims.JTI)
+	require.Empty(t, actualClaims.CodeHash)
+	require.Empty(t, actualClaims.AccessTokenHash)
+	require.Empty(t, actualClaims.AuthenticationContextClassReference)
+	require.Empty(t, actualClaims.AuthenticationMethodsReference)
+
+	return storedRequestFromAuthcode, storedSessionFromAuthcode
+}
+
+func validatePKCEStorage(
+	t *testing.T,
+	oauthStore fositestoragei.AllFositeStorage,
+	storeKey string,
+	storedRequestFromAuthcode *fosite.Request,
+	storedSessionFromAuthcode *openid.DefaultSession,
+	wantDownstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod string,
+) {
+	t.Helper()
+
+	storedAuthorizeRequestFromPKCE, err := oauthStore.GetPKCERequestSession(context.Background(), storeKey, nil)
+	require.NoError(t, err)
+
+	// Check that storage returned the expected concrete data types.
+	storedRequestFromPKCE, storedSessionFromPKCE := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromPKCE)
+
+	// The stored PKCE request should be the same as the stored authcode request.
+	require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromPKCE.ID)
+	require.Equal(t, storedSessionFromAuthcode, storedSessionFromPKCE)
+
+	// The stored PKCE request should also contain the PKCE challenge that the downstream sent us.
+	require.Equal(t, wantDownstreamPKCEChallenge, storedRequestFromPKCE.Form.Get("code_challenge"))
+	require.Equal(t, wantDownstreamPKCEChallengeMethod, storedRequestFromPKCE.Form.Get("code_challenge_method"))
+}
+
+func validateIDSessionStorage(
+	t *testing.T,
+	oauthStore fositestoragei.AllFositeStorage,
+	storeKey string,
+	storedRequestFromAuthcode *fosite.Request,
+	storedSessionFromAuthcode *openid.DefaultSession,
+	wantDownstreamNonce string,
+) {
+	t.Helper()
+
+	storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil)
+	require.NoError(t, err)
+
+	// Check that storage returned the expected concrete data types.
+	storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession)
+
+	// The stored IDSession request should be the same as the stored authcode request.
+	require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID)
+	require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession)
+
+	// The stored IDSession request should also contain the nonce that the downstream sent us.
+	require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce"))
+}
+
+func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requester) (*fosite.Request, *openid.DefaultSession) {
+	t.Helper()
+
+	storedRequest, ok := storedAuthorizeRequest.(*fosite.Request)
+	require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest, &fosite.Request{})
+	storedSession, ok := storedAuthorizeRequest.GetSession().(*openid.DefaultSession)
+	require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest.GetSession(), &openid.DefaultSession{})
+
+	return storedRequest, storedSession
+}