Merge pull request #1163 from vmware-tanzu/ldap-login-ui

Support a browser-based login flow for LDAP and Active Directory providers

cmd/pinniped/cmd/kubeconfig.go

@@ -233,7 +233,7 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 	// When all the upstream IDP flags are set by the user, then skip discovery and don't validate their input. Maybe they know something
 	// that we can't know, like the name of an IDP that they are going to define in the future.
 	if len(flags.oidc.issuer) > 0 && (flags.oidc.upstreamIDPType == "" || flags.oidc.upstreamIDPName == "" || flags.oidc.upstreamIDPFlow == "") {
-		if err := discoverSupervisorUpstreamIDP(ctx, &flags); err != nil {
+		if err := discoverSupervisorUpstreamIDP(ctx, &flags, deps.log); err != nil {
 			return err
 		}
 	}
@@ -726,7 +726,7 @@ func hasPendingStrategy(credentialIssuer *configv1alpha1.CredentialIssuer) bool
 	return false
 }
 
-func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigParams) error {
+func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigParams, log logr.Logger) error {
 	httpClient, err := newDiscoveryHTTPClient(flags.oidc.caBundle)
 	if err != nil {
 		return err
@@ -758,7 +758,7 @@ func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigPara
 		return err
 	}
 
-	selectedIDPFlow, err := selectUpstreamIDPFlow(discoveredIDPFlows, selectedIDPName, selectedIDPType, flags.oidc.upstreamIDPFlow)
+	selectedIDPFlow, err := selectUpstreamIDPFlow(discoveredIDPFlows, selectedIDPName, selectedIDPType, flags.oidc.upstreamIDPFlow, log)
 	if err != nil {
 		return err
 	}
@@ -898,7 +898,7 @@ func selectUpstreamIDPNameAndType(pinnipedIDPs []idpdiscoveryv1alpha1.PinnipedID
 	}
 }
 
-func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, selectedIDPName string, selectedIDPType idpdiscoveryv1alpha1.IDPType, specifiedFlow string) (idpdiscoveryv1alpha1.IDPFlow, error) {
+func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, selectedIDPName string, selectedIDPType idpdiscoveryv1alpha1.IDPType, specifiedFlow string, log logr.Logger) (idpdiscoveryv1alpha1.IDPFlow, error) {
 	switch {
 	case len(discoveredIDPFlows) == 0:
 		// No flows listed by discovery means that we are talking to an old Supervisor from before this feature existed.
@@ -922,10 +922,9 @@ func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, se
 		return discoveredIDPFlows[0], nil
 	default:
 		// The user did not specify a flow, and more than one was found.
-		return "", fmt.Errorf(
+		log.Info("multiple client flows found, selecting first value as default",
-			"multiple client flows for Supervisor upstream identity provider %q of type %q were found, "+
+			"idpName", selectedIDPName, "idpType", selectedIDPType,
-				"so the --upstream-identity-provider-flow flag must be specified. "+
+			"selectedFlow", discoveredIDPFlows[0].String(), "availableFlows", discoveredIDPFlows)
-				"Found these flows: %v",
+		return discoveredIDPFlows[0], nil
-			selectedIDPName, selectedIDPType, discoveredIDPFlows)
 	}
 }

cmd/pinniped/cmd/kubeconfig_test.go

@@ -1261,13 +1261,52 @@ func TestGetKubeconfig(t *testing.T) {
 			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
 			idpsDiscoveryResponse: here.Docf(`{
 				"pinniped_identity_providers": [
-					{"name": "some-oidc-idp", "type": "oidc", "flows": ["flow1", "flow2"]}
+					{"name": "some-ldap-idp", "type": "ldap", "flows": ["cli_password", "flow2"]}
 				]
 			}`),
-			wantError: true,
+			wantStdout: func(issuerCABundle string, issuerURL string) string {
-			wantStderr: func(issuerCABundle string, issuerURL string) string {
+				return here.Docf(`
-				return `Error: multiple client flows for Supervisor upstream identity provider "some-oidc-idp" of type "oidc" were found, so the --upstream-identity-provider-flow flag must be specified.` +
+					apiVersion: v1
-					` Found these flows: [flow1 flow2]` + "\n"
+					clusters:
+					- cluster:
+						certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==
+						server: https://fake-server-url-value
+					  name: kind-cluster-pinniped
+					contexts:
+					- context:
+						cluster: kind-cluster-pinniped
+						user: kind-user-pinniped
+					  name: kind-context-pinniped
+					current-context: kind-context-pinniped
+					kind: Config
+					preferences: {}
+					users:
+					- name: kind-user-pinniped
+					  user:
+						exec:
+						  apiVersion: client.authentication.k8s.io/v1beta1
+						  args:
+						  - login
+						  - oidc
+						  - --issuer=%s
+						  - --client-id=pinniped-cli
+						  - --scopes=offline_access,openid,pinniped:request-audience
+						  - --ca-bundle-data=%s
+						  - --upstream-identity-provider-name=some-ldap-idp
+						  - --upstream-identity-provider-type=ldap
+						  - --upstream-identity-provider-flow=cli_password
+						  command: '.../path/to/pinniped'
+						  env: []
+						  installHint: The pinniped CLI does not appear to be installed.  See https://get.pinniped.dev/cli
+             for more details
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
+			wantLogs: func(_ string, _ string) []string {
+				return []string{`"level"=0 "msg"="multiple client flows found, selecting first value as default"  ` +
+					`"availableFlows"=["cli_password","flow2"] "idpName"="some-ldap-idp" "idpType"="ldap" "selectedFlow"="cli_password"`}
 			},
 		},
 		{

cmd/pinniped/cmd/login_oidc.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package cmd
@@ -271,11 +271,11 @@ func flowOptions(requestedIDPType idpdiscoveryv1alpha1.IDPType, requestedFlow id
 		case idpdiscoveryv1alpha1.IDPFlowCLIPassword, "":
 			return useCLIFlow, nil
 		case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode:
-			fallthrough // not supported for LDAP providers, so fallthrough to error case
+			return nil, nil
 		default:
 			return nil, fmt.Errorf(
 				"--upstream-identity-provider-flow value not recognized for identity provider type %q: %s (supported values: %s)",
-				requestedIDPType, requestedFlow, []string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String()})
+				requestedIDPType, requestedFlow, strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String(), idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String()}, ", "))
 		}
 	default:
 		// Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236

cmd/pinniped/cmd/login_oidc_test.go

@@ -235,18 +235,30 @@ func TestLoginOIDCCommand(t *testing.T) {
 			wantOptionsCount: 5,
 			wantStdout:       `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n",
 		},
+		{
+			name: "ldap upstream type with browser_authcode flow is allowed",
+			args: []string{
+				"--issuer", "test-issuer",
+				"--client-id", "test-client-id",
+				"--upstream-identity-provider-type", "ldap",
+				"--upstream-identity-provider-flow", "browser_authcode",
+				"--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution
+			},
+			wantOptionsCount: 4,
+			wantStdout:       `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n",
+		},
 		{
 			name: "ldap upstream type with unsupported flow is an error",
 			args: []string{
 				"--issuer", "test-issuer",
 				"--client-id", "test-client-id",
 				"--upstream-identity-provider-type", "ldap",
-				"--upstream-identity-provider-flow", "browser_authcode", // "browser_authcode" is only supported for OIDC upstreams
+				"--upstream-identity-provider-flow", "foo",
 				"--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution
 			},
 			wantError: true,
 			wantStderr: here.Doc(`
-				Error: --upstream-identity-provider-flow value not recognized for identity provider type "ldap": browser_authcode (supported values: [cli_password])
+				Error: --upstream-identity-provider-flow value not recognized for identity provider type "ldap": foo (supported values: cli_password, browser_authcode)
 			`),
 		},
 		{
@@ -261,18 +273,30 @@ func TestLoginOIDCCommand(t *testing.T) {
 			wantOptionsCount: 5,
 			wantStdout:       `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n",
 		},
+		{
+			name: "active directory upstream type with browser_authcode is allowed",
+			args: []string{
+				"--issuer", "test-issuer",
+				"--client-id", "test-client-id",
+				"--upstream-identity-provider-type", "activedirectory",
+				"--upstream-identity-provider-flow", "browser_authcode",
+				"--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution
+			},
+			wantOptionsCount: 4,
+			wantStdout:       `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n",
+		},
 		{
 			name: "active directory upstream type with unsupported flow is an error",
 			args: []string{
 				"--issuer", "test-issuer",
 				"--client-id", "test-client-id",
 				"--upstream-identity-provider-type", "activedirectory",
-				"--upstream-identity-provider-flow", "browser_authcode", // "browser_authcode" is only supported for OIDC upstreams
+				"--upstream-identity-provider-flow", "foo",
 				"--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution
 			},
 			wantError: true,
 			wantStderr: here.Doc(`
-				Error: --upstream-identity-provider-flow value not recognized for identity provider type "activedirectory": browser_authcode (supported values: [cli_password])
+				Error: --upstream-identity-provider-flow value not recognized for identity provider type "activedirectory": foo (supported values: cli_password, browser_authcode)
 			`),
 		},
 		{

hack/prepare-supervisor-on-kind.sh

@@ -25,11 +25,36 @@ set -euo pipefail
 ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 cd "$ROOT"
 
+function log_error() {
+  RED='\033[0;31m'
+  NC='\033[0m'
+  if [[ ${COLORTERM:-unknown} =~ ^(truecolor|24bit)$ ]]; then
+    echo -e "🙁${RED} Error: $* ${NC}"
+  else
+    echo ":( Error: $*"
+  fi
+}
+
 use_oidc_upstream=no
 use_ldap_upstream=no
 use_ad_upstream=no
+use_flow=""
 while (("$#")); do
   case "$1" in
+  --flow)
+    shift
+    # If there are no more command line arguments, or there is another command line argument but it starts with a dash, then error
+    if [[ "$#" == "0" || "$1" == -* ]]; then
+      log_error "--flow requires a flow name to be specified (e.g. cli_password or browser_authcode"
+      exit 1
+    fi
+    if [[ "$1" != "browser_authcode" && "$1" != "cli_password" ]]; then
+      log_error "--flow must be cli_password or browser_authcode"
+      exit 1
+    fi
+    use_flow=$1
+    shift
+    ;;
   --ldap)
     use_ldap_upstream=yes
     shift
@@ -56,7 +81,7 @@ while (("$#")); do
 done
 
 if [[ "$use_oidc_upstream" == "no" && "$use_ldap_upstream" == "no" && "$use_ad_upstream" == "no" ]]; then
-  echo "Error: Please use --oidc, --ldap, or --ad to specify which type of upstream identity provider(s) you would like"
+  log_error "Error: Please use --oidc, --ldap, or --ad to specify which type of upstream identity provider(s) you would like"
   exit 1
 fi
 
@@ -127,6 +152,7 @@ spec:
     certificateAuthorityData: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE"
   authorizationConfig:
     additionalScopes: [ ${PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES} ]
+    allowPasswordGrant: true
   claims:
     username: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME_CLAIM"
     groups: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_GROUPS_CLAIM"
@@ -196,7 +222,7 @@ EOF
 fi
 
 if [[ "$use_ad_upstream" == "yes" ]]; then
-  # Make an ActiveDirectoryIdentityProvider.
+  # Make an ActiveDirectoryIdentityProvider. Needs to be pointed to a real AD server by env vars.
   cat <<EOF | kubectl apply --namespace "$PINNIPED_TEST_SUPERVISOR_NAMESPACE" -f -
 apiVersion: idp.supervisor.pinniped.dev/v1alpha1
 kind: ActiveDirectoryIdentityProvider
@@ -256,7 +282,11 @@ while [[ -z "$(kubectl get credentialissuer pinniped-concierge-config -o=jsonpat
 done
 
 # Use the CLI to get the kubeconfig. Tell it that you don't want the browser to automatically open for logins.
-https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" ./pinniped get kubeconfig --oidc-skip-browser >kubeconfig
+flow_arg=""
+if [[ -n "$use_flow" ]]; then
+  flow_arg="--upstream-identity-provider-flow $use_flow"
+fi
+https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" ./pinniped get kubeconfig --oidc-skip-browser $flow_arg >kubeconfig
 
 # Clear the local CLI cache to ensure that the kubectl command below will need to perform a fresh login.
 rm -f "$HOME/.config/pinniped/sessions.yaml"
@@ -265,25 +295,27 @@ rm -f "$HOME/.config/pinniped/credentials.yaml"
 echo
 echo "Ready! 🚀"
 
-if [[ "$use_oidc_upstream" == "yes" ]]; then
+if [[ "$use_oidc_upstream" == "yes" || "$use_flow" == "browser_authcode" ]]; then
   echo
   echo "To be able to access the login URL shown below, start Chrome like this:"
   echo "    open -a \"Google Chrome\" --args --proxy-server=\"$PINNIPED_TEST_PROXY\""
-  echo "Then use these credentials at the Dex login page:"
+  echo "Note that Chrome must be fully quit before being started with --proxy-server."
+  echo "Then open the login URL shown below in that new Chrome window."
+  echo
+  echo "When prompted for username and password, use these values:"
+fi
+
+if [[ "$use_oidc_upstream" == "yes" ]]; then
   echo "    Username: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME"
   echo "    Password: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD"
 fi
 
 if [[ "$use_ldap_upstream" == "yes" ]]; then
-  echo
-  echo "When prompted for username and password by the CLI, use these values:"
   echo "    Username: $PINNIPED_TEST_LDAP_USER_CN"
   echo "    Password: $PINNIPED_TEST_LDAP_USER_PASSWORD"
 fi
 
 if [[ "$use_ad_upstream" == "yes" ]]; then
-  echo
-  echo "When prompted for username and password by the CLI, use these values:"
   echo "    Username: $PINNIPED_TEST_AD_USER_USER_PRINCIPAL_NAME"
   echo "    Password: $PINNIPED_TEST_AD_USER_PASSWORD"
 fi

internal/oidc/auth/auth_handler.go

@@ -7,23 +7,22 @@ package auth
 import (
 	"fmt"
 	"net/http"
+	"net/url"
 	"time"
 
 	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
-	"github.com/felixge/httpsnoop"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/ory/fosite/token/jwt"
-	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
 
 	supervisoroidc "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
-	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/downstreamsession"
+	"go.pinniped.dev/internal/oidc/login"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/psession"
@@ -55,6 +54,12 @@ func NewHandler(
 			return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method)
 		}
 
+		// Note that the client might have used supervisoroidc.AuthorizeUpstreamIDPNameParamName and
+		// supervisoroidc.AuthorizeUpstreamIDPTypeParamName query params to request a certain upstream IDP.
+		// The Pinniped CLI has been sending these params since v0.9.0.
+		// Currently, these are ignored because the Supervisor does not yet support logins when multiple IDPs
+		// are configured. However, these params should be honored in the future when choosing an upstream
+		// here, e.g. by calling supervisoroidc.FindUpstreamIDPByNameAndType() when the params are present.
 		oidcUpstream, ldapUpstream, idpType, err := chooseUpstreamIDP(idpLister)
 		if err != nil {
 			plog.WarningErr("authorize upstream config", err)
@@ -62,11 +67,12 @@ func NewHandler(
 		}
 
 		if idpType == psession.ProviderTypeOIDC {
-			if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 {
+			if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 ||
+				len(r.Header.Values(supervisoroidc.AuthorizePasswordHeaderName)) > 0 {
 				// The client set a username header, so they are trying to log in with a username/password.
 				return handleAuthRequestForOIDCUpstreamPasswordGrant(r, w, oauthHelperWithStorage, oidcUpstream)
 			}
-			return handleAuthRequestForOIDCUpstreamAuthcodeGrant(r, w,
+			return handleAuthRequestForOIDCUpstreamBrowserFlow(r, w,
 				oauthHelperWithoutStorage,
 				generateCSRF, generateNonce, generatePKCE,
 				oidcUpstream,
@@ -75,15 +81,34 @@ func NewHandler(
 				cookieCodec,
 			)
 		}
-		return handleAuthRequestForLDAPUpstream(r, w,
+
-			oauthHelperWithStorage,
+		// We know it's an AD/LDAP upstream.
+		if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 ||
+			len(r.Header.Values(supervisoroidc.AuthorizePasswordHeaderName)) > 0 {
+			// The client set a username header, so they are trying to log in with a username/password.
+			return handleAuthRequestForLDAPUpstreamCLIFlow(r, w,
+				oauthHelperWithStorage,
+				ldapUpstream,
+				idpType,
+			)
+		}
+		return handleAuthRequestForLDAPUpstreamBrowserFlow(
+			r,
+			w,
+			oauthHelperWithoutStorage,
+			generateCSRF,
+			generateNonce,
+			generatePKCE,
 			ldapUpstream,
 			idpType,
+			downstreamIssuer,
+			upstreamStateEncoder,
+			cookieCodec,
 		)
 	}))
 }
 
-func handleAuthRequestForLDAPUpstream(
+func handleAuthRequestForLDAPUpstreamCLIFlow(
 	r *http.Request,
 	w http.ResponseWriter,
 	oauthHelper fosite.OAuth2Provider,
@@ -106,36 +131,55 @@ func handleAuthRequestForLDAPUpstream(
 		return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication")
 	}
 	if !authenticated {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Username/password not accepted by LDAP provider."), true)
+		return nil
 	}
 
-	subject := downstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse)
+	subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse)
 	username = authenticateResponse.User.GetName()
 	groups := authenticateResponse.User.GetGroups()
-	dn := authenticateResponse.DN
+	customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse)
+	openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData)
+	oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true)
 
-	customSessionData := &psession.CustomSessionData{
+	return nil
-		ProviderUID:  ldapUpstream.GetResourceUID(),
+}
-		ProviderName: ldapUpstream.GetName(),
+
-		ProviderType: idpType,
+func handleAuthRequestForLDAPUpstreamBrowserFlow(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	generateCSRF func() (csrftoken.CSRFToken, error),
+	generateNonce func() (nonce.Nonce, error),
+	generatePKCE func() (pkce.Code, error),
+	ldapUpstream provider.UpstreamLDAPIdentityProviderI,
+	idpType psession.ProviderType,
+	downstreamIssuer string,
+	upstreamStateEncoder oidc.Encoder,
+	cookieCodec oidc.Codec,
+) error {
+	authRequestState, err := handleBrowserFlowAuthRequest(
+		r,
+		w,
+		oauthHelper,
+		generateCSRF,
+		generateNonce,
+		generatePKCE,
+		ldapUpstream.GetName(),
+		idpType,
+		cookieCodec,
+		upstreamStateEncoder,
+	)
+	if err != nil {
+		return err
+	}
+	if authRequestState == nil {
+		// There was an error but handleBrowserFlowAuthRequest() already took care of writing the response for it.
+		return nil
 	}
 
-	if idpType == psession.ProviderTypeLDAP {
+	return login.RedirectToLoginPage(r, w, downstreamIssuer, authRequestState.encodedStateParam, login.ShowNoError)
-		customSessionData.LDAP = &psession.LDAPSessionData{
-			UserDN:                 dn,
-			ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes,
-		}
-	}
-	if idpType == psession.ProviderTypeActiveDirectory {
-		customSessionData.ActiveDirectory = &psession.ActiveDirectorySessionData{
-			UserDN:                 dn,
-			ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes,
-		}
-	}
-
-	return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w,
-		oauthHelper, authorizeRequester, subject, username, groups, customSessionData)
 }
 
 func handleAuthRequestForOIDCUpstreamPasswordGrant(
@@ -156,9 +200,10 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
 
 	if !oidcUpstream.AllowsPasswordGrant() {
 		// Return a user-friendly error for this case which is entirely within our control.
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHint(
 				"Resource owner password credentials grant is not allowed for this upstream provider according to its configuration."), true)
+		return nil
 	}
 
 	token, err := oidcUpstream.PasswordCredentialsGrantAndValidateTokens(r.Context(), username, password)
@@ -170,29 +215,36 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
 		// However, the exact response is undefined in the sense that there is no such thing as a password grant in
 		// the OIDC spec, so we don't try too hard to read the upstream errors in this case. (E.g. Dex departs from the
 		// spec and returns something other than an "invalid_grant" error for bad resource owner credentials.)
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithDebug(err.Error()), true) // WithDebug hides the error from the client
+		return nil
 	}
 
 	subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims)
 	if err != nil {
 		// Return a user-friendly error for this case which is entirely within our control.
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true,
 		)
+		return nil
 	}
 
 	customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token)
 	if err != nil {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true,
 		)
+		return nil
 	}
 
-	return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups, customSessionData)
+	openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData)
+
+	oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true)
+
+	return nil
 }
 
-func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
+func handleAuthRequestForOIDCUpstreamBrowserFlow(
 	r *http.Request,
 	w http.ResponseWriter,
 	oauthHelper fosite.OAuth2Provider,
@@ -204,34 +256,24 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
 	upstreamStateEncoder oidc.Encoder,
 	cookieCodec oidc.Codec,
 ) error {
-	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false)
+	authRequestState, err := handleBrowserFlowAuthRequest(
-	if !created {
+		r,
-		return nil
+		w,
-	}
+		oauthHelper,
-
+		generateCSRF,
-	now := time.Now()
+		generateNonce,
-	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{
+		generatePKCE,
-		Fosite: &openid.DefaultSession{
+		oidcUpstream.GetName(),
-			Claims: &jwt.IDTokenClaims{
+		psession.ProviderTypeOIDC,
-				// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
+		cookieCodec,
-				Subject:     "none",
+		upstreamStateEncoder,
-				AuthTime:    now,
+	)
-				RequestedAt: now,
-			},
-		},
-	})
 	if err != nil {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, err, false)
-	}
-
-	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
-	if err != nil {
-		plog.Error("authorize generate error", err)
 		return err
 	}
-	csrfFromCookie := readCSRFCookie(r, cookieCodec)
+	if authRequestState == nil {
-	if csrfFromCookie != "" {
+		// There was an error but handleBrowserFlowAuthRequest() already took care of writing the response for it.
-		csrfValue = csrfFromCookie
+		return nil
 	}
 
 	upstreamOAuthConfig := oauth2.Config{
@@ -243,46 +285,19 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
 		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
-	}
-
 	authCodeOptions := []oauth2.AuthCodeOption{
-		nonceValue.Param(),
+		authRequestState.nonce.Param(),
-		pkceValue.Challenge(),
+		authRequestState.pkce.Challenge(),
-		pkceValue.Method(),
+		authRequestState.pkce.Method(),
-	}
-
-	promptParam := r.Form.Get(promptParamName)
-	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
 	}
 
 	for key, val := range oidcUpstream.GetAdditionalAuthcodeParams() {
 		authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(key, val))
 	}
 
-	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
-		}
-	}
-
 	http.Redirect(w, r,
 		upstreamOAuthConfig.AuthCodeURL(
-			encodedStateParamValue,
+			authRequestState.encodedStateParam,
 			authCodeOptions...,
 		),
 		http.StatusSeeOther, // match fosite and https://tools.ietf.org/id/draft-ietf-oauth-security-topics-18.html#section-4.11
@@ -291,78 +306,11 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
 	return nil
 }
 
-func writeAuthorizeError(w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester, err error, isBrowserless bool) error {
-	if plog.Enabled(plog.LevelTrace) {
-		// When trace level logging is enabled, include the stack trace in the log message.
-		keysAndValues := oidc.FositeErrorForLog(err)
-		errWithStack := errors.WithStack(err)
-		keysAndValues = append(keysAndValues, "errWithStack")
-		// klog always prints error values using %s, which does not include stack traces,
-		// so convert the error to a string which includes the stack trace here.
-		keysAndValues = append(keysAndValues, fmt.Sprintf("%+v", errWithStack))
-		plog.Trace("authorize response error", keysAndValues...)
-	} else {
-		plog.Info("authorize response error", oidc.FositeErrorForLog(err)...)
-	}
-	if isBrowserless {
-		w = rewriteStatusSeeOtherToStatusFoundForBrowserless(w)
-	}
-	// Return an error according to OIDC spec 3.1.2.6 (second paragraph).
-	oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
-	return nil
-}
-
-func makeDownstreamSessionAndReturnAuthcodeRedirect(
-	r *http.Request,
-	w http.ResponseWriter,
-	oauthHelper fosite.OAuth2Provider,
-	authorizeRequester fosite.AuthorizeRequester,
-	subject string,
-	username string,
-	groups []string,
-	customSessionData *psession.CustomSessionData,
-) error {
-	openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData)
-
-	authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
-	if err != nil {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, err, true)
-	}
-
-	w = rewriteStatusSeeOtherToStatusFoundForBrowserless(w)
-	oauthHelper.WriteAuthorizeResponse(w, authorizeRequester, authorizeResponder)
-
-	return nil
-}
-
-func rewriteStatusSeeOtherToStatusFoundForBrowserless(w http.ResponseWriter) http.ResponseWriter {
-	// rewrite http.StatusSeeOther to http.StatusFound for backwards compatibility with old pinniped CLIs.
-	// we can drop this in a few releases once we feel enough time has passed for users to update.
-	//
-	// WriteAuthorizeResponse/WriteAuthorizeError calls used to result in http.StatusFound until
-	// https://github.com/ory/fosite/pull/636 changed it to http.StatusSeeOther to address
-	// https://tools.ietf.org/id/draft-ietf-oauth-security-topics-18.html#section-4.11
-	// Safari has the bad behavior in the case of http.StatusFound and not just http.StatusTemporaryRedirect.
-	//
-	// in the browserless flows, the OAuth client is the pinniped CLI and it already has access to the user's
-	// password.  Thus there is no security issue with using http.StatusFound vs. http.StatusSeeOther.
-	return httpsnoop.Wrap(w, httpsnoop.Hooks{
-		WriteHeader: func(delegate httpsnoop.WriteHeaderFunc) httpsnoop.WriteHeaderFunc {
-			return func(code int) {
-				if code == http.StatusSeeOther {
-					code = http.StatusFound
-				}
-				delegate(code)
-			}
-		},
-	})
-}
-
 func requireNonEmptyUsernameAndPasswordHeaders(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) (string, string, bool) {
 	username := r.Header.Get(supervisoroidc.AuthorizeUsernameHeaderName)
 	password := r.Header.Get(supervisoroidc.AuthorizePasswordHeaderName)
 	if username == "" || password == "" {
-		_ = writeAuthorizeError(w, oauthHelper, authorizeRequester,
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Missing or blank username or password."), true)
 		return "", "", false
 	}
@@ -372,7 +320,7 @@ func requireNonEmptyUsernameAndPasswordHeaders(r *http.Request, w http.ResponseW
 func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, isBrowserless bool) (fosite.AuthorizeRequester, bool) {
 	authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r)
 	if err != nil {
-		_ = writeAuthorizeError(w, oauthHelper, authorizeRequester, err, isBrowserless)
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester, err, isBrowserless)
 		return nil, false
 	}
 
@@ -404,7 +352,8 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {
 	return csrfFromCookie
 }
 
-// Select either an OIDC, an LDAP or an AD IDP, or return an error.
+// chooseUpstreamIDP selects either an OIDC, an LDAP, or an AD IDP, or returns an error.
+// Note that AD and LDAP IDPs both return the same interface type, but different ProviderTypes values.
 func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, psession.ProviderType, error) {
 	oidcUpstreams := idpLister.GetOIDCIdentityProviders()
 	ldapUpstreams := idpLister.GetLDAPIdentityProviders()
@@ -440,6 +389,99 @@ func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider
 	}
 }
 
+type browserFlowAuthRequestState struct {
+	encodedStateParam string
+	pkce              pkce.Code
+	nonce             nonce.Nonce
+}
+
+// handleBrowserFlowAuthRequest performs the shared validations and setup between browser based
+// auth requests regardless of IDP type-- LDAP, Active Directory and OIDC.
+// It generates the state param, sets the CSRF cookie, and validates the prompt param.
+// It returns an error when it encounters an error without handling it, leaving it to
+// the caller to decide how to handle it.
+// It returns nil with no error when it encounters an error and also has already handled writing
+// the error response to the ResponseWriter, in which case the caller should not also try to
+// write the error response.
+func handleBrowserFlowAuthRequest(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	generateCSRF func() (csrftoken.CSRFToken, error),
+	generateNonce func() (nonce.Nonce, error),
+	generatePKCE func() (pkce.Code, error),
+	upstreamName string,
+	idpType psession.ProviderType,
+	cookieCodec oidc.Codec,
+	upstreamStateEncoder oidc.Encoder,
+) (*browserFlowAuthRequestState, error) {
+	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false)
+	if !created {
+		return nil, nil // already wrote the error response, don't return error
+	}
+
+	now := time.Now()
+	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{
+		Fosite: &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 {
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester, err, false)
+		return nil, nil // already wrote the error response, don't return error
+	}
+
+	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
+	if err != nil {
+		plog.Error("authorize generate error", err)
+		return nil, err
+	}
+	csrfFromCookie := readCSRFCookie(r, cookieCodec)
+	if csrfFromCookie != "" {
+		csrfValue = csrfFromCookie
+	}
+
+	encodedStateParamValue, err := upstreamStateParam(
+		authorizeRequester,
+		upstreamName,
+		string(idpType),
+		nonceValue,
+		csrfValue,
+		pkceValue,
+		upstreamStateEncoder,
+	)
+	if err != nil {
+		plog.Error("authorize upstream state param error", err)
+		return nil, err
+	}
+
+	promptParam := r.Form.Get(promptParamName)
+	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
+		return nil, nil // already wrote the error response, don't return error
+	}
+
+	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 nil, err
+		}
+	}
+
+	return &browserFlowAuthRequestState{
+		encodedStateParam: encodedStateParamValue,
+		pkce:              pkceValue,
+		nonce:             nonceValue,
+	}, nil
+}
+
 func generateValues(
 	generateCSRF func() (csrftoken.CSRFToken, error),
 	generateNonce func() (nonce.Nonce, error),
@@ -463,14 +505,21 @@ func generateValues(
 func upstreamStateParam(
 	authorizeRequester fosite.AuthorizeRequester,
 	upstreamName string,
+	upstreamType string,
 	nonceValue nonce.Nonce,
 	csrfValue csrftoken.CSRFToken,
 	pkceValue pkce.Code,
 	encoder oidc.Encoder,
 ) (string, error) {
 	stateParamData := oidc.UpstreamStateParamData{
-		AuthParams:    authorizeRequester.GetRequestForm().Encode(),
+		// The auth params might have included supervisoroidc.AuthorizeUpstreamIDPNameParamName and
+		// supervisoroidc.AuthorizeUpstreamIDPTypeParamName, but those can be ignored by other handlers
+		// that are reading from the encoded upstream state param being built here.
+		// The UpstreamName and UpstreamType struct fields can be used instead.
+		// Remove those params here to avoid potential confusion about which should be used later.
+		AuthParams:    removeCustomIDPParams(authorizeRequester.GetRequestForm()).Encode(),
 		UpstreamName:  upstreamName,
+		UpstreamType:  upstreamType,
 		Nonce:         nonceValue,
 		CSRFToken:     csrfValue,
 		PKCECode:      pkceValue,
@@ -483,6 +532,18 @@ func upstreamStateParam(
 	return encodedStateParamValue, nil
 }
 
+func removeCustomIDPParams(params url.Values) url.Values {
+	p := url.Values{}
+	// Copy all params.
+	for k, v := range params {
+		p[k] = v
+	}
+	// Remove the unnecessary params.
+	delete(p, supervisoroidc.AuthorizeUpstreamIDPNameParamName)
+	delete(p, supervisoroidc.AuthorizeUpstreamIDPTypeParamName)
+	return p
+}
+
 func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken, codec oidc.Encoder) error {
 	encodedCSRFValue, err := codec.Encode(oidc.CSRFCookieEncodingName, csrfValue)
 	if err != nil {
@@ -500,8 +561,3 @@ func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken
 
 	return nil
 }
-
-func downstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentityProviderI, authenticateResponse *authenticators.Response) string {
-	ldapURL := *ldapUpstream.GetURL()
-	return downstreamsession.DownstreamLDAPSubject(authenticateResponse.User.GetUID(), ldapURL)
-}

internal/oidc/auth/auth_handler_test.go

@@ -70,6 +70,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		downstreamClientID                     = "pinniped-cli"
 		upstreamLDAPURL                        = "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev"
 		htmlContentType                        = "text/html; charset=utf-8"
+		jsonContentType                        = "application/json; charset=utf-8"
+		formContentType                        = "application/x-www-form-urlencoded"
 	)
 
 	require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
@@ -409,23 +411,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		return pathWithQuery("/some/path", modifiedHappyGetRequestQueryMap(queryOverrides))
 	}
 
-	expectedUpstreamStateParam := func(queryOverrides map[string]string, csrfValueOverride, upstreamNameOverride string) string {
+	expectedUpstreamStateParam := func(queryOverrides map[string]string, csrfValueOverride, upstreamName, upstreamType string) string {
 		csrf := happyCSRF
 		if csrfValueOverride != "" {
 			csrf = csrfValueOverride
 		}
-		upstreamName := oidcUpstreamName
-		if upstreamNameOverride != "" {
-			upstreamName = upstreamNameOverride
-		}
 		encoded, err := happyStateEncoder.Encode("s",
 			oidctestutil.ExpectedUpstreamStateParamFormat{
 				P: encodeQuery(modifiedHappyGetRequestQueryMap(queryOverrides)),
 				U: upstreamName,
+				T: upstreamType,
 				N: happyNonce,
 				C: csrf,
 				K: happyPKCE,
-				V: "1",
+				V: "2",
 			},
 		)
 		require.NoError(t, err)
@@ -558,7 +557,41 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantStatus:                             http.StatusSeeOther,
 			wantContentType:                        htmlContentType,
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
+		{
+			name:                                   "LDAP upstream browser flow happy path using GET without a CSRF cookie",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   happyGetRequestPath,
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", ldapUpstreamName, "ldap")}),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
+		{
+			name:                                   "Active Directory upstream browser flow happy path using GET without a CSRF cookie",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   happyGetRequestPath,
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", activeDirectoryUpstreamName, "activedirectory")}),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -585,7 +618,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
 		},
 		{
-			name:                              "LDAP upstream happy path using GET",
+			name:                              "LDAP cli upstream happy path using GET",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			method:                            http.MethodGet,
 			path:                              happyGetRequestPath,
@@ -606,7 +639,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
 		},
 		{
-			name:                              "ActiveDirectory upstream happy path using GET",
+			name:                              "ActiveDirectory cli upstream happy path using GET",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
 			method:                            http.MethodGet,
 			path:                              happyGetRequestPath,
@@ -639,7 +672,41 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			csrfCookie:                             "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
 			wantStatus:                             http.StatusSeeOther,
 			wantContentType:                        htmlContentType,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), nil),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
+		{
+			name:                                   "LDAP upstream browser flow happy path using GET with a CSRF cookie",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   happyGetRequestPath,
+			csrfCookie:                             "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ldapUpstreamName, "ldap")}),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
+		{
+			name:                                   "Active Directory upstream browser flow happy path using GET with a CSRF cookie",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   happyGetRequestPath,
+			csrfCookie:                             "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, incomingCookieCSRFValue, activeDirectoryUpstreamName, "activedirectory")}),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -653,13 +720,51 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			cookieEncoder:                          happyCookieEncoder,
 			method:                                 http.MethodPost,
 			path:                                   "/some/path",
-			contentType:                            "application/x-www-form-urlencoded",
+			contentType:                            formContentType,
 			body:                                   encodeQuery(happyGetRequestQueryMap),
 			wantStatus:                             http.StatusSeeOther,
 			wantContentType:                        "",
 			wantBodyString:                         "",
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
+		{
+			name:                                   "LDAP upstream browser flow happy path using POST",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodPost,
+			path:                                   "/some/path",
+			contentType:                            formContentType,
+			body:                                   encodeQuery(happyGetRequestQueryMap),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        "",
+			wantBodyString:                         "",
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", ldapUpstreamName, "ldap")}),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
+		{
+			name:                                   "Active Directory upstream browser flow happy path using POST",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodPost,
+			path:                                   "/some/path",
+			contentType:                            formContentType,
+			body:                                   encodeQuery(happyGetRequestQueryMap),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        "",
+			wantBodyString:                         "",
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", activeDirectoryUpstreamName, "activedirectory")}),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
 		{
@@ -667,7 +772,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
 			method:                            http.MethodPost,
 			path:                              "/some/path",
-			contentType:                       "application/x-www-form-urlencoded",
+			contentType:                       formContentType,
 			body:                              encodeQuery(happyGetRequestQueryMap),
 			customUsernameHeader:              pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader:              pointer.StringPtr(oidcUpstreamPassword),
@@ -687,11 +792,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
 		},
 		{
-			name:                              "LDAP upstream happy path using POST",
+			name:                              "LDAP cli upstream happy path using POST",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			method:                            http.MethodPost,
 			path:                              "/some/path",
-			contentType:                       "application/x-www-form-urlencoded",
+			contentType:                       formContentType,
 			body:                              encodeQuery(happyGetRequestQueryMap),
 			customUsernameHeader:              pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:              pointer.StringPtr(happyLDAPPassword),
@@ -710,11 +815,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
 		},
 		{
-			name:                              "Active Directory upstream happy path using POST",
+			name:                              "Active Directory cli upstream happy path using POST",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
 			method:                            http.MethodPost,
 			path:                              "/some/path",
-			contentType:                       "application/x-www-form-urlencoded",
+			contentType:                       formContentType,
 			body:                              encodeQuery(happyGetRequestQueryMap),
 			customUsernameHeader:              pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:              pointer.StringPtr(happyLDAPPassword),
@@ -742,13 +847,29 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			cookieEncoder:                          happyCookieEncoder,
 			method:                                 http.MethodGet,
 			path:                                   modifiedHappyGetRequestPath(map[string]string{"prompt": "login"}),
-			contentType:                            "application/x-www-form-urlencoded",
-			body:                                   encodeQuery(happyGetRequestQueryMap),
 			wantStatus:                             http.StatusSeeOther,
 			wantContentType:                        htmlContentType,
 			wantBodyStringWithLocationInHref:       true,
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), nil),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
+		{
+			name:                                   "OIDC upstream browser flow happy path with custom IDP name and type query params, which are excluded from the query params in the upstream state",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   modifiedHappyGetRequestPath(map[string]string{"pinniped_idp_name": "currently-ignored", "pinniped_idp_type": "oidc"}),
+			contentType:                            formContentType,
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantBodyStringWithLocationInHref:       true,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", oidcUpstreamName, "oidc"), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
 		{
@@ -761,13 +882,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			cookieEncoder:                          happyCookieEncoder,
 			method:                                 http.MethodGet,
 			path:                                   modifiedHappyGetRequestPath(map[string]string{"prompt": "login"}),
-			contentType:                            "application/x-www-form-urlencoded",
-			body:                                   encodeQuery(happyGetRequestQueryMap),
 			wantStatus:                             http.StatusSeeOther,
 			wantContentType:                        htmlContentType,
 			wantBodyStringWithLocationInHref:       true,
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), map[string]string{"prompt": "consent", "abc": "123", "def": "456"}),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", oidcUpstreamName, "oidc"), map[string]string{"prompt": "consent", "abc": "123", "def": "456"}),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
 		{
@@ -780,10 +899,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			cookieEncoder:      happyCookieEncoder,
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"prompt": "none"}),
-			contentType:        "application/x-www-form-urlencoded",
-			body:               encodeQuery(happyGetRequestQueryMap),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeLoginRequiredErrorQuery),
 			wantBodyString:     "",
 		},
@@ -802,7 +919,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType: htmlContentType,
 			// Generated a new CSRF cookie and set it in the response.
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil),
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", oidcUpstreamName, "oidc"), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -823,7 +940,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{
 				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
-			}, "", ""), nil),
+			}, "", oidcUpstreamName, "oidc"), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -889,7 +1006,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{
 				"scope": "openid offline_access",
-			}, "", ""), nil),
+			}, "", oidcUpstreamName, "oidc"), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -1010,7 +1127,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 					Password: "wrong-password",
 				}},
 			wantStatus:         http.StatusFound,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1022,7 +1139,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr("wrong-password"),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1034,7 +1151,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr("wrong-password"),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1046,7 +1163,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr("wrong-username"),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1058,19 +1175,31 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr("wrong-username"),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
 		{
-			name:                 "missing upstream username on request for LDAP authentication",
+			name:                 "missing upstream username but has password on request for OIDC password grant",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: nil, // do not send header
+			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "missing upstream username but has password on request for LDAP authentication",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			method:               http.MethodGet,
 			path:                 happyGetRequestPath,
 			customUsernameHeader: nil, // do not send header
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1082,7 +1211,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: nil, // do not send header
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1094,7 +1223,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: nil, // do not send header
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1106,7 +1235,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: nil, // do not send header
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1119,7 +1248,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1132,7 +1261,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1145,7 +1274,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1158,7 +1287,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1171,7 +1300,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1184,7 +1313,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1196,7 +1325,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: nil, // do not send header
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1208,7 +1337,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithPasswordGrantDisallowedHintErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1225,7 +1354,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				"redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-pinniped-cli-client",
 			}),
 			wantStatus:      http.StatusBadRequest,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidRedirectURIErrorBody,
 		},
 		{
@@ -1238,7 +1367,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusBadRequest,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantBodyJSON:         fositeInvalidRedirectURIErrorBody,
 		},
 		{
@@ -1251,7 +1380,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusBadRequest,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantBodyJSON:         fositeInvalidRedirectURIErrorBody,
 		},
 		{
@@ -1264,7 +1393,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusBadRequest,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantBodyJSON:         fositeInvalidRedirectURIErrorBody,
 		},
 		{
@@ -1278,7 +1407,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:          http.MethodGet,
 			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}),
 			wantStatus:      http.StatusUnauthorized,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
@@ -1289,7 +1418,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusUnauthorized,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantBodyJSON:         fositeInvalidClientErrorBody,
 		},
 		{
@@ -1298,7 +1427,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:          http.MethodGet,
 			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}),
 			wantStatus:      http.StatusUnauthorized,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
@@ -1307,7 +1436,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:          http.MethodGet,
 			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}),
 			wantStatus:      http.StatusUnauthorized,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
@@ -1321,7 +1450,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1333,27 +1462,51 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:       "",
 		},
 		{
-			name:               "response type is unsupported when using LDAP upstream",
+			name:                 "response type is unsupported when using LDAP cli upstream",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
+			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
+			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:               "response type is unsupported when using LDAP browser upstream",
 			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
-			wantStatus:         http.StatusFound,
+			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
 		{
-			name:               "response type is unsupported when using active directory upstream",
+			name:                 "response type is unsupported when using active directory cli upstream",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
+			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:               "response type is unsupported when using active directory browser upstream",
 			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}),
-			wantStatus:         http.StatusFound,
+			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1368,7 +1521,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"scope": "openid profile email tuna"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1380,7 +1533,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1392,7 +1545,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1404,7 +1557,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:       "",
 		},
@@ -1419,7 +1572,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1431,27 +1584,51 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:       "",
 		},
 		{
-			name:               "missing response type in request using LDAP upstream",
+			name:                 "missing response type in request using LDAP cli upstream",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
+			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:               "missing response type in request using LDAP browser upstream",
 			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
-			wantStatus:         http.StatusFound,
+			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
 		{
-			name:               "missing response type in request using Active Directory upstream",
+			name:                 "missing response type in request using Active Directory cli upstream",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
+			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:               "missing response type in request using Active Directory browser upstream",
 			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"response_type": ""}),
-			wantStatus:         http.StatusFound,
+			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1466,7 +1643,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:          http.MethodGet,
 			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": ""}),
 			wantStatus:      http.StatusUnauthorized,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
@@ -1477,7 +1654,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusUnauthorized,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantBodyJSON:         fositeInvalidClientErrorBody,
 		},
 		{
@@ -1486,7 +1663,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:          http.MethodGet,
 			path:            modifiedHappyGetRequestPath(map[string]string{"client_id": ""}),
 			wantStatus:      http.StatusUnauthorized,
-			wantContentType: "application/json; charset=utf-8",
+			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidClientErrorBody,
 		},
 		{
@@ -1500,7 +1677,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"code_challenge": ""}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1513,7 +1690,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:         pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall:        happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1526,7 +1703,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader:         pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:         pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1542,7 +1719,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1555,7 +1732,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:         pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall:        happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1568,7 +1745,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader:         pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:         pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1584,7 +1761,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "plain"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1597,7 +1774,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:         pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall:        happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1610,7 +1787,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader:         pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:         pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1626,7 +1803,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": ""}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1639,7 +1816,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:         pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall:        happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1652,7 +1829,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader:         pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:         pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
@@ -1670,7 +1847,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
 			wantBodyString:     "",
 		},
@@ -1685,7 +1862,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:         pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall:        happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 1, // fosite already stored the authcode before it noticed the error
@@ -1700,7 +1877,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader:         pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader:         pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                   http.StatusFound,
-			wantContentType:              "application/json; charset=utf-8",
+			wantContentType:              jsonContentType,
 			wantLocationHeader:           urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
 			wantBodyString:               "",
 			wantUnnecessaryStoredRecords: 1, // fosite already stored the authcode before it noticed the error
@@ -1720,7 +1897,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType:             htmlContentType,
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(
-				map[string]string{"prompt": "none login", "scope": "email"}, "", "",
+				map[string]string{"prompt": "none login", "scope": "email"}, "", oidcUpstreamName, "oidc",
 			), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
@@ -1889,7 +2066,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithInvalidEmailVerifiedHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1907,7 +2084,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithFalseEmailVerifiedHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -1996,7 +2173,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimMissingHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2035,7 +2212,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2050,7 +2227,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimEmptyHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2065,7 +2242,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimMissingHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2080,7 +2257,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimEmptyHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2095,7 +2272,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2110,7 +2287,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimMissingHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2125,7 +2302,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimEmptyHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2140,7 +2317,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2155,7 +2332,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2170,7 +2347,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2185,7 +2362,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
 			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:            http.StatusFound,
-			wantContentType:       "application/json; charset=utf-8",
+			wantContentType:       jsonContentType,
 			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithRequiredClaimInvalidFormatHintErrorQuery),
 			wantBodyString:        "",
 		},
@@ -2200,7 +2377,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			method:             http.MethodGet,
 			path:               modifiedHappyGetRequestPath(map[string]string{"state": "short"}),
 			wantStatus:         http.StatusSeeOther,
-			wantContentType:    "application/json; charset=utf-8",
+			wantContentType:    jsonContentType,
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
 			wantBodyString:     "",
 		},
@@ -2212,7 +2389,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
 			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
 			wantBodyString:       "",
 		},
@@ -2224,7 +2401,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
 			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
 			wantStatus:           http.StatusFound,
-			wantContentType:      "application/json; charset=utf-8",
+			wantContentType:      jsonContentType,
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
 			wantBodyString:       "",
 		},
@@ -2401,7 +2578,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 
 		require.Equal(t, test.wantStatus, rsp.Code)
 		testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType)
-		testutil.RequireSecurityHeaders(t, rsp)
+		testutil.RequireSecurityHeadersWithoutCustomCSPs(t, rsp)
 
 		if test.wantPasswordGrantCall != nil {
 			test.wantPasswordGrantCall.args.Ctx = reqContext
@@ -2543,7 +2720,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				"scope":         "some-other-new-scope1 some-other-new-scope2", // updated expectation
 				"client_id":     "some-other-new-client-id",                    // updated expectation
 				"state": expectedUpstreamStateParam(
-					nil, "", "some-other-new-idp-name",
+					nil, "", "some-other-new-idp-name", "oidc",
 				), // updated expectation
 				"nonce":                 happyNonce,
 				"code_challenge":        expectedUpstreamCodeChallenge,

internal/oidc/callback/callback_handler.go

@@ -5,7 +5,6 @@
 package callback
 
 import (
-	"crypto/subtle"
 	"net/http"
 	"net/url"
 
@@ -14,7 +13,6 @@ import (
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/oidc"
-	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/oidc/provider/formposthtml"
@@ -102,9 +100,9 @@ func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder)
 		return nil, httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET)", r.Method)
 	}
 
-	csrfValue, err := readCSRFCookie(r, cookieDecoder)
+	_, decodedState, err := oidc.ReadStateParamAndValidateCSRFCookie(r, cookieDecoder, stateDecoder)
 	if err != nil {
-		plog.InfoErr("error reading CSRF cookie", err)
+		plog.InfoErr("state or CSRF error", err)
 		return nil, err
 	}
 
@@ -113,23 +111,7 @@ func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder)
 		return nil, httperr.New(http.StatusBadRequest, "code param not found")
 	}
 
-	if r.FormValue("state") == "" {
+	return decodedState, nil
-		plog.Info("state param not found")
-		return nil, httperr.New(http.StatusBadRequest, "state param not found")
-	}
-
-	state, err := readState(r, stateDecoder)
-	if err != nil {
-		plog.InfoErr("error reading state", err)
-		return nil, err
-	}
-
-	if subtle.ConstantTimeCompare([]byte(state.CSRFToken), []byte(csrfValue)) != 1 {
-		plog.InfoErr("CSRF value does not match", err)
-		return nil, httperr.Wrap(http.StatusForbidden, "CSRF value does not match", err)
-	}
-
-	return state, nil
 }
 
 func findUpstreamIDPConfig(upstreamName string, upstreamIDPs oidc.UpstreamOIDCIdentityProvidersLister) provider.UpstreamOIDCIdentityProviderI {
@@ -140,36 +122,3 @@ func findUpstreamIDPConfig(upstreamName string, upstreamIDPs oidc.UpstreamOIDCId
 	}
 	return nil
 }
-
-func readCSRFCookie(r *http.Request, cookieDecoder oidc.Decoder) (csrftoken.CSRFToken, error) {
-	receivedCSRFCookie, err := r.Cookie(oidc.CSRFCookieName)
-	if err != nil {
-		// Error means that the cookie was not found
-		return "", httperr.Wrap(http.StatusForbidden, "CSRF cookie is missing", err)
-	}
-
-	var csrfFromCookie csrftoken.CSRFToken
-	err = cookieDecoder.Decode(oidc.CSRFCookieEncodingName, receivedCSRFCookie.Value, &csrfFromCookie)
-	if err != nil {
-		return "", httperr.Wrap(http.StatusForbidden, "error reading CSRF cookie", err)
-	}
-
-	return csrfFromCookie, nil
-}
-
-func readState(r *http.Request, stateDecoder oidc.Decoder) (*oidc.UpstreamStateParamData, error) {
-	var state oidc.UpstreamStateParamData
-	if err := stateDecoder.Decode(
-		oidc.UpstreamStateParamEncodingName,
-		r.FormValue("state"),
-		&state,
-	); err != nil {
-		return nil, httperr.New(http.StatusBadRequest, "error reading state")
-	}
-
-	if state.FormatVersion != oidc.UpstreamStateParamFormatVersion {
-		return nil, httperr.New(http.StatusUnprocessableEntity, "state format version is invalid")
-	}
-
-	return &state, nil
-}

internal/oidc/callback/callback_handler_test.go

@@ -48,7 +48,7 @@ const (
 	happyDownstreamCSRF         = "test-csrf"
 	happyDownstreamPKCE         = "test-pkce"
 	happyDownstreamNonce        = "test-nonce"
-	happyDownstreamStateVersion = "1"
+	happyDownstreamStateVersion = "2"
 
 	downstreamIssuer              = "https://my-downstream-issuer.com/path"
 	downstreamRedirectURI         = "http://127.0.0.1/callback"
@@ -1034,7 +1034,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			t.Logf("response: %#v", rsp)
 			t.Logf("response body: %q", rsp.Body.String())
 
-			testutil.RequireSecurityHeaders(t, rsp)
+			testutil.RequireSecurityHeadersWithFormPostPageCSPs(t, rsp)
 
 			if test.wantAuthcodeExchangeCall != nil {
 				test.wantAuthcodeExchangeCall.args.Ctx = reqContext
@@ -1156,12 +1156,11 @@ func (r *requestPath) String() string {
 	return path + params.Encode()
 }
 
-type upstreamStateParamBuilder oidctestutil.ExpectedUpstreamStateParamFormat
+func happyUpstreamStateParam() *oidctestutil.UpstreamStateParamBuilder {
-
+	return &oidctestutil.UpstreamStateParamBuilder{
-func happyUpstreamStateParam() *upstreamStateParamBuilder {
-	return &upstreamStateParamBuilder{
 		U: happyUpstreamIDPName,
 		P: happyDownstreamRequestParams,
+		T: "oidc",
 		N: happyDownstreamNonce,
 		C: happyDownstreamCSRF,
 		K: happyDownstreamPKCE,
@@ -1169,37 +1168,6 @@ func happyUpstreamStateParam() *upstreamStateParamBuilder {
 	}
 }
 
-func (b upstreamStateParamBuilder) Build(t *testing.T, stateEncoder *securecookie.SecureCookie) string {
-	state, err := stateEncoder.Encode("s", b)
-	require.NoError(t, err)
-	return state
-}
-
-func (b *upstreamStateParamBuilder) WithAuthorizeRequestParams(params string) *upstreamStateParamBuilder {
-	b.P = params
-	return b
-}
-
-func (b *upstreamStateParamBuilder) WithNonce(nonce string) *upstreamStateParamBuilder {
-	b.N = nonce
-	return b
-}
-
-func (b *upstreamStateParamBuilder) WithCSRF(csrf string) *upstreamStateParamBuilder {
-	b.C = csrf
-	return b
-}
-
-func (b *upstreamStateParamBuilder) WithPKCVE(pkce string) *upstreamStateParamBuilder {
-	b.K = pkce
-	return b
-}
-
-func (b *upstreamStateParamBuilder) WithStateVersion(version string) *upstreamStateParamBuilder {
-	b.V = version
-	return b
-}
-
 func happyUpstream() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder {
 	return oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
 		WithName(happyUpstreamIDPName).

internal/oidc/downstreamsession/downstream_session.go

@@ -16,6 +16,7 @@ import (
 	"github.com/ory/fosite/token/jwt"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/constable"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/provider"
@@ -61,6 +62,34 @@ func MakeDownstreamSession(subject string, username string, groups []string, cus
 	return openIDSession
 }
 
+func MakeDownstreamLDAPOrADCustomSessionData(
+	ldapUpstream provider.UpstreamLDAPIdentityProviderI,
+	idpType psession.ProviderType,
+	authenticateResponse *authenticators.Response,
+) *psession.CustomSessionData {
+	customSessionData := &psession.CustomSessionData{
+		ProviderUID:  ldapUpstream.GetResourceUID(),
+		ProviderName: ldapUpstream.GetName(),
+		ProviderType: idpType,
+	}
+
+	if idpType == psession.ProviderTypeLDAP {
+		customSessionData.LDAP = &psession.LDAPSessionData{
+			UserDN:                 authenticateResponse.DN,
+			ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes,
+		}
+	}
+
+	if idpType == psession.ProviderTypeActiveDirectory {
+		customSessionData.ActiveDirectory = &psession.ActiveDirectorySessionData{
+			UserDN:                 authenticateResponse.DN,
+			ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes,
+		}
+	}
+
+	return customSessionData
+}
+
 func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdentityProviderI, token *oidctypes.Token) (*psession.CustomSessionData, error) {
 	upstreamSubject, err := ExtractStringClaimValue(oidc.IDTokenSubjectClaim, oidcUpstream.GetName(), token.IDToken.Claims)
 	if err != nil {
@@ -228,6 +257,11 @@ func ExtractStringClaimValue(claimName string, upstreamIDPName string, idTokenCl
 	return valueAsString, nil
 }
 
+func DownstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentityProviderI, authenticateResponse *authenticators.Response) string {
+	ldapURL := *ldapUpstream.GetURL()
+	return DownstreamLDAPSubject(authenticateResponse.User.GetUID(), ldapURL)
+}
+
 func DownstreamLDAPSubject(uid string, ldapURL url.URL) string {
 	q := ldapURL.Query()
 	q.Set(oidc.IDTokenSubjectClaim, uid)

internal/oidc/idpdiscovery/idp_discovery_handler.go

@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package idpdiscovery provides a handler for the upstream IDP discovery endpoint.
@@ -44,14 +44,14 @@ func responseAsJSON(upstreamIDPs oidc.UpstreamIdentityProvidersLister) ([]byte,
 		r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{
 			Name:  provider.GetName(),
 			Type:  v1alpha1.IDPTypeLDAP,
-			Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword},
+			Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode},
 		})
 	}
 	for _, provider := range upstreamIDPs.GetActiveDirectoryIdentityProviders() {
 		r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{
 			Name:  provider.GetName(),
 			Type:  v1alpha1.IDPTypeActiveDirectory,
-			Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword},
+			Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode},
 		})
 	}
 	for _, provider := range upstreamIDPs.GetOIDCIdentityProviders() {

internal/oidc/idpdiscovery/idp_discovery_handler_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package idpdiscovery
@@ -37,22 +37,22 @@ func TestIDPDiscovery(t *testing.T) {
 			wantContentType: "application/json",
 			wantFirstResponseBodyJSON: here.Doc(`{
 				"pinniped_identity_providers": [
-					{"name": "a-some-ldap-idp", "type": "ldap",            "flows": ["cli_password"]},
+					{"name": "a-some-ldap-idp", "type": "ldap",            "flows": ["cli_password", "browser_authcode"]},
 					{"name": "a-some-oidc-idp", "type": "oidc",            "flows": ["browser_authcode"]},
-					{"name": "x-some-idp",      "type": "ldap",            "flows": ["cli_password"]},
+					{"name": "x-some-idp",      "type": "ldap",            "flows": ["cli_password", "browser_authcode"]},
 					{"name": "x-some-idp",      "type": "oidc",            "flows": ["browser_authcode"]},
-					{"name": "y-some-ad-idp",   "type": "activedirectory", "flows": ["cli_password"]},
+					{"name": "y-some-ad-idp",   "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]},
-					{"name": "z-some-ad-idp",   "type": "activedirectory", "flows": ["cli_password"]},
+					{"name": "z-some-ad-idp",   "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]},
-					{"name": "z-some-ldap-idp", "type": "ldap",            "flows": ["cli_password"]},
+					{"name": "z-some-ldap-idp", "type": "ldap",            "flows": ["cli_password", "browser_authcode"]},
 					{"name": "z-some-oidc-idp", "type": "oidc",            "flows": ["browser_authcode", "cli_password"]}
 				]
 			}`),
 			wantSecondResponseBodyJSON: here.Doc(`{
 				"pinniped_identity_providers": [
-					{"name": "some-other-ad-idp-1",   "type": "activedirectory", "flows": ["cli_password"]},
+					{"name": "some-other-ad-idp-1",   "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]},
-					{"name": "some-other-ad-idp-2",   "type": "activedirectory", "flows": ["cli_password"]},
+					{"name": "some-other-ad-idp-2",   "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]},
-					{"name": "some-other-ldap-idp-1", "type": "ldap",            "flows": ["cli_password"]},
+					{"name": "some-other-ldap-idp-1", "type": "ldap",            "flows": ["cli_password", "browser_authcode"]},
-					{"name": "some-other-ldap-idp-2", "type": "ldap",            "flows": ["cli_password"]},
+					{"name": "some-other-ldap-idp-2", "type": "ldap",            "flows": ["cli_password", "browser_authcode"]},
 					{"name": "some-other-oidc-idp-1", "type": "oidc",            "flows": ["browser_authcode", "cli_password"]},
 					{"name": "some-other-oidc-idp-2", "type": "oidc",            "flows": ["browser_authcode"]}
 				]

internal/oidc/login/get_login_handler.go

@@ -0,0 +1,42 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"net/http"
+
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/login/loginhtml"
+)
+
+const (
+	internalErrorMessage                    = "An internal error occurred. Please contact your administrator for help."
+	incorrectUsernameOrPasswordErrorMessage = "Incorrect username or password."
+)
+
+func NewGetHandler(loginPath string) HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request, encodedState string, decodedState *oidc.UpstreamStateParamData) error {
+		alertMessage, hasAlert := getAlert(r)
+
+		pageInputs := &loginhtml.PageData{
+			PostPath:      loginPath,
+			State:         encodedState,
+			IDPName:       decodedState.UpstreamName,
+			HasAlertError: hasAlert,
+			AlertMessage:  alertMessage,
+		}
+		return loginhtml.Template().Execute(w, pageInputs)
+	}
+}
+
+func getAlert(r *http.Request) (string, bool) {
+	errorParamValue := r.URL.Query().Get(errParamName)
+
+	message := internalErrorMessage
+	if errorParamValue == string(ShowBadUserPassErr) {
+		message = incorrectUsernameOrPasswordErrorMessage
+	}
+
+	return message, errorParamValue != ""
+}

internal/oidc/login/get_login_handler_test.go

@@ -0,0 +1,116 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/login/loginhtml"
+	"go.pinniped.dev/internal/testutil"
+)
+
+func TestGetLogin(t *testing.T) {
+	const (
+		testPath         = "/some/path/login"
+		testUpstreamName = "some-ldap-idp"
+		testUpstreamType = "ldap"
+		testEncodedState = "fake-encoded-state-value"
+	)
+
+	tests := []struct {
+		name            string
+		decodedState    *oidc.UpstreamStateParamData
+		encodedState    string
+		errParam        string
+		idps            oidc.UpstreamIdentityProvidersLister
+		wantStatus      int
+		wantContentType string
+		wantBody        string
+	}{
+		{
+			name: "Happy path ldap",
+			decodedState: &oidc.UpstreamStateParamData{
+				UpstreamName: testUpstreamName,
+				UpstreamType: testUpstreamType,
+			},
+			encodedState:    testEncodedState, // the encoded and decoded state don't match, but that verification is handled one level up.
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBody:        testutil.ExpectedLoginPageHTML(loginhtml.CSS(), testUpstreamName, testPath, testEncodedState, ""), // no alert message
+		},
+		{
+			name: "displays error banner when err=login_error param is sent",
+			decodedState: &oidc.UpstreamStateParamData{
+				UpstreamName: testUpstreamName,
+				UpstreamType: testUpstreamType,
+			},
+			encodedState:    testEncodedState,
+			errParam:        "login_error",
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBody: testutil.ExpectedLoginPageHTML(loginhtml.CSS(), testUpstreamName, testPath, testEncodedState,
+				"Incorrect username or password.",
+			),
+		},
+		{
+			name: "displays error banner when err=internal_error param is sent",
+			decodedState: &oidc.UpstreamStateParamData{
+				UpstreamName: testUpstreamName,
+				UpstreamType: testUpstreamType,
+			},
+			encodedState:    testEncodedState,
+			errParam:        "internal_error",
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBody: testutil.ExpectedLoginPageHTML(loginhtml.CSS(), testUpstreamName, testPath, testEncodedState,
+				"An internal error occurred. Please contact your administrator for help.",
+			),
+		},
+		{
+			// If we get an error that we don't recognize, that's also an error, so we
+			// should probably just tell you to contact your administrator...
+			name: "displays generic error banner when unrecognized err param is sent",
+			decodedState: &oidc.UpstreamStateParamData{
+				UpstreamName: testUpstreamName,
+				UpstreamType: testUpstreamType,
+			},
+			encodedState:    testEncodedState,
+			errParam:        "some_other_error",
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBody: testutil.ExpectedLoginPageHTML(loginhtml.CSS(), testUpstreamName, testPath, testEncodedState,
+				"An internal error occurred. Please contact your administrator for help.",
+			),
+		},
+	}
+
+	for _, test := range tests {
+		tt := test
+
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			handler := NewGetHandler(testPath)
+			target := testPath + "?state=" + tt.encodedState
+			if tt.errParam != "" {
+				target += "&err=" + tt.errParam
+			}
+			req := httptest.NewRequest(http.MethodGet, target, nil)
+			rsp := httptest.NewRecorder()
+			err := handler(rsp, req, tt.encodedState, tt.decodedState)
+			require.NoError(t, err)
+
+			require.Equal(t, tt.wantStatus, rsp.Code)
+			testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), tt.wantContentType)
+			body := rsp.Body.String()
+			// t.Log("actual body:", body) // useful when updating expected values
+			require.Equal(t, tt.wantBody, body)
+		})
+	}
+}

internal/oidc/login/login_handler.go

@@ -0,0 +1,125 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"net/http"
+	"net/url"
+
+	idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
+	"go.pinniped.dev/internal/httputil/httperr"
+	"go.pinniped.dev/internal/httputil/securityheader"
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/login/loginhtml"
+	"go.pinniped.dev/internal/oidc/provider/formposthtml"
+	"go.pinniped.dev/internal/plog"
+)
+
+type ErrorParamValue string
+
+const (
+	usernameParamName = "username"
+	passwordParamName = "password"
+	stateParamName    = "state"
+	errParamName      = "err"
+
+	ShowNoError        ErrorParamValue = ""
+	ShowInternalError  ErrorParamValue = "internal_error"
+	ShowBadUserPassErr ErrorParamValue = "login_error"
+)
+
+// HandlerFunc is a function that can handle either a GET or POST request for the login endpoint.
+type HandlerFunc func(
+	w http.ResponseWriter,
+	r *http.Request,
+	encodedState string,
+	decodedState *oidc.UpstreamStateParamData,
+) error
+
+// NewHandler returns a http.Handler that serves the login endpoint for IDPs that don't have their own web UI for login.
+//
+// This handler takes care of the shared concerns between the GET and POST methods of the login endpoint:
+// checking the method, checking the CSRF cookie, decoding the state param, and adding security headers.
+// Then it defers the rest of the handling to the passed in handler functions for GET and POST requests.
+// Note that CSRF protection isn't needed on GET requests, but it doesn't hurt. Putting it here
+// keeps the implementations and tests of HandlerFunc simpler since they won't need to deal with any decoders.
+// Users should always initially get redirected to this page from the authorization endpoint, and never need
+// to navigate directly to this page in their browser without going through the authorization endpoint first.
+// Once their browser has landed on this page, it should be okay for the user to refresh the browser.
+func NewHandler(
+	stateDecoder oidc.Decoder,
+	cookieDecoder oidc.Decoder,
+	getHandler HandlerFunc, // use NewGetHandler() for production
+	postHandler HandlerFunc, // use NewPostHandler() for production
+) http.Handler {
+	loginHandler := httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+		var handler HandlerFunc
+		switch r.Method {
+		case http.MethodGet:
+			handler = getHandler
+		case http.MethodPost:
+			handler = postHandler
+		default:
+			return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method)
+		}
+
+		encodedState, decodedState, err := oidc.ReadStateParamAndValidateCSRFCookie(r, cookieDecoder, stateDecoder)
+		if err != nil {
+			plog.InfoErr("state or CSRF error", err)
+			return err
+		}
+
+		switch decodedState.UpstreamType {
+		case string(idpdiscoveryv1alpha1.IDPTypeLDAP), string(idpdiscoveryv1alpha1.IDPTypeActiveDirectory):
+			// these are the types supported by this endpoint, so no error here
+		default:
+			return httperr.Newf(http.StatusBadRequest, "not a supported upstream IDP type for this endpoint: %q", decodedState.UpstreamType)
+		}
+
+		return handler(w, r, encodedState, decodedState)
+	})
+
+	return wrapSecurityHeaders(loginHandler)
+}
+
+func wrapSecurityHeaders(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		wrapped := securityheader.WrapWithCustomCSP(handler, loginhtml.ContentSecurityPolicy())
+		if r.Method == http.MethodPost {
+			// POST requests can result in the form_post html page, so allow it with CSP headers.
+			wrapped = securityheader.WrapWithCustomCSP(handler, formposthtml.ContentSecurityPolicy())
+		}
+		wrapped.ServeHTTP(w, r)
+	})
+}
+
+// RedirectToLoginPage redirects to the GET /login page of the specified issuer.
+// The specified issuer should never end with a "/", which is validated by
+// provider.FederationDomainIssuer when the issuer string comes from that type.
+func RedirectToLoginPage(
+	r *http.Request,
+	w http.ResponseWriter,
+	downstreamIssuer string,
+	encodedStateParamValue string,
+	errToDisplay ErrorParamValue,
+) error {
+	loginURL, err := url.Parse(downstreamIssuer + oidc.PinnipedLoginPath)
+	if err != nil {
+		return err
+	}
+
+	q := loginURL.Query()
+	q.Set(stateParamName, encodedStateParamValue)
+	if errToDisplay != ShowNoError {
+		q.Set(errParamName, string(errToDisplay))
+	}
+	loginURL.RawQuery = q.Encode()
+
+	http.Redirect(w, r,
+		loginURL.String(),
+		http.StatusSeeOther, // match fosite and https://tools.ietf.org/id/draft-ietf-oauth-security-topics-18.html#section-4.11
+	)
+
+	return nil
+}

internal/oidc/login/login_handler_test.go

@@ -0,0 +1,457 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/gorilla/securecookie"
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/httputil/httperr"
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
+)
+
+const (
+	htmlContentType = "text/html; charset=utf-8"
+)
+
+func TestLoginEndpoint(t *testing.T) {
+	const (
+		happyGetResult  = "<p>get handler result</p>"
+		happyPostResult = "<p>post handler result</p>"
+
+		happyUpstreamIDPName        = "upstream-idp-name"
+		happyUpstreamIDPType        = "ldap"
+		happyDownstreamCSRF         = "test-csrf"
+		happyDownstreamPKCE         = "test-pkce"
+		happyDownstreamNonce        = "test-nonce"
+		happyDownstreamStateVersion = "2"
+
+		downstreamClientID            = "pinniped-cli"
+		happyDownstreamState          = "8b-state"
+		downstreamNonce               = "some-nonce-value"
+		downstreamPKCEChallenge       = "some-challenge"
+		downstreamPKCEChallengeMethod = "S256"
+		downstreamRedirectURI         = "http://127.0.0.1/callback"
+	)
+
+	happyDownstreamScopesRequested := []string{"openid"}
+	happyDownstreamRequestParamsQuery := url.Values{
+		"response_type":         []string{"code"},
+		"scope":                 []string{strings.Join(happyDownstreamScopesRequested, " ")},
+		"client_id":             []string{downstreamClientID},
+		"state":                 []string{happyDownstreamState},
+		"nonce":                 []string{downstreamNonce},
+		"code_challenge":        []string{downstreamPKCEChallenge},
+		"code_challenge_method": []string{downstreamPKCEChallengeMethod},
+		"redirect_uri":          []string{downstreamRedirectURI},
+	}
+	happyDownstreamRequestParams := happyDownstreamRequestParamsQuery.Encode()
+
+	expectedHappyDecodedUpstreamStateParam := func() *oidc.UpstreamStateParamData {
+		return &oidc.UpstreamStateParamData{
+			UpstreamName:  happyUpstreamIDPName,
+			UpstreamType:  happyUpstreamIDPType,
+			AuthParams:    happyDownstreamRequestParams,
+			Nonce:         happyDownstreamNonce,
+			CSRFToken:     happyDownstreamCSRF,
+			PKCECode:      happyDownstreamPKCE,
+			FormatVersion: happyDownstreamStateVersion,
+		}
+	}
+
+	expectedHappyDecodedUpstreamStateParamForActiveDirectory := func() *oidc.UpstreamStateParamData {
+		s := expectedHappyDecodedUpstreamStateParam()
+		s.UpstreamType = "activedirectory"
+		return s
+	}
+
+	happyUpstreamStateParam := func() *oidctestutil.UpstreamStateParamBuilder {
+		return &oidctestutil.UpstreamStateParamBuilder{
+			U: happyUpstreamIDPName,
+			T: happyUpstreamIDPType,
+			P: happyDownstreamRequestParams,
+			N: happyDownstreamNonce,
+			C: happyDownstreamCSRF,
+			K: happyDownstreamPKCE,
+			V: happyDownstreamStateVersion,
+		}
+	}
+
+	stateEncoderHashKey := []byte("fake-hash-secret")
+	stateEncoderBlockKey := []byte("0123456789ABCDEF") // block encryption requires 16/24/32 bytes for AES
+	cookieEncoderHashKey := []byte("fake-hash-secret2")
+	cookieEncoderBlockKey := []byte("0123456789ABCDE2") // block encryption requires 16/24/32 bytes for AES
+	require.NotEqual(t, stateEncoderHashKey, cookieEncoderHashKey)
+	require.NotEqual(t, stateEncoderBlockKey, cookieEncoderBlockKey)
+
+	happyStateCodec := securecookie.New(stateEncoderHashKey, stateEncoderBlockKey)
+	happyStateCodec.SetSerializer(securecookie.JSONEncoder{})
+	happyCookieCodec := securecookie.New(cookieEncoderHashKey, cookieEncoderBlockKey)
+	happyCookieCodec.SetSerializer(securecookie.JSONEncoder{})
+
+	happyState := happyUpstreamStateParam().Build(t, happyStateCodec)
+	happyPathWithState := newRequestPath().WithState(happyState).String()
+
+	happyActiveDirectoryState := happyUpstreamStateParam().WithUpstreamIDPType("activedirectory").Build(t, happyStateCodec)
+
+	encodedIncomingCookieCSRFValue, err := happyCookieCodec.Encode("csrf", happyDownstreamCSRF)
+	require.NoError(t, err)
+	happyCSRFCookie := "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue
+
+	tests := []struct {
+		name           string
+		method         string
+		path           string
+		csrfCookie     string
+		getHandlerErr  error
+		postHandlerErr error
+
+		wantStatus       int
+		wantContentType  string
+		wantBody         string
+		wantEncodedState string
+		wantDecodedState *oidc.UpstreamStateParamData
+	}{
+		{
+			name:            "PUT method is invalid",
+			method:          http.MethodPut,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: PUT (try GET or POST)\n",
+		},
+		{
+			name:            "PATCH method is invalid",
+			method:          http.MethodPatch,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: PATCH (try GET or POST)\n",
+		},
+		{
+			name:            "DELETE method is invalid",
+			method:          http.MethodDelete,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: DELETE (try GET or POST)\n",
+		},
+		{
+			name:            "HEAD method is invalid",
+			method:          http.MethodHead,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: HEAD (try GET or POST)\n",
+		},
+		{
+			name:            "CONNECT method is invalid",
+			method:          http.MethodConnect,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: CONNECT (try GET or POST)\n",
+		},
+		{
+			name:            "OPTIONS method is invalid",
+			method:          http.MethodOptions,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: OPTIONS (try GET or POST)\n",
+		},
+		{
+			name:            "TRACE method is invalid",
+			method:          http.MethodTrace,
+			path:            happyPathWithState,
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: htmlContentType,
+			wantBody:        "Method Not Allowed: TRACE (try GET or POST)\n",
+		},
+		{
+			name:            "state param was not included on GET request",
+			method:          http.MethodGet,
+			path:            newRequestPath().WithoutState().String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: state param not found\n",
+		},
+		{
+			name:            "state param was not included on POST request",
+			method:          http.MethodPost,
+			path:            newRequestPath().WithoutState().String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			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 on GET request",
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState("this-will-not-decode").String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error reading state\n",
+		},
+		{
+			name:            "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason on POST request",
+			method:          http.MethodPost,
+			path:            newRequestPath().WithState("this-will-not-decode").String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error reading state\n",
+		},
+		{
+			name:            "the CSRF cookie does not exist on GET request",
+			method:          http.MethodGet,
+			path:            happyPathWithState,
+			csrfCookie:      "",
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF cookie is missing\n",
+		},
+		{
+			name:            "the CSRF cookie does not exist on POST request",
+			method:          http.MethodPost,
+			path:            happyPathWithState,
+			csrfCookie:      "",
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF cookie is missing\n",
+		},
+		{
+			name:            "the CSRF cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason on GET request",
+			method:          http.MethodGet,
+			path:            happyPathWithState,
+			csrfCookie:      "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: error reading CSRF cookie\n",
+		},
+		{
+			name:            "the CSRF cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason on POST request",
+			method:          http.MethodPost,
+			path:            happyPathWithState,
+			csrfCookie:      "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: error reading CSRF cookie\n",
+		},
+		{
+			name:            "cookie csrf value does not match state csrf value on GET request",
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF value does not match\n",
+		},
+		{
+			name:            "cookie csrf value does not match state csrf value on POST request",
+			method:          http.MethodPost,
+			path:            newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusForbidden,
+			wantContentType: htmlContentType,
+			wantBody:        "Forbidden: CSRF value does not match\n",
+		},
+		{
+			name:   "GET request when upstream IDP type in state param is not supported by this endpoint",
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithUpstreamIDPType("oidc").Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: not a supported upstream IDP type for this endpoint: \"oidc\"\n",
+		},
+		{
+			name:   "POST request when upstream IDP type in state param is not supported by this endpoint",
+			method: http.MethodPost,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithUpstreamIDPType("oidc").Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: not a supported upstream IDP type for this endpoint: \"oidc\"\n",
+		},
+		{
+			name:             "valid GET request when GET endpoint handler returns an error",
+			method:           http.MethodGet,
+			path:             happyPathWithState,
+			csrfCookie:       happyCSRFCookie,
+			getHandlerErr:    httperr.Newf(http.StatusInternalServerError, "some get error"),
+			wantStatus:       http.StatusInternalServerError,
+			wantContentType:  htmlContentType,
+			wantBody:         "Internal Server Error: some get error\n",
+			wantEncodedState: happyState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParam(),
+		},
+		{
+			name:             "valid POST request when POST endpoint handler returns an error",
+			method:           http.MethodPost,
+			path:             happyPathWithState,
+			csrfCookie:       happyCSRFCookie,
+			postHandlerErr:   httperr.Newf(http.StatusInternalServerError, "some post error"),
+			wantStatus:       http.StatusInternalServerError,
+			wantContentType:  htmlContentType,
+			wantBody:         "Internal Server Error: some post error\n",
+			wantEncodedState: happyState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParam(),
+		},
+		{
+			name:             "happy GET request for LDAP upstream",
+			method:           http.MethodGet,
+			path:             happyPathWithState,
+			csrfCookie:       happyCSRFCookie,
+			wantStatus:       http.StatusOK,
+			wantContentType:  htmlContentType,
+			wantBody:         happyGetResult,
+			wantEncodedState: happyState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParam(),
+		},
+		{
+			name:             "happy POST request for LDAP upstream",
+			method:           http.MethodPost,
+			path:             happyPathWithState,
+			csrfCookie:       happyCSRFCookie,
+			wantStatus:       http.StatusOK,
+			wantContentType:  htmlContentType,
+			wantBody:         happyPostResult,
+			wantEncodedState: happyState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParam(),
+		},
+		{
+			name:             "happy GET request for ActiveDirectory upstream",
+			method:           http.MethodGet,
+			path:             newRequestPath().WithState(happyActiveDirectoryState).String(),
+			csrfCookie:       happyCSRFCookie,
+			wantStatus:       http.StatusOK,
+			wantContentType:  htmlContentType,
+			wantBody:         happyGetResult,
+			wantEncodedState: happyActiveDirectoryState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParamForActiveDirectory(),
+		},
+		{
+			name:             "happy POST request for ActiveDirectory upstream",
+			method:           http.MethodPost,
+			path:             newRequestPath().WithState(happyActiveDirectoryState).String(),
+			csrfCookie:       happyCSRFCookie,
+			wantStatus:       http.StatusOK,
+			wantContentType:  htmlContentType,
+			wantBody:         happyPostResult,
+			wantEncodedState: happyActiveDirectoryState,
+			wantDecodedState: expectedHappyDecodedUpstreamStateParamForActiveDirectory(),
+		},
+	}
+
+	for _, test := range tests {
+		tt := test
+
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := httptest.NewRequest(tt.method, tt.path, nil)
+			if tt.csrfCookie != "" {
+				req.Header.Set("Cookie", tt.csrfCookie)
+			}
+			rsp := httptest.NewRecorder()
+
+			testGetHandler := func(
+				w http.ResponseWriter,
+				r *http.Request,
+				encodedState string,
+				decodedState *oidc.UpstreamStateParamData,
+			) error {
+				require.Equal(t, req, r)
+				require.Equal(t, rsp, w)
+				require.Equal(t, tt.wantEncodedState, encodedState)
+				require.Equal(t, tt.wantDecodedState, decodedState)
+				if tt.getHandlerErr == nil {
+					_, err := w.Write([]byte(happyGetResult))
+					require.NoError(t, err)
+				}
+				return tt.getHandlerErr
+			}
+
+			testPostHandler := func(
+				w http.ResponseWriter,
+				r *http.Request,
+				encodedState string,
+				decodedState *oidc.UpstreamStateParamData,
+			) error {
+				require.Equal(t, req, r)
+				require.Equal(t, rsp, w)
+				require.Equal(t, tt.wantEncodedState, encodedState)
+				require.Equal(t, tt.wantDecodedState, decodedState)
+				if tt.postHandlerErr == nil {
+					_, err := w.Write([]byte(happyPostResult))
+					require.NoError(t, err)
+				}
+				return tt.postHandlerErr
+			}
+
+			subject := NewHandler(happyStateCodec, happyCookieCodec, testGetHandler, testPostHandler)
+
+			subject.ServeHTTP(rsp, req)
+
+			if tt.method == http.MethodPost {
+				testutil.RequireSecurityHeadersWithFormPostPageCSPs(t, rsp)
+			} else {
+				testutil.RequireSecurityHeadersWithLoginPageCSPs(t, rsp)
+			}
+
+			require.Equal(t, tt.wantStatus, rsp.Code)
+			testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), tt.wantContentType)
+			require.Equal(t, tt.wantBody, rsp.Body.String())
+		})
+	}
+}
+
+type requestPath struct {
+	state *string
+}
+
+func newRequestPath() *requestPath {
+	return &requestPath{}
+}
+
+func (r *requestPath) WithState(state string) *requestPath {
+	r.state = &state
+	return r
+}
+
+func (r *requestPath) WithoutState() *requestPath {
+	r.state = nil
+	return r
+}
+
+func (r *requestPath) String() string {
+	path := "/login?"
+	params := url.Values{}
+	if r.state != nil {
+		params.Add("state", *r.state)
+	}
+	return path + params.Encode()
+}

internal/oidc/login/loginhtml/login_form.css

@@ -0,0 +1,94 @@
+/* Copyright 2022 the Pinniped contributors. All Rights Reserved. */
+/* SPDX-License-Identifier: Apache-2.0 */
+
+html {
+    height: 100%;
+}
+
+body {
+    font-family: "Metropolis-Light", Helvetica, sans-serif;
+    display: flex;
+    flex-flow: column wrap;
+    justify-content: flex-start;
+    align-items: center;
+    /* subtle gradient make the login box stand out */
+    background: linear-gradient(to top, #f8f8f8, white);
+    min-height: 100%;
+}
+
+h1 {
+    font-size: 20px;
+    margin: 0;
+}
+
+.box {
+    display: flex;
+    flex-direction: column;
+    flex-wrap: nowrap;
+    border-radius: 4px;
+    border-color: #ddd;
+    border-width: 1px;
+    border-style: solid;
+    width: 400px;
+    padding:30px 30px 0;
+    margin: 60px 20px 0;
+    background: white;
+    font-size: 14px;
+}
+
+input {
+    color: inherit;
+    font: inherit;
+    border: 0;
+    margin: 0;
+    outline: 0;
+    padding: 0;
+}
+
+.form-field {
+    display: flex;
+    margin-bottom: 30px;
+}
+
+.form-field input[type="password"], .form-field input[type="text"], .form-field input[type="submit"] {
+    width: 100%;
+    padding: 1em;
+}
+
+.form-field input[type="password"], .form-field input[type="text"] {
+    border-radius: 3px;
+    border-width: 1px;
+    border-style: solid;
+    border-color: #a6a6a6;
+}
+
+.form-field input[type="submit"] {
+    background-color: #218fcf; /* this is a color from the Pinniped logo :) */
+    color: #eee;
+    font-weight: bold;
+    cursor: pointer;
+    transition: all .3s;
+}
+
+.form-field input[type="submit"]:focus, .form-field input[type="submit"]:hover {
+    background-color: #1abfd3; /* this is a color from the Pinniped logo :) */
+}
+
+.form-field input[type="submit"]:active {
+    transform: scale(.99);
+}
+
+.hidden {
+    border: 0;
+    clip: rect(0 0 0 0);
+    height: 1px;
+    margin: -1px;
+    overflow: hidden;
+    padding: 0;
+    position: absolute;
+    width: 1px;
+}
+
+.alert {
+    color: crimson;
+}

internal/oidc/login/loginhtml/loginhtml.go

@@ -0,0 +1,65 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package loginhtml defines HTML templates used by the Supervisor.
+//nolint: gochecknoglobals // This package uses globals to ensure that all parsing and minifying happens at init.
+package loginhtml
+
+import (
+	_ "embed" // Needed to trigger //go:embed directives below.
+	"html/template"
+	"strings"
+
+	"github.com/tdewolff/minify/v2/minify"
+
+	"go.pinniped.dev/internal/oidc/provider/csp"
+)
+
+var (
+	//go:embed login_form.css
+	rawCSS      string
+	minifiedCSS = panicOnError(minify.CSS(rawCSS))
+
+	//go:embed login_form.gohtml
+	rawHTMLTemplate string
+)
+
+// Parse the Go templated HTML and inject functions providing the minified inline CSS and JS.
+var parsedHTMLTemplate = template.Must(template.New("login_form.gohtml").Funcs(template.FuncMap{
+	"minifiedCSS": func() template.CSS { return template.CSS(CSS()) },
+}).Parse(rawHTMLTemplate))
+
+// Generate the CSP header value once since it's effectively constant.
+var cspValue = strings.Join([]string{
+	`default-src 'none'`,
+	`style-src '` + csp.Hash(minifiedCSS) + `'`,
+	`frame-ancestors 'none'`,
+}, "; ")
+
+func panicOnError(s string, err error) string {
+	if err != nil {
+		panic(err)
+	}
+	return s
+}
+
+// ContentSecurityPolicy returns the Content-Security-Policy header value to make the Template() operate correctly.
+//
+// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy.
+func ContentSecurityPolicy() string { return cspValue }
+
+// Template returns the html/template.Template for rendering the login page.
+func Template() *template.Template { return parsedHTMLTemplate }
+
+// CSS returns the minified CSS that will be embedded into the page template.
+func CSS() string { return minifiedCSS }
+
+// PageData represents the inputs to the template.
+type PageData struct {
+	State         string
+	IDPName       string
+	HasAlertError bool
+	AlertMessage  string
+	MinifiedCSS   template.CSS
+	PostPath      string
+}

internal/oidc/login/loginhtml/loginhtml_test.go

@@ -0,0 +1,68 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package loginhtml
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"go.pinniped.dev/internal/testutil"
+
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	testExpectedCSS = `html{height:100%}body{font-family:metropolis-light,Helvetica,sans-serif;display:flex;flex-flow:column wrap;justify-content:flex-start;align-items:center;background:linear-gradient(to top,#f8f8f8,white);min-height:100%}h1{font-size:20px;margin:0}.box{display:flex;flex-direction:column;flex-wrap:nowrap;border-radius:4px;border-color:#ddd;border-width:1px;border-style:solid;width:400px;padding:30px 30px 0;margin:60px 20px 0;background:#fff;font-size:14px}input{color:inherit;font:inherit;border:0;margin:0;outline:0;padding:0}.form-field{display:flex;margin-bottom:30px}.form-field input[type=password],.form-field input[type=text],.form-field input[type=submit]{width:100%;padding:1em}.form-field input[type=password],.form-field input[type=text]{border-radius:3px;border-width:1px;border-style:solid;border-color:#a6a6a6}.form-field input[type=submit]{background-color:#218fcf;color:#eee;font-weight:700;cursor:pointer;transition:all .3s}.form-field input[type=submit]:focus,.form-field input[type=submit]:hover{background-color:#1abfd3}.form-field input[type=submit]:active{transform:scale(.99)}.hidden{border:0;clip:rect(0 0 0 0);height:1px;margin:-1px;overflow:hidden;padding:0;position:absolute;width:1px}.alert{color:crimson}`
+
+	// It's okay if this changes in the future, but this gives us a chance to eyeball the formatting.
+	// Our browser-based integration tests should find any incompatibilities.
+	testExpectedCSP = `default-src 'none'; ` +
+		`style-src 'sha256-QC9ckaUFAdcN0Ysmu8q8iqCazYFgrJSQDJPa/przPXU='; ` +
+		`frame-ancestors 'none'`
+)
+
+func TestTemplate(t *testing.T) {
+	const (
+		testUpstreamName = "test-idp-name"
+		testPath         = "test-post-path"
+		testEncodedState = "test-encoded-state"
+		testAlert        = "test-alert-message"
+	)
+
+	var buf bytes.Buffer
+	pageInputs := &PageData{
+		PostPath:      testPath,
+		State:         testEncodedState,
+		IDPName:       testUpstreamName,
+		HasAlertError: true,
+		AlertMessage:  testAlert,
+	}
+
+	// Render with an alert.
+	expectedHTMLWithAlert := testutil.ExpectedLoginPageHTML(testExpectedCSS, testUpstreamName, testPath, testEncodedState, testAlert)
+	require.NoError(t, Template().Execute(&buf, pageInputs))
+	// t.Logf("actual value:\n%s", buf.String()) // useful when updating minify library causes new output
+	require.Equal(t, expectedHTMLWithAlert, buf.String())
+
+	// Render again without an alert.
+	pageInputs.HasAlertError = false
+	expectedHTMLWithoutAlert := testutil.ExpectedLoginPageHTML(testExpectedCSS, testUpstreamName, testPath, testEncodedState, "")
+	buf = bytes.Buffer{} // clear previous result from buffer
+	require.NoError(t, Template().Execute(&buf, pageInputs))
+	require.Equal(t, expectedHTMLWithoutAlert, buf.String())
+}
+
+func TestContentSecurityPolicy(t *testing.T) {
+	require.Equal(t, testExpectedCSP, ContentSecurityPolicy())
+}
+
+func TestCSS(t *testing.T) {
+	require.Equal(t, testExpectedCSS, CSS())
+}
+
+func TestHelpers(t *testing.T) {
+	require.Equal(t, "test", panicOnError("test", nil))
+	require.PanicsWithError(t, "some error", func() { panicOnError("", fmt.Errorf("some error")) })
+}

internal/oidc/login/post_login_handler.go

@@ -0,0 +1,88 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"net/http"
+	"net/url"
+
+	"github.com/ory/fosite"
+
+	"go.pinniped.dev/internal/httputil/httperr"
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/downstreamsession"
+	"go.pinniped.dev/internal/plog"
+)
+
+func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvidersLister, oauthHelper fosite.OAuth2Provider) HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request, encodedState string, decodedState *oidc.UpstreamStateParamData) error {
+		// Note that the login handler prevents this handler from being called with OIDC upstreams.
+		_, ldapUpstream, idpType, err := oidc.FindUpstreamIDPByNameAndType(upstreamIDPs, decodedState.UpstreamName, decodedState.UpstreamType)
+		if err != nil {
+			// This shouldn't normally happen because the authorization endpoint ensured that this provider existed
+			// at that time. It would be possible in the unlikely event that the provider was deleted during the login.
+			plog.Error("error finding upstream provider", err)
+			return httperr.Wrap(http.StatusUnprocessableEntity, "error finding upstream provider", err)
+		}
+
+		// Get the original params that were used at the authorization endpoint.
+		downstreamAuthParams, err := url.ParseQuery(decodedState.AuthParams)
+		if err != nil {
+			// This shouldn't really happen because the authorization endpoint encoded these query params correctly.
+			plog.Error("error reading state downstream auth params", err)
+			return httperr.New(http.StatusBadRequest, "error reading state downstream auth params")
+		}
+
+		// Recreate enough of the original authorize request so we can pass it to NewAuthorizeRequest().
+		reconstitutedAuthRequest := &http.Request{Form: downstreamAuthParams}
+		authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), reconstitutedAuthRequest)
+		if err != nil {
+			// This shouldn't really happen because the authorization endpoint has already validated these params
+			// by calling NewAuthorizeRequest() itself.
+			plog.Error("error using state downstream auth params", err)
+			return httperr.New(http.StatusBadRequest, "error using state downstream auth params")
+		}
+
+		// Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested.
+		downstreamsession.GrantScopesIfRequested(authorizeRequester)
+
+		// Get the username and password form params from the POST body.
+		username := r.PostFormValue(usernameParamName)
+		password := r.PostFormValue(passwordParamName)
+
+		// Treat blank username or password as a bad username/password combination, as opposed to an internal error.
+		if username == "" || password == "" {
+			// User forgot to enter one of the required fields.
+			// The user may try to log in again if they'd like, so redirect back to the login page with an error.
+			return RedirectToLoginPage(r, w, issuerURL, encodedState, ShowBadUserPassErr)
+		}
+
+		// Attempt to authenticate the user with the upstream IDP.
+		authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password)
+		if err != nil {
+			plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName())
+			// There was some problem during authentication with the upstream, aside from bad username/password.
+			// The user may try to log in again if they'd like, so redirect back to the login page with an error.
+			return RedirectToLoginPage(r, w, issuerURL, encodedState, ShowInternalError)
+		}
+		if !authenticated {
+			// The upstream did not accept the username/password combination.
+			// The user may try to log in again if they'd like, so redirect back to the login page with an error.
+			return RedirectToLoginPage(r, w, issuerURL, encodedState, ShowBadUserPassErr)
+		}
+
+		// We had previously interrupted the regular steps of the OIDC authcode flow to show the login page UI.
+		// Now the upstream IDP has authenticated the user, so now we're back into the regular OIDC authcode flow steps.
+		// Both success and error responses from this point onwards should look like the usual fosite redirect
+		// responses, and a happy redirect response will include a downstream authcode.
+		subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse)
+		username = authenticateResponse.User.GetName()
+		groups := authenticateResponse.User.GetGroups()
+		customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse)
+		openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData)
+		oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false)
+
+		return nil
+	}
+}

internal/oidc/login/post_login_handler_test.go

@@ -0,0 +1,748 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package login
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/client-go/kubernetes/fake"
+
+	"go.pinniped.dev/internal/authenticators"
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/psession"
+	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
+)
+
+func TestPostLoginEndpoint(t *testing.T) {
+	const (
+		htmlContentType = "text/html; charset=utf-8"
+
+		happyDownstreamCSRF         = "test-csrf"
+		happyDownstreamPKCE         = "test-pkce"
+		happyDownstreamNonce        = "test-nonce"
+		happyDownstreamStateVersion = "2"
+		happyEncodedUpstreamState   = "fake-encoded-state-param-value"
+
+		downstreamIssuer              = "https://my-downstream-issuer.com/path"
+		downstreamRedirectURI         = "http://127.0.0.1/callback"
+		downstreamClientID            = "pinniped-cli"
+		happyDownstreamState          = "8b-state"
+		downstreamNonce               = "some-nonce-value"
+		downstreamPKCEChallenge       = "some-challenge"
+		downstreamPKCEChallengeMethod = "S256"
+
+		ldapUpstreamName                   = "some-ldap-idp"
+		ldapUpstreamType                   = "ldap"
+		ldapUpstreamResourceUID            = "ldap-resource-uid"
+		activeDirectoryUpstreamName        = "some-active-directory-idp"
+		activeDirectoryUpstreamType        = "activedirectory"
+		activeDirectoryUpstreamResourceUID = "active-directory-resource-uid"
+		upstreamLDAPURL                    = "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev"
+
+		userParam                = "username"
+		passParam                = "password"
+		badUserPassErrParamValue = "login_error"
+		internalErrParamValue    = "internal_error"
+	)
+
+	var (
+		fositeMissingCodeChallengeErrorQuery = map[string]string{
+			"error":             "invalid_request",
+			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Clients must include a code_challenge when performing the authorize code flow, but it is missing.",
+			"state":             happyDownstreamState,
+		}
+
+		fositeInvalidCodeChallengeErrorQuery = map[string]string{
+			"error":             "invalid_request",
+			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. The code_challenge_method is not supported, use S256 instead.",
+			"state":             happyDownstreamState,
+		}
+
+		fositeMissingCodeChallengeMethodErrorQuery = map[string]string{
+			"error":             "invalid_request",
+			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Clients must use code_challenge_method=S256, plain is not allowed.",
+			"state":             happyDownstreamState,
+		}
+
+		fositePromptHasNoneAndOtherValueErrorQuery = map[string]string{
+			"error":             "invalid_request",
+			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Parameter 'prompt' was set to 'none', but contains other values as well which is not allowed.",
+			"state":             happyDownstreamState,
+		}
+	)
+
+	happyDownstreamScopesRequested := []string{"openid"}
+	happyDownstreamScopesGranted := []string{"openid"}
+
+	happyDownstreamRequestParamsQuery := url.Values{
+		"response_type":         []string{"code"},
+		"scope":                 []string{strings.Join(happyDownstreamScopesRequested, " ")},
+		"client_id":             []string{downstreamClientID},
+		"state":                 []string{happyDownstreamState},
+		"nonce":                 []string{downstreamNonce},
+		"code_challenge":        []string{downstreamPKCEChallenge},
+		"code_challenge_method": []string{downstreamPKCEChallengeMethod},
+		"redirect_uri":          []string{downstreamRedirectURI},
+	}
+	happyDownstreamRequestParams := happyDownstreamRequestParamsQuery.Encode()
+
+	copyOfHappyDownstreamRequestParamsQuery := func() url.Values {
+		params := url.Values{}
+		for k, v := range happyDownstreamRequestParamsQuery {
+			params[k] = make([]string, len(v))
+			copy(params[k], v)
+		}
+		return params
+	}
+
+	happyLDAPDecodedState := &oidc.UpstreamStateParamData{
+		AuthParams:    happyDownstreamRequestParams,
+		UpstreamName:  ldapUpstreamName,
+		UpstreamType:  ldapUpstreamType,
+		Nonce:         happyDownstreamNonce,
+		CSRFToken:     happyDownstreamCSRF,
+		PKCECode:      happyDownstreamPKCE,
+		FormatVersion: happyDownstreamStateVersion,
+	}
+
+	modifyHappyLDAPDecodedState := func(edit func(*oidc.UpstreamStateParamData)) *oidc.UpstreamStateParamData {
+		copyOfHappyLDAPDecodedState := *happyLDAPDecodedState
+		edit(&copyOfHappyLDAPDecodedState)
+		return &copyOfHappyLDAPDecodedState
+	}
+
+	happyActiveDirectoryDecodedState := &oidc.UpstreamStateParamData{
+		AuthParams:    happyDownstreamRequestParams,
+		UpstreamName:  activeDirectoryUpstreamName,
+		UpstreamType:  activeDirectoryUpstreamType,
+		Nonce:         happyDownstreamNonce,
+		CSRFToken:     happyDownstreamCSRF,
+		PKCECode:      happyDownstreamPKCE,
+		FormatVersion: happyDownstreamStateVersion,
+	}
+
+	happyLDAPUsername := "some-ldap-user"
+	happyLDAPUsernameFromAuthenticator := "some-mapped-ldap-username"
+	happyLDAPPassword := "some-ldap-password" //nolint:gosec
+	happyLDAPUID := "some-ldap-uid"
+	happyLDAPUserDN := "cn=foo,dn=bar"
+	happyLDAPGroups := []string{"group1", "group2", "group3"}
+	happyLDAPExtraRefreshAttribute := "some-refresh-attribute"
+	happyLDAPExtraRefreshValue := "some-refresh-attribute-value"
+
+	parsedUpstreamLDAPURL, err := url.Parse(upstreamLDAPURL)
+	require.NoError(t, err)
+
+	ldapAuthenticateFunc := func(ctx context.Context, username, password string) (*authenticators.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 &authenticators.Response{
+				User: &user.DefaultInfo{
+					Name:   happyLDAPUsernameFromAuthenticator,
+					UID:    happyLDAPUID,
+					Groups: happyLDAPGroups,
+				},
+				DN: happyLDAPUserDN,
+				ExtraRefreshAttributes: map[string]string{
+					happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue,
+				},
+			}, true, nil
+		}
+		return nil, false, nil
+	}
+
+	upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{
+		Name:             ldapUpstreamName,
+		ResourceUID:      ldapUpstreamResourceUID,
+		URL:              parsedUpstreamLDAPURL,
+		AuthenticateFunc: ldapAuthenticateFunc,
+	}
+
+	upstreamActiveDirectoryIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{
+		Name:             activeDirectoryUpstreamName,
+		ResourceUID:      activeDirectoryUpstreamResourceUID,
+		URL:              parsedUpstreamLDAPURL,
+		AuthenticateFunc: ldapAuthenticateFunc,
+	}
+
+	erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{
+		Name:        ldapUpstreamName,
+		ResourceUID: ldapUpstreamResourceUID,
+		AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticators.Response, bool, error) {
+			return nil, false, fmt.Errorf("some ldap upstream auth error")
+		},
+	}
+
+	expectedHappyActiveDirectoryUpstreamCustomSession := &psession.CustomSessionData{
+		ProviderUID:  activeDirectoryUpstreamResourceUID,
+		ProviderName: activeDirectoryUpstreamName,
+		ProviderType: psession.ProviderTypeActiveDirectory,
+		OIDC:         nil,
+		LDAP:         nil,
+		ActiveDirectory: &psession.ActiveDirectorySessionData{
+			UserDN:                 happyLDAPUserDN,
+			ExtraRefreshAttributes: map[string]string{happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue},
+		},
+	}
+
+	expectedHappyLDAPUpstreamCustomSession := &psession.CustomSessionData{
+		ProviderUID:  ldapUpstreamResourceUID,
+		ProviderName: ldapUpstreamName,
+		ProviderType: psession.ProviderTypeLDAP,
+		OIDC:         nil,
+		LDAP: &psession.LDAPSessionData{
+			UserDN:                 happyLDAPUserDN,
+			ExtraRefreshAttributes: map[string]string{happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue},
+		},
+		ActiveDirectory: nil,
+	}
+
+	// Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it
+	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState
+
+	happyUsernamePasswordFormParams := url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{happyLDAPPassword}}
+
+	encodeQuery := func(query map[string]string) string {
+		values := url.Values{}
+		for k, v := range query {
+			values[k] = []string{v}
+		}
+		return values.Encode()
+	}
+
+	urlWithQuery := func(baseURL string, query map[string]string) string {
+		urlToReturn := fmt.Sprintf("%s?%s", baseURL, encodeQuery(query))
+		_, err := url.Parse(urlToReturn)
+		require.NoError(t, err, "urlWithQuery helper was used to create an illegal URL")
+		return urlToReturn
+	}
+
+	tests := []struct {
+		name         string
+		idps         *oidctestutil.UpstreamIDPListerBuilder
+		decodedState *oidc.UpstreamStateParamData
+		formParams   url.Values
+		reqURIQuery  url.Values
+
+		wantStatus      int
+		wantContentType string
+		wantBodyString  string
+		wantErr         string
+
+		// Assertion that the response should be a redirect to the login page with an error param.
+		wantRedirectToLoginPageError string
+
+		// Assertions for when an authcode should be returned, i.e. the request was authenticated by an
+		// upstream LDAP or AD provider.
+		wantRedirectLocationRegexp        string // for loose matching
+		wantRedirectLocationString        string // for exact matching instead
+		wantBodyFormResponseRegexp        string // for form_post html page matching instead
+		wantDownstreamRedirectURI         string
+		wantDownstreamGrantedScopes       []string
+		wantDownstreamIDTokenSubject      string
+		wantDownstreamIDTokenUsername     string
+		wantDownstreamIDTokenGroups       []string
+		wantDownstreamRequestedScopes     []string
+		wantDownstreamPKCEChallenge       string
+		wantDownstreamPKCEChallengeMethod string
+		wantDownstreamNonce               string
+		wantDownstreamCustomSessionData   *psession.CustomSessionData
+
+		// 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).
+		wantUnnecessaryStoredRecords int
+	}{
+		{
+			name: "happy LDAP login",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one
+				WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider),
+			decodedState:                      happyLDAPDecodedState,
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy AD login",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithLDAP(&erroringUpstreamLDAPIdentityProvider).
+				WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // should pick this one
+			decodedState:                      happyActiveDirectoryDecodedState,
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyActiveDirectoryUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP login when downstream response_mode=form_post returns 200 with HTML+JS form",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["response_mode"] = []string{"form_post"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:      happyUsernamePasswordFormParams,
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBodyFormResponseRegexp: `(?s)<html.*<script>.*To finish logging in, paste this authorization code` +
+				`.*<form>.*<code id="manual-auth-code">(.+)</code>.*</html>`, // "(?s)" means match "." across newlines
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP login when downstream redirect uri matches what is configured for client except for the port number",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["redirect_uri"] = []string{"http://127.0.0.1:4242/callback"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         "http://127.0.0.1:4242/callback",
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP login when there are additional allowed downstream requested scopes",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["scope"] = []string{"openid offline_access pinniped:request-audience"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+pinniped%3Arequest-audience&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "pinniped:request-audience"},
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "pinniped:request-audience"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP when downstream OIDC validations are skipped because the openid scope was not requested",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["scope"] = []string{"email"}
+				// The following prompt value is illegal when openid is requested, but note that openid is not requested.
+				query["prompt"] = []string{"none login"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyDownstreamState, // no scopes granted
+			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,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name:                         "bad username LDAP login",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			formParams:                   url.Values{userParam: []string{"wrong!"}, passParam: []string{happyLDAPPassword}},
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: badUserPassErrParamValue,
+		},
+		{
+			name:                         "bad password LDAP login",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			formParams:                   url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{"wrong!"}},
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: badUserPassErrParamValue,
+		},
+		{
+			name:                         "blank username LDAP login",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			formParams:                   url.Values{userParam: []string{""}, passParam: []string{happyLDAPPassword}},
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: badUserPassErrParamValue,
+		},
+		{
+			name:                         "blank password LDAP login",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			formParams:                   url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{""}},
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: badUserPassErrParamValue,
+		},
+		{
+			name:                         "username and password sent as URI query params should be ignored since they are expected in form post body",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			reqURIQuery:                  happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: badUserPassErrParamValue,
+		},
+		{
+			name:                         "error during upstream LDAP authentication",
+			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider),
+			decodedState:                 happyLDAPDecodedState,
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectToLoginPageError: internalErrParamValue,
+		},
+		{
+			name: "downstream redirect uri does not match what is configured for client",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["redirect_uri"] = []string{"http://127.0.0.1/wrong_callback"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "downstream client does not exist",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["client_id"] = []string{"wrong_client_id"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "downstream client is missing",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				delete(query, "client_id")
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "response type is unsupported",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["response_type"] = []string{"unsupported"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "response type is missing",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				delete(query, "response_type")
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "PKCE code_challenge is missing",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				delete(query, "code_challenge")
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name: "PKCE code_challenge_method is invalid",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["code_challenge_method"] = []string{"this-is-not-a-valid-pkce-alg"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name: "PKCE code_challenge_method is `plain`",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["code_challenge_method"] = []string{"plain"} // plain is not allowed
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name: "PKCE code_challenge_method is missing",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				delete(query, "code_challenge_method")
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name: "prompt param is not allowed to have none and another legal value at the same time",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["prompt"] = []string{"none login"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
+			wantUnnecessaryStoredRecords: 1, // fosite already stored the authcode before it noticed the error
+		},
+		{
+			name: "downstream state does not have enough entropy",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["state"] = []string{"short"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "downstream scopes do not match what is configured for client",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				query := copyOfHappyDownstreamRequestParamsQuery()
+				query["scope"] = []string{"openid offline_access pinniped:request-audience scope_not_allowed"}
+				data.AuthParams = query.Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name:         "no upstream providers are configured or provider cannot be found by name",
+			idps:         oidctestutil.NewUpstreamIDPListerBuilder(), // empty
+			decodedState: happyLDAPDecodedState,
+			formParams:   happyUsernamePasswordFormParams,
+			wantErr:      "error finding upstream provider: provider not found",
+		},
+		{
+			name:         "upstream provider cannot be found by name and type",
+			idps:         oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			decodedState: happyActiveDirectoryDecodedState, // correct upstream IDP name, but wrong upstream IDP type
+			formParams:   happyUsernamePasswordFormParams,
+			wantErr:      "error finding upstream provider: provider not found",
+		},
+	}
+
+	for _, test := range tests {
+		tt := test
+
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			kubeClient := fake.NewSimpleClientset()
+			secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+
+			// Configure fosite the same way that the production code would.
+			// 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)
+			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()
+			oauthHelper := oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
+
+			req := httptest.NewRequest(http.MethodPost, "/ignored", strings.NewReader(tt.formParams.Encode()))
+			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			if tt.reqURIQuery != nil {
+				req.URL.RawQuery = tt.reqURIQuery.Encode()
+			}
+
+			rsp := httptest.NewRecorder()
+
+			subject := NewPostHandler(downstreamIssuer, tt.idps.Build(), oauthHelper)
+
+			err := subject(rsp, req, happyEncodedUpstreamState, tt.decodedState)
+			if tt.wantErr != "" {
+				require.EqualError(t, err, tt.wantErr)
+				require.Empty(t, kubeClient.Actions())
+				return // the http response doesn't matter when the function returns an error, because the caller should handle the error
+			}
+			// Otherwise, expect no error.
+			require.NoError(t, err)
+
+			require.Equal(t, tt.wantStatus, rsp.Code)
+			testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), tt.wantContentType)
+
+			actualLocation := rsp.Header().Get("Location")
+
+			switch {
+			case tt.wantRedirectLocationRegexp != "":
+				// Expecting a success redirect to the client.
+				require.Equal(t, tt.wantBodyString, rsp.Body.String())
+				require.Len(t, rsp.Header().Values("Location"), 1)
+				oidctestutil.RequireAuthCodeRegexpMatch(
+					t,
+					actualLocation,
+					tt.wantRedirectLocationRegexp,
+					kubeClient,
+					secretsClient,
+					kubeOauthStore,
+					tt.wantDownstreamGrantedScopes,
+					tt.wantDownstreamIDTokenSubject,
+					tt.wantDownstreamIDTokenUsername,
+					tt.wantDownstreamIDTokenGroups,
+					tt.wantDownstreamRequestedScopes,
+					tt.wantDownstreamPKCEChallenge,
+					tt.wantDownstreamPKCEChallengeMethod,
+					tt.wantDownstreamNonce,
+					downstreamClientID,
+					tt.wantDownstreamRedirectURI,
+					tt.wantDownstreamCustomSessionData,
+				)
+			case tt.wantRedirectToLoginPageError != "":
+				// Expecting an error redirect to the login UI page.
+				require.Equal(t, tt.wantBodyString, rsp.Body.String())
+				expectedLocation := downstreamIssuer + oidc.PinnipedLoginPath +
+					"?err=" + tt.wantRedirectToLoginPageError + "&state=" + happyEncodedUpstreamState
+				require.Equal(t, expectedLocation, actualLocation)
+				require.Len(t, kubeClient.Actions(), tt.wantUnnecessaryStoredRecords)
+			case tt.wantRedirectLocationString != "":
+				// Expecting an error redirect to the client.
+				require.Equal(t, tt.wantBodyString, rsp.Body.String())
+				require.Equal(t, tt.wantRedirectLocationString, actualLocation)
+				require.Len(t, kubeClient.Actions(), tt.wantUnnecessaryStoredRecords)
+			case tt.wantBodyFormResponseRegexp != "":
+				// Expecting the body of the response to be a html page with a form (for "response_mode=form_post").
+				_, hasLocationHeader := rsp.Header()["Location"]
+				require.False(t, hasLocationHeader)
+				oidctestutil.RequireAuthCodeRegexpMatch(
+					t,
+					rsp.Body.String(),
+					tt.wantBodyFormResponseRegexp,
+					kubeClient,
+					secretsClient,
+					kubeOauthStore,
+					tt.wantDownstreamGrantedScopes,
+					tt.wantDownstreamIDTokenSubject,
+					tt.wantDownstreamIDTokenUsername,
+					tt.wantDownstreamIDTokenGroups,
+					tt.wantDownstreamRequestedScopes,
+					tt.wantDownstreamPKCEChallenge,
+					tt.wantDownstreamPKCEChallengeMethod,
+					tt.wantDownstreamNonce,
+					downstreamClientID,
+					tt.wantDownstreamRedirectURI,
+					tt.wantDownstreamCustomSessionData,
+				)
+			default:
+				require.Failf(t, "test should have expected a redirect or form body",
+					"actual location was %q", actualLocation)
+			}
+		})
+	}
+}

internal/oidc/oidc.go

@@ -1,20 +1,30 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package oidc contains common OIDC functionality needed by Pinniped.
 package oidc
 
 import (
+	"crypto/subtle"
+	"errors"
+	"fmt"
+	"net/http"
 	"time"
 
 	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
+	"github.com/felixge/httpsnoop"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
+	errorsx "github.com/pkg/errors"
 
+	"go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
+	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/oidc/provider/formposthtml"
+	"go.pinniped.dev/internal/plog"
+	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
 	"go.pinniped.dev/pkg/oidcclient/pkce"
 )
@@ -26,13 +36,17 @@ const (
 	CallbackEndpointPath      = "/callback"
 	JWKSEndpointPath          = "/jwks.json"
 	PinnipedIDPsPathV1Alpha1  = "/v1alpha1/pinniped_identity_providers"
+	PinnipedLoginPath         = "/login"
 )
 
 const (
 	// Just in case we need to make a breaking change to the format of the upstream state param,
 	// we are including a format version number. This gives the opportunity for a future version of Pinniped
 	// to have the consumer of this format decide to reject versions that it doesn't understand.
-	UpstreamStateParamFormatVersion = "1"
+	//
+	// Version 1 was the original version.
+	// Version 2 added the UpstreamType field to the UpstreamStateParamData struct.
+	UpstreamStateParamFormatVersion = "2"
 
 	// The `name` passed to the encoder for encoding the upstream state param value. This name is short
 	// because it will be encoded into the upstream state param value and we're trying to keep that small.
@@ -93,6 +107,7 @@ type Codec interface {
 type UpstreamStateParamData struct {
 	AuthParams    string              `json:"p"`
 	UpstreamName  string              `json:"u"`
+	UpstreamType  string              `json:"t"`
 	Nonce         nonce.Nonce         `json:"n"`
 	CSRFToken     csrftoken.CSRFToken `json:"c"`
 	PKCECode      pkce.Code           `json:"k"`
@@ -295,3 +310,171 @@ func ScopeWasRequested(authorizeRequester fosite.AuthorizeRequester, scopeName s
 	}
 	return false
 }
+
+func ReadStateParamAndValidateCSRFCookie(r *http.Request, cookieDecoder Decoder, stateDecoder Decoder) (string, *UpstreamStateParamData, error) {
+	csrfValue, err := readCSRFCookie(r, cookieDecoder)
+	if err != nil {
+		return "", nil, err
+	}
+
+	encodedState, decodedState, err := readStateParam(r, stateDecoder)
+	if err != nil {
+		return "", nil, err
+	}
+
+	err = validateCSRFValue(decodedState, csrfValue)
+	if err != nil {
+		return "", nil, err
+	}
+
+	return encodedState, decodedState, nil
+}
+
+func readCSRFCookie(r *http.Request, cookieDecoder Decoder) (csrftoken.CSRFToken, error) {
+	receivedCSRFCookie, err := r.Cookie(CSRFCookieName)
+	if err != nil {
+		// Error means that the cookie was not found
+		return "", httperr.Wrap(http.StatusForbidden, "CSRF cookie is missing", err)
+	}
+
+	var csrfFromCookie csrftoken.CSRFToken
+	err = cookieDecoder.Decode(CSRFCookieEncodingName, receivedCSRFCookie.Value, &csrfFromCookie)
+	if err != nil {
+		return "", httperr.Wrap(http.StatusForbidden, "error reading CSRF cookie", err)
+	}
+
+	return csrfFromCookie, nil
+}
+
+func readStateParam(r *http.Request, stateDecoder Decoder) (string, *UpstreamStateParamData, error) {
+	encodedState := r.FormValue("state")
+
+	if encodedState == "" {
+		return "", nil, httperr.New(http.StatusBadRequest, "state param not found")
+	}
+
+	var state UpstreamStateParamData
+	if err := stateDecoder.Decode(
+		UpstreamStateParamEncodingName,
+		r.FormValue("state"),
+		&state,
+	); err != nil {
+		return "", nil, httperr.New(http.StatusBadRequest, "error reading state")
+	}
+
+	if state.FormatVersion != UpstreamStateParamFormatVersion {
+		return "", nil, httperr.New(http.StatusUnprocessableEntity, "state format version is invalid")
+	}
+
+	return encodedState, &state, nil
+}
+
+func validateCSRFValue(state *UpstreamStateParamData, csrfCookieValue csrftoken.CSRFToken) error {
+	if subtle.ConstantTimeCompare([]byte(state.CSRFToken), []byte(csrfCookieValue)) != 1 {
+		return httperr.New(http.StatusForbidden, "CSRF value does not match")
+	}
+	return nil
+}
+
+// FindUpstreamIDPByNameAndType finds the requested IDP by name and type, or returns an error.
+// Note that AD and LDAP IDPs both return the same interface type, but different ProviderTypes values.
+func FindUpstreamIDPByNameAndType(
+	idpLister UpstreamIdentityProvidersLister,
+	upstreamName string,
+	upstreamType string,
+) (
+	provider.UpstreamOIDCIdentityProviderI,
+	provider.UpstreamLDAPIdentityProviderI,
+	psession.ProviderType,
+	error,
+) {
+	switch upstreamType {
+	case string(v1alpha1.IDPTypeOIDC):
+		for _, p := range idpLister.GetOIDCIdentityProviders() {
+			if p.GetName() == upstreamName {
+				return p, nil, psession.ProviderTypeOIDC, nil
+			}
+		}
+	case string(v1alpha1.IDPTypeLDAP):
+		for _, p := range idpLister.GetLDAPIdentityProviders() {
+			if p.GetName() == upstreamName {
+				return nil, p, psession.ProviderTypeLDAP, nil
+			}
+		}
+	case string(v1alpha1.IDPTypeActiveDirectory):
+		for _, p := range idpLister.GetActiveDirectoryIdentityProviders() {
+			if p.GetName() == upstreamName {
+				return nil, p, psession.ProviderTypeActiveDirectory, nil
+			}
+		}
+	}
+	return nil, nil, "", errors.New("provider not found")
+}
+
+// WriteAuthorizeError writes an authorization error as it should be returned by the authorization endpoint and other
+// similar endpoints that are the end of the downstream authcode flow. Errors responses are written in the usual fosite style.
+func WriteAuthorizeError(w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester, err error, isBrowserless bool) {
+	if plog.Enabled(plog.LevelTrace) {
+		// When trace level logging is enabled, include the stack trace in the log message.
+		keysAndValues := FositeErrorForLog(err)
+		errWithStack := errorsx.WithStack(err)
+		keysAndValues = append(keysAndValues, "errWithStack")
+		// klog always prints error values using %s, which does not include stack traces,
+		// so convert the error to a string which includes the stack trace here.
+		keysAndValues = append(keysAndValues, fmt.Sprintf("%+v", errWithStack))
+		plog.Trace("authorize response error", keysAndValues...)
+	} else {
+		plog.Info("authorize response error", FositeErrorForLog(err)...)
+	}
+	if isBrowserless {
+		w = rewriteStatusSeeOtherToStatusFoundForBrowserless(w)
+	}
+	// Return an error according to OIDC spec 3.1.2.6 (second paragraph).
+	oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
+}
+
+// PerformAuthcodeRedirect successfully completes a downstream login by creating a session and
+// writing the authcode redirect response as it should be returned by the authorization endpoint and other
+// similar endpoints that are the end of the downstream authcode flow.
+func PerformAuthcodeRedirect(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	authorizeRequester fosite.AuthorizeRequester,
+	openIDSession *psession.PinnipedSession,
+	isBrowserless bool,
+) {
+	authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
+	if err != nil {
+		plog.WarningErr("error while generating and saving authcode", err)
+		WriteAuthorizeError(w, oauthHelper, authorizeRequester, err, isBrowserless)
+		return
+	}
+	if isBrowserless {
+		w = rewriteStatusSeeOtherToStatusFoundForBrowserless(w)
+	}
+	oauthHelper.WriteAuthorizeResponse(w, authorizeRequester, authorizeResponder)
+}
+
+func rewriteStatusSeeOtherToStatusFoundForBrowserless(w http.ResponseWriter) http.ResponseWriter {
+	// rewrite http.StatusSeeOther to http.StatusFound for backwards compatibility with old pinniped CLIs.
+	// we can drop this in a few releases once we feel enough time has passed for users to update.
+	//
+	// WriteAuthorizeResponse/WriteAuthorizeError calls used to result in http.StatusFound until
+	// https://github.com/ory/fosite/pull/636 changed it to http.StatusSeeOther to address
+	// https://tools.ietf.org/id/draft-ietf-oauth-security-topics-18.html#section-4.11
+	// Safari has the bad behavior in the case of http.StatusFound and not just http.StatusTemporaryRedirect.
+	//
+	// in the browserless flows, the OAuth client is the pinniped CLI and it already has access to the user's
+	// password.  Thus there is no security issue with using http.StatusFound vs. http.StatusSeeOther.
+	return httpsnoop.Wrap(w, httpsnoop.Hooks{
+		WriteHeader: func(delegate httpsnoop.WriteHeaderFunc) httpsnoop.WriteHeaderFunc {
+			return func(code int) {
+				if code == http.StatusSeeOther {
+					code = http.StatusFound
+				}
+				delegate(code)
+			}
+		},
+	})
+}

internal/oidc/provider/csp/csp.go

@@ -0,0 +1,15 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package csp defines helpers related to HTML Content Security Policies.
+package csp
+
+import (
+	"crypto/sha256"
+	"encoding/base64"
+)
+
+func Hash(s string) string {
+	hashBytes := sha256.Sum256([]byte(s))
+	return "sha256-" + base64.StdEncoding.EncodeToString(hashBytes[:])
+}

internal/oidc/provider/csp/csp_test.go

@@ -0,0 +1,15 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package csp
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestHash(t *testing.T) {
+	// Example test vector from https://content-security-policy.com/hash/.
+	require.Equal(t, "sha256-RFWPLDbv2BY+rCkDzsE+0fr8ylGr2R2faWMhq4lfEQc=", Hash("doSomething();"))
+}

internal/oidc/provider/formposthtml/formposthtml.go

@@ -6,23 +6,23 @@
 package formposthtml
 
 import (
-	"crypto/sha256"
 	_ "embed" // Needed to trigger //go:embed directives below.
-	"encoding/base64"
 	"html/template"
 	"strings"
 
 	"github.com/tdewolff/minify/v2/minify"
+
+	"go.pinniped.dev/internal/oidc/provider/csp"
 )
 
 var (
 	//go:embed form_post.css
 	rawCSS      string
-	minifiedCSS = mustMinify(minify.CSS(rawCSS))
+	minifiedCSS = panicOnError(minify.CSS(rawCSS))
 
 	//go:embed form_post.js
 	rawJS      string
-	minifiedJS = mustMinify(minify.JS(rawJS))
+	minifiedJS = panicOnError(minify.JS(rawJS))
 
 	//go:embed form_post.gohtml
 	rawHTMLTemplate string
@@ -37,28 +37,23 @@ var parsedHTMLTemplate = template.Must(template.New("form_post.gohtml").Funcs(te
 // Generate the CSP header value once since it's effectively constant.
 var cspValue = strings.Join([]string{
 	`default-src 'none'`,
-	`script-src '` + cspHash(minifiedJS) + `'`,
+	`script-src '` + csp.Hash(minifiedJS) + `'`,
-	`style-src '` + cspHash(minifiedCSS) + `'`,
+	`style-src '` + csp.Hash(minifiedCSS) + `'`,
 	`img-src data:`,
 	`connect-src *`,
 	`frame-ancestors 'none'`,
 }, "; ")
 
-func mustMinify(s string, err error) string {
+func panicOnError(s string, err error) string {
 	if err != nil {
 		panic(err)
 	}
 	return s
 }
 
-func cspHash(s string) string {
-	hashBytes := sha256.Sum256([]byte(s))
-	return "sha256-" + base64.StdEncoding.EncodeToString(hashBytes[:])
-}
-
 // ContentSecurityPolicy returns the Content-Security-Policy header value to make the Template() operate correctly.
 //
-// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src#:~:text=%27%3Chash-algorithm%3E-%3Cbase64-value%3E%27.
+// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy.
 func ContentSecurityPolicy() string { return cspValue }
 
 // Template returns the html/template.Template for rendering the response_type=form_post response page.

internal/oidc/provider/formposthtml/formposthtml_test.go

@@ -93,10 +93,6 @@ func TestContentSecurityPolicyHashes(t *testing.T) {
 }
 
 func TestHelpers(t *testing.T) {
-	// These are silly tests but it's easy to we might as well have them.
+	require.Equal(t, "test", panicOnError("test", nil))
-	require.Equal(t, "test", mustMinify("test", nil))
+	require.PanicsWithError(t, "some error", func() { panicOnError("", fmt.Errorf("some error")) })
-	require.PanicsWithError(t, "some error", func() { mustMinify("", fmt.Errorf("some error")) })
-
-	// Example test vector from https://content-security-policy.com/hash/.
-	require.Equal(t, "sha256-RFWPLDbv2BY+rCkDzsE+0fr8ylGr2R2faWMhq4lfEQc=", cspHash("doSomething();"))
 }

internal/oidc/provider/manager/manager.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package manager
@@ -18,6 +18,7 @@ import (
 	"go.pinniped.dev/internal/oidc/dynamiccodec"
 	"go.pinniped.dev/internal/oidc/idpdiscovery"
 	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/oidc/login"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/oidc/token"
 	"go.pinniped.dev/internal/plog"
@@ -134,6 +135,13 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs
 			oauthHelperWithKubeStorage,
 		)
 
+		m.providerHandlers[(issuerHostWithPath + oidc.PinnipedLoginPath)] = login.NewHandler(
+			upstreamStateEncoder,
+			csrfCookieEncoder,
+			login.NewGetHandler(incomingProvider.IssuerPath()+oidc.PinnipedLoginPath),
+			login.NewPostHandler(issuer, m.upstreamIDPs, oauthHelperWithKubeStorage),
+		)
+
 		plog.Debug("oidc provider manager added or updated issuer", "issuer", issuer)
 	}
 }

internal/testutil/assertions.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testutil
@@ -54,9 +54,47 @@ func RequireNumberOfSecretsMatchingLabelSelector(t *testing.T, secrets v1.Secret
 	require.Len(t, storedAuthcodeSecrets.Items, expectedNumberOfSecrets)
 }
 
-func RequireSecurityHeaders(t *testing.T, response *httptest.ResponseRecorder) {
+func RequireSecurityHeadersWithFormPostPageCSPs(t *testing.T, response *httptest.ResponseRecorder) {
-	// This is a more relaxed assertion rather than an exact match, so it can cover all the CSP headers we use.
+	// Loosely confirm that the unique CSPs needed for the form_post page were used.
-	require.Contains(t, response.Header().Get("Content-Security-Policy"), "default-src 'none'")
+	cspHeader := response.Header().Get("Content-Security-Policy")
+	require.Contains(t, cspHeader, "script-src '") // loose assertion
+	require.Contains(t, cspHeader, "style-src '")  // loose assertion
+	require.Contains(t, cspHeader, "img-src data:")
+	require.Contains(t, cspHeader, "connect-src *")
+
+	// Also require all the usual security headers.
+	requireSecurityHeaders(t, response)
+}
+
+func RequireSecurityHeadersWithLoginPageCSPs(t *testing.T, response *httptest.ResponseRecorder) {
+	// Loosely confirm that the unique CSPs needed for the login page were used.
+	cspHeader := response.Header().Get("Content-Security-Policy")
+	require.Contains(t, cspHeader, "style-src '")      // loose assertion
+	require.NotContains(t, cspHeader, "script-src")    // only needed by form_post page
+	require.NotContains(t, cspHeader, "img-src data:") // only needed by form_post page
+	require.NotContains(t, cspHeader, "connect-src *") // only needed by form_post page
+
+	// Also require all the usual security headers.
+	requireSecurityHeaders(t, response)
+}
+
+func RequireSecurityHeadersWithoutCustomCSPs(t *testing.T, response *httptest.ResponseRecorder) {
+	// Confirm that the unique CSPs needed for the form_post or login page were NOT used.
+	cspHeader := response.Header().Get("Content-Security-Policy")
+	require.NotContains(t, cspHeader, "script-src")
+	require.NotContains(t, cspHeader, "style-src")
+	require.NotContains(t, cspHeader, "img-src data:")
+	require.NotContains(t, cspHeader, "connect-src *")
+
+	// Also require all the usual security headers.
+	requireSecurityHeaders(t, response)
+}
+
+func requireSecurityHeaders(t *testing.T, response *httptest.ResponseRecorder) {
+	// Loosely confirm that the generic default CSPs were used.
+	cspHeader := response.Header().Get("Content-Security-Policy")
+	require.Contains(t, cspHeader, "default-src 'none'")
+	require.Contains(t, cspHeader, "frame-ancestors 'none'")
 
 	require.Equal(t, "DENY", response.Header().Get("X-Frame-Options"))
 	require.Equal(t, "1; mode=block", response.Header().Get("X-XSS-Protection"))

internal/testutil/oidctestutil/oidctestutil.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
+	"github.com/gorilla/securecookie"
 	"github.com/ory/fosite"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
@@ -830,12 +831,51 @@ func NewTestUpstreamOIDCIdentityProviderBuilder() *TestUpstreamOIDCIdentityProvi
 type ExpectedUpstreamStateParamFormat struct {
 	P string `json:"p"`
 	U string `json:"u"`
+	T string `json:"t"`
 	N string `json:"n"`
 	C string `json:"c"`
 	K string `json:"k"`
 	V string `json:"v"`
 }
 
+type UpstreamStateParamBuilder ExpectedUpstreamStateParamFormat
+
+func (b UpstreamStateParamBuilder) Build(t *testing.T, stateEncoder *securecookie.SecureCookie) string {
+	state, err := stateEncoder.Encode("s", b)
+	require.NoError(t, err)
+	return state
+}
+
+func (b *UpstreamStateParamBuilder) WithAuthorizeRequestParams(params string) *UpstreamStateParamBuilder {
+	b.P = params
+	return b
+}
+
+func (b *UpstreamStateParamBuilder) WithNonce(nonce string) *UpstreamStateParamBuilder {
+	b.N = nonce
+	return b
+}
+
+func (b *UpstreamStateParamBuilder) WithCSRF(csrf string) *UpstreamStateParamBuilder {
+	b.C = csrf
+	return b
+}
+
+func (b *UpstreamStateParamBuilder) WithPKCE(pkce string) *UpstreamStateParamBuilder {
+	b.K = pkce
+	return b
+}
+
+func (b *UpstreamStateParamBuilder) WithUpstreamIDPType(upstreamIDPType string) *UpstreamStateParamBuilder {
+	b.T = upstreamIDPType
+	return b
+}
+
+func (b *UpstreamStateParamBuilder) WithStateVersion(version string) *UpstreamStateParamBuilder {
+	b.V = version
+	return b
+}
+
 type staticKeySet struct {
 	publicKey crypto.PublicKey
 }

site/content/docs/howto/configure-supervisor-with-activedirectory.md

@@ -27,8 +27,8 @@ Create an [ActiveDirectoryIdentityProvider](https://github.com/vmware-tanzu/pinn
 ### ActiveDirectoryIdentityProvider with default options
 
 This ActiveDirectoryIdentityProvider uses all the default configuration options.
-
+The default configuration options are documented in the
-Learn more about the default configuration [here]({{< ref "../reference/active-directory-configuration">}})
+[Active Directory configuration reference]({{< ref "../reference/active-directory-configuration">}}).
 
 ```yaml
 apiVersion: idp.supervisor.pinniped.dev/v1alpha1
@@ -41,14 +41,13 @@ spec:
   # Specify the host of the Active Directory server.
   host: "activedirectory.example.com:636"
 
-  # Specify the name of the Kubernetes Secret that contains your Active Directory
+  # Specify the name of the Kubernetes Secret that contains your Active
-  # bind account credentials. This service account will be used by the
+  # Directory bind account credentials. This service account will be
-  # Supervisor to perform LDAP user and group searches.
+  # used by the Supervisor to perform LDAP user and group searches.
   bind:
     secretName: "active-directory-bind-account"
 
 ---
-
 apiVersion: v1
 kind: Secret
 metadata:
@@ -64,6 +63,10 @@ stringData:
   password: "YOUR_PASSWORD"
 ```
 
+Note that the `metadata.name` of the ActiveDirectoryIdentityProvider resource may be visible to end users at login prompts,
+so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-active-directory` over `my-idp`.
+
 If you've saved this into a file `activedirectory.yaml`, then install it into your cluster using:
 
 ```sh
@@ -140,13 +143,16 @@ spec:
       # successful authentication.
       groupName: "dn"
 
-  # Specify the name of the Kubernetes Secret that contains your Active Directory
+  # Specify the name of the Kubernetes Secret that contains your Active
-  # bind account credentials. This service account will be used by the
+  # Directory bind account credentials. This service account will be
-  # Supervisor to perform LDAP user and group searches.
+  # used by the Supervisor to perform LDAP user and group searches.
   bind:
     secretName: "active-directory-bind-account"
 ```
 
+More information about the defaults for these configuration options can be found in
+the [Active Directory configuration reference]({{< ref "../reference/active-directory-configuration">}}).
+
 ## Next steps
 
 Next, [configure the Concierge to validate JWTs issued by the Supervisor]({{< ref "configure-concierge-supervisor-jwt" >}})!

site/content/docs/howto/configure-supervisor-with-dex.md

@@ -104,19 +104,21 @@ spec:
     # to the "username" claim in downstream tokens minted by the Supervisor.
     username: email
 
-    # Specify the name of the claim in your Dex ID token that represents the groups
+    # Specify the name of the claim in your Dex ID token that represents the
-    # that the user belongs to. This matches what you specified above
+    # groups to which the user belongs. This matches what you specified above
     # with the Groups claim filter.
     # Note that the group claims from Github are in the format of "org:team".
-    # To query for the group scope, you should set the organization you want Dex to
+    # To query for the group scope, you should set the organization you
-    # search against in its configuration, otherwise your group claim would be empty.
+    # want Dex to search against in its configuration, otherwise your group
-    # An example config can be found at - https://dexidp.io/docs/connectors/github/#configuration
+    # claim would be empty. An example config can be found at
+    # https://dexidp.io/docs/connectors/github/#configuration
     groups: groups
 
   # Specify the name of the Kubernetes Secret that contains your Dex
   # application's client credentials (created below).
   client:
     secretName: dex-client-credentials
+
 ---
 apiVersion: v1
 kind: Secret
@@ -125,13 +127,19 @@ metadata:
   name: dex-client-credentials
 type: secrets.pinniped.dev/oidc-client
 stringData:
-  # The "Client ID" that you set in Dex. For example, in our case this is "pinniped-supervisor"
+  # The "Client ID" that you set in Dex. For example, in our case
+  # this is "pinniped-supervisor".
   clientID: "<your-client-id>"
 
-  # The "Client secret" that you set in Dex. For example, in our case this is "pinniped-supervisor-secret"
+  # The "Client secret" that you set in Dex. For example, in our
+  # case this is "pinniped-supervisor-secret".
   clientSecret: "<your-client-secret>"
 ```
 
+Note that the `metadata.name` of the OIDCIdentityProvider resource may be visible to end users at login prompts
+if you choose to enable `allowPasswordGrant`, so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-ldap` over `my-idp`.
+
 Once your OIDCIdentityProvider resource has been created, you can validate your configuration by running:
 
 ```bash

site/content/docs/howto/configure-supervisor-with-gitlab.md

@@ -89,6 +89,7 @@ spec:
   # application's client credentials (created below).
   client:
     secretName: gitlab-client-credentials
+
 ---
 apiVersion: v1
 kind: Secret
@@ -105,6 +106,10 @@ stringData:
   clientSecret: "<your-client-secret>"
 ```
 
+Note that the `metadata.name` of the OIDCIdentityProvider resource may be visible to end users at login prompts
+if you choose to enable `allowPasswordGrant`, so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-gitlab` over `my-idp`.
+
 Once your OIDCIdentityProvider has been created, you can validate your configuration by running:
 
 ```shell

site/content/docs/howto/configure-supervisor-with-jumpcloudldap.md

@@ -120,7 +120,6 @@ spec:
     secretName: "jumpcloudldap-bind-account"
 
 ---
-
 apiVersion: v1
 kind: Secret
 metadata:
@@ -138,6 +137,10 @@ stringData:
   password: "YOUR_PASSWORD"
 ```
 
+Note that the `metadata.name` of the LDAPIdentityProvider resource may be visible to end users at login prompts,
+so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-ldap` over `my-idp`.
+
 If you've saved this into a file `jumpcloud.yaml`, then install it into your cluster using:
 
 ```sh

site/content/docs/howto/configure-supervisor-with-okta.md

@@ -97,6 +97,7 @@ spec:
   # application's client credentials (created below).
   client:
     secretName: okta-client-credentials
+
 ---
 apiVersion: v1
 kind: Secret
@@ -113,6 +114,10 @@ stringData:
   clientSecret: "<your-client-secret>"
 ```
 
+Note that the `metadata.name` of the OIDCIdentityProvider resource may be visible to end users at login prompts
+if you choose to enable `allowPasswordGrant`, so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-okta` over `my-idp`.
+
 Once your OIDCIdentityProvider has been created, you can validate your configuration by running:
 
 ```shell

site/content/docs/howto/configure-supervisor-with-openldap.md

@@ -158,6 +158,7 @@ spec:
         - name: certs
           secret:
             secretName: certs
+
 ---
 apiVersion: v1
 kind: Service
@@ -265,7 +266,6 @@ spec:
     secretName: openldap-bind-account
 
 ---
-
 apiVersion: v1
 kind: Secret
 metadata:
@@ -284,6 +284,10 @@ stringData:
 EOF
 ```
 
+Note that the `metadata.name` of the LDAPIdentityProvider resource may be visible to end users at login prompts,
+so choose a name which will be understood by your end users.
+For example, if you work at Acme Corp, choose something like `acme-corporate-ldap` over `my-idp`.
+
 Once your LDAPIdentityProvider has been created, you can validate your configuration by running:
 
 ```sh

site/content/docs/howto/configure-supervisor-with-workspace_one_access.md

@@ -76,7 +76,8 @@ spec:
     # the default claims in your token. The "openid" scope is always
     # included.
     #
-    # See the example claims below to learn how to customize the claims returned.
+    # See the example claims below to learn how to customize the
+    # claims returned.
     additionalScopes: [group, email]
 
   # Specify how Workspace ONE Access claims are mapped to Kubernetes identities.
@@ -85,22 +86,22 @@ spec:
     # Specify the name of the claim in your Workspace ONE Access token that
     # will be mapped to the username in your Kubernetes environment.
     #
-    # User's emails can change. Use the sub claim if 
+    # User's emails can change. Use the sub claim if your environment
-    # your environment requires a stable identifier.
+    # requires a stable identifier.
     username: email
 
-    # Specify the name of the claim in Workspace ONE Access that represents the
+    # Specify the name of the claim in Workspace ONE Access that represents
-    # groups the user belongs to.
+    # the groups to which the user belongs.
     #
-    # Group names may not be unique and can change.
+    # Group names may not be unique and can change. The group_ids claim is
-    # The group_ids claim is recommended for environments
+    # recommended for environments that want to use a more stable identifier.
-    # that want to use a more stable identifier.
     groups: group_names
 
   # Specify the name of the Kubernetes Secret that contains your
   # Workspace ONE Access application's client credentials (created below).
   client:
     secretName: ws1-client-credentials
+
 ---
 apiVersion: v1
 kind: Secret

site/content/docs/howto/configure-supervisor.md

@@ -244,6 +244,6 @@ should be signed by a certificate authority that is trusted by their browsers.
 ## Next steps
 
 Next, configure an OIDCIdentityProvider, ActiveDirectoryIdentityProvider, or an LDAPIdentityProvider for the Supervisor
-(several examples are available in these guides),
+(several examples are available in these guides). Then
-and [configure the Concierge to use the Supervisor for authentication]({{< ref "configure-concierge-supervisor-jwt" >}})
+[configure the Concierge to use the Supervisor for authentication]({{< ref "configure-concierge-supervisor-jwt" >}})
 on each cluster!

site/content/docs/howto/login.md

@@ -72,6 +72,9 @@ pinniped get kubeconfig \
 The new Pinniped-compatible kubeconfig YAML will be output as stdout, and can be redirected to a file.
 
 Various default behaviors of `pinniped get kubeconfig` can be overridden using [its command-line options]({{< ref "cli" >}}).
+One flag of note is `--upstream-identity-provider-flow browser_authcode` to choose end-user `kubectl` login via a web browser
+(the default for OIDCIdentityProviders), and `--upstream-identity-provider-flow cli_password` to choose end-user `kubectl`
+login via CLI username/password prompts (the default for LDAPIdentityProviders and ActiveDirectoryIdentityProviders).
 
 ## Use the generated kubeconfig with `kubectl` to access the cluster
 
@@ -94,20 +97,33 @@ to authenticate the user to the cluster.
 If the Pinniped Supervisor is used for authentication to that cluster, then the user's authentication experience
 will depend on which type of identity provider was configured.
 
-- For an OIDC identity provider, there are two supported client flows.
+- For an OIDC identity provider, there are two supported client flows:
 
-  When using the default browser-based flow, `kubectl` will open the user's web browser and direct it to the login page of
+  1. When using the default browser-based flow, `kubectl` will open the user's web browser and direct it to the login page of
   their OIDC Provider. This login flow is controlled by the provider, so it may include two-factor authentication or
   other features provided by the OIDC Provider. If the user's browser is not available, then `kubectl` will instead
   print a URL which can be visited in a browser (potentially on a different computer) to complete the authentication.
 
-  When using the optional CLI-based flow, `kubectl` will interactively prompt the user for their username and password at the CLI.
+  2. When using the optional CLI-based flow, `kubectl` will interactively prompt the user for their username and password at the CLI.
+  Alternatively, the user can set the environment variables `PINNIPED_USERNAME` and `PINNIPED_PASSWORD` for the
+  `kubectl` process to avoid the interactive prompts. Note that the optional CLI-based flow must be enabled by the
+  administrator in the OIDCIdentityProvider configuration before use
+  (see `allowPasswordGrant` in the
+  [API docs](https://github.com/vmware-tanzu/pinniped/blob/main/generated/{{< latestcodegenversion >}}/README.adoc#oidcauthorizationconfig)
+  for more details).
+
+- For LDAP and Active Directory identity providers, there are also two supported client flows:
+
+  1. When using the default CLI-based flow, `kubectl` will interactively prompt the user for their username and password at the CLI.
   Alternatively, the user can set the environment variables `PINNIPED_USERNAME` and `PINNIPED_PASSWORD` for the
   `kubectl` process to avoid the interactive prompts.
 
-- For an LDAP identity provider, `kubectl` will interactively prompt the user for their username and password at the CLI.
+  2. When using the optional browser-based flow, `kubectl` will open the user's web browser and direct it to a login page
-  Alternatively, the user can set the environment variables `PINNIPED_USERNAME` and `PINNIPED_PASSWORD` for the
+  hosted by the Pinniped Supervisor. When the user enters their username and password, the Supervisor will authenticate
-  `kubectl` process to avoid the interactive prompts.
+  the user using the LDAP or Active Directory provider. If the user's browser is not available, then `kubectl` will instead
+  print a URL which can be visited in a browser (potentially on a different computer) to complete the authentication.
+  Unlike the optional flow for OIDC providers described above, this optional flow does not need to be configured in
+  the LDAPIdentityProvider or ActiveDirectoryIdentityProvider resource, so it is always available for end-users.
 
 Once the user completes authentication, the `kubectl` command will automatically continue and complete the user's requested command.
 For the example above, `kubectl` would list the cluster's namespaces.
@@ -135,8 +151,14 @@ in the upstream identity provider, for example:
     --group auditors
   ```
 
-## Other notes
+## Session and credential caching by the CLI
 
-- Temporary session credentials such as ID, access, and refresh tokens are stored in:
+Temporary session credentials such as ID, access, and refresh tokens are stored in:
-    - `~/.config/pinniped/sessions.yaml` (macOS/Linux)
+  - `$HOME/.config/pinniped/sessions.yaml` (macOS/Linux)
-    - `%USERPROFILE%/.config/pinniped/sessions.yaml` (Windows).
+  - `%USERPROFILE%/.config/pinniped/sessions.yaml` (Windows).
+
+Temporary cluster credentials such mTLS client certificates are stored in:
+  - `$HOME/.config/pinniped/credentials.yaml` (macOS/Linux)
+  - `%USERPROFILE%/.config/pinniped/credentials.yaml` (Windows).
+
+Deleting the contents of these directories is equivalent to performing a client-side logout.

site/content/docs/reference/code-walkthrough.md

@@ -206,6 +206,8 @@ The per-FederationDomain endpoints are:
   See [internal/oidc/callback/callback_handler.go](https://github.com/vmware-tanzu/pinniped/blob/main/internal/oidc/callback/callback_handler.go).
 - `<issuer_path>/v1alpha1/pinniped_identity_providers` is a custom discovery endpoint for clients to learn about available upstream identity providers.
   See [internal/oidc/idpdiscovery/idp_discovery_handler.go](https://github.com/vmware-tanzu/pinniped/blob/main/internal/oidc/idpdiscovery/idp_discovery_handler.go).
+- `<issuer_path>/login` is a login UI page to support the optional browser-based login flow for LDAP and Active Directory identity providers.
+  See [internal/oidc/login/login_handler.go](https://github.com/vmware-tanzu/pinniped/blob/main/internal/oidc/login/login_handler.go).
 
 The OIDC specifications implemented by the Supervisor can be found at [openid.net](https://openid.net/connect).
 

test/integration/cli_test.go

@@ -338,7 +338,7 @@ func runPinnipedLoginOIDC(
 	require.NoError(t, page.Navigate(loginURL))
 
 	// Expect to be redirected to the upstream provider and log in.
-	browsertest.LoginToUpstream(t, page, env.CLIUpstreamOIDC)
+	browsertest.LoginToUpstreamOIDC(t, page, env.CLIUpstreamOIDC)
 
 	// Expect to be redirected to the localhost callback.
 	t.Logf("waiting for redirect to callback")

test/integration/e2e_test.go

@@ -26,6 +26,7 @@ import (
 
 	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/creack/pty"
+	"github.com/sclevine/agouti"
 	"github.com/stretchr/testify/require"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -49,7 +50,7 @@ import (
 )
 
 // TestE2EFullIntegration_Browser tests a full integration scenario that combines the supervisor, concierge, and CLI.
-func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
+func TestE2EFullIntegration_Browser(t *testing.T) {
 	env := testlib.IntegrationEnv(t)
 
 	topSetupCtx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute)
@@ -57,7 +58,6 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 
 	// Build pinniped CLI.
 	pinnipedExe := testlib.PinnipedCLIPath(t)
-	tempDir := testutil.TempDir(t)
 
 	// Infer the downstream issuer URL from the callback associated with the upstream test client registration.
 	issuerURL, err := url.Parse(env.SupervisorUpstreamOIDC.CallbackURL)
@@ -72,7 +72,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 	require.NoError(t, err)
 
 	// Save that bundle plus the one that signs the upstream issuer, for test purposes.
-	testCABundlePath := filepath.Join(tempDir, "test-ca.pem")
+	testCABundlePath := filepath.Join(testutil.TempDir(t), "test-ca.pem")
 	testCABundlePEM := []byte(string(ca.Bundle()) + "\n" + env.SupervisorUpstreamOIDC.CABundle)
 	testCABundleBase64 := base64.StdEncoding.EncodeToString(testCABundlePEM)
 	require.NoError(t, ioutil.WriteFile(testCABundlePath, testCABundlePEM, 0600))
@@ -108,10 +108,12 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 	})
 
 	// Add an OIDC upstream IDP and try using it to authenticate during kubectl commands.
-	t.Run("with Supervisor OIDC upstream IDP and automatic flow", func(t *testing.T) {
+	t.Run("with Supervisor OIDC upstream IDP and browser flow with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
 		page := browsertest.Open(t)
 
@@ -149,7 +151,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		}, idpv1alpha1.PhaseReady)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/oidc-test-sessions.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
 
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
@@ -162,90 +164,14 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		})
 
 		// Run "kubectl get namespaces" which should trigger a browser login via the plugin.
-		start := time.Now()
 		kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6")
 		kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...)
 
-		// Wrap the stdout and stderr pipes with TeeReaders which will copy each incremental read to an
+		// Run the kubectl command, wait for the Pinniped CLI to print the authorization URL, and open it in the browser.
-		// in-memory buffer, so we can have the full output available to us at the end.
+		kubectlOutputChan := startKubectlAndOpenAuthorizationURLInBrowser(testCtx, t, kubectlCmd, page)
-		originalStderrPipe, err := kubectlCmd.StderrPipe()
-		require.NoError(t, err)
-		originalStdoutPipe, err := kubectlCmd.StdoutPipe()
-		require.NoError(t, err)
-		var stderrPipeBuf, stdoutPipeBuf bytes.Buffer
-		stderrPipe := io.TeeReader(originalStderrPipe, &stderrPipeBuf)
-		stdoutPipe := io.TeeReader(originalStdoutPipe, &stdoutPipeBuf)
 
-		t.Logf("starting kubectl subprocess")
+		// Confirm that we got to the upstream IDP's login page, fill out the form, and submit the form.
-		require.NoError(t, kubectlCmd.Start())
+		browsertest.LoginToUpstreamOIDC(t, page, env.SupervisorUpstreamOIDC)
-		t.Cleanup(func() {
-			// Consume readers so that the tee buffers will contain all the output so far.
-			_, stdoutReadAllErr := readAllCtx(testCtx, stdoutPipe)
-			_, stderrReadAllErr := readAllCtx(testCtx, stderrPipe)
-
-			// Note that Wait closes the stdout/stderr pipes, so we don't need to close them ourselves.
-			waitErr := kubectlCmd.Wait()
-			t.Logf("kubectl subprocess exited with code %d", kubectlCmd.ProcessState.ExitCode())
-
-			// Upon failure, print the full output so far of the kubectl command.
-			var testAlreadyFailedErr error
-			if t.Failed() {
-				testAlreadyFailedErr = errors.New("test failed prior to clean up function")
-			}
-			cleanupErrs := utilerrors.NewAggregate([]error{waitErr, stdoutReadAllErr, stderrReadAllErr, testAlreadyFailedErr})
-
-			if cleanupErrs != nil {
-				t.Logf("kubectl stdout was:\n----start of stdout\n%s\n----end of stdout", stdoutPipeBuf.String())
-				t.Logf("kubectl stderr was:\n----start of stderr\n%s\n----end of stderr", stderrPipeBuf.String())
-			}
-			require.NoErrorf(t, cleanupErrs, "kubectl process did not exit cleanly and/or the test failed. "+
-				"Note: if kubectl's first call to the Pinniped CLI results in the Pinniped CLI returning an error, "+
-				"then kubectl may call the Pinniped CLI again, which may hang because it will wait for the user "+
-				"to finish the login. This test will kill the kubectl process after a timeout. In this case, the "+
-				" kubectl output printed above will include multiple prompts for the user to enter their authcode.",
-			)
-		})
-
-		// Start a background goroutine to read stderr from the CLI and parse out the login URL.
-		loginURLChan := make(chan string, 1)
-		spawnTestGoroutine(testCtx, t, func() error {
-			reader := bufio.NewReader(testlib.NewLoggerReader(t, "stderr", stderrPipe))
-			scanner := bufio.NewScanner(reader)
-			for scanner.Scan() {
-				loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
-				if err == nil && loginURL.Scheme == "https" {
-					loginURLChan <- loginURL.String() // this channel is buffered so this will not block
-					return nil
-				}
-			}
-			return fmt.Errorf("expected stderr to contain login URL")
-		})
-
-		// Start a background goroutine to read stdout from kubectl and return the result as a string.
-		kubectlOutputChan := make(chan string, 1)
-		spawnTestGoroutine(testCtx, t, func() error {
-			output, err := readAllCtx(testCtx, stdoutPipe)
-			if err != nil {
-				return err
-			}
-			t.Logf("kubectl output:\n%s\n", output)
-			kubectlOutputChan <- string(output) // this channel is buffered so this will not block
-			return nil
-		})
-
-		// Wait for the CLI to print out the login URL and open the browser to it.
-		t.Logf("waiting for CLI to output login URL")
-		var loginURL string
-		select {
-		case <-time.After(1 * time.Minute):
-			require.Fail(t, "timed out waiting for login URL")
-		case loginURL = <-loginURLChan:
-		}
-		t.Logf("navigating to login page: %q", loginURL)
-		require.NoError(t, page.Navigate(loginURL))
-
-		// Expect to be redirected to the upstream provider and log in.
-		browsertest.LoginToUpstream(t, page, env.SupervisorUpstreamOIDC)
 
 		// Expect to be redirected to the downstream callback which is serving the form_post HTML.
 		t.Logf("waiting for response page %s", downstream.Spec.Issuer)
@@ -255,17 +181,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		// It should now be in the "success" state.
 		formpostExpectSuccessState(t, page)
 
-		// Expect the CLI to output a list of namespaces.
+		requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan))
-		t.Logf("waiting for kubectl to output namespace list")
-		var kubectlOutput string
-		select {
-		case <-time.After(1 * time.Minute):
-			require.Fail(t, "timed out waiting for kubectl output")
-		case kubectlOutput = <-kubectlOutputChan:
-		}
-		requireKubectlGetNamespaceOutput(t, env, kubectlOutput)
-
-		t.Logf("first kubectl command took %s", time.Since(start).String())
 
 		requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env,
 			downstream,
@@ -281,6 +197,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
 		page := browsertest.Open(t)
 
@@ -318,7 +236,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		}, idpv1alpha1.PhaseReady)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/oidc-test-sessions-manual.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
 			"--concierge-api-group-suffix", env.APIGroupSuffix,
@@ -358,7 +277,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		require.NoError(t, page.Navigate(loginURL))
 
 		// Expect to be redirected to the upstream provider and log in.
-		browsertest.LoginToUpstream(t, page, env.SupervisorUpstreamOIDC)
+		browsertest.LoginToUpstreamOIDC(t, page, env.SupervisorUpstreamOIDC)
 
 		// Expect to be redirected to the downstream callback which is serving the form_post HTML.
 		t.Logf("waiting for response page %s", downstream.Spec.Issuer)
@@ -395,6 +314,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
 		page := browsertest.Open(t)
 
@@ -440,7 +361,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		}, idpv1alpha1.PhaseReady)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/oidc-test-sessions-manual.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
 			"--concierge-api-group-suffix", env.APIGroupSuffix,
@@ -486,7 +408,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		require.NoError(t, page.Navigate(loginURL))
 
 		// Expect to be redirected to the upstream provider and log in.
-		browsertest.LoginToUpstream(t, page, env.SupervisorUpstreamOIDC)
+		browsertest.LoginToUpstreamOIDC(t, page, env.SupervisorUpstreamOIDC)
 
 		// Expect to be redirected to the downstream callback which is serving the form_post HTML.
 		t.Logf("waiting for response page %s", downstream.Spec.Issuer)
@@ -534,6 +456,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		expectedUsername := env.SupervisorUpstreamOIDC.Username
 		expectedGroups := env.SupervisorUpstreamOIDC.ExpectedGroups
 
@@ -569,7 +493,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		}, idpv1alpha1.PhaseReady)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/oidc-test-sessions-password-grant.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
 			"--concierge-api-group-suffix", env.APIGroupSuffix,
@@ -620,6 +545,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		// Create upstream OIDC provider and wait for it to become ready.
 		oidcIdentityProvider := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{
 			Issuer: env.SupervisorUpstreamOIDC.Issuer,
@@ -640,7 +567,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		}, idpv1alpha1.PhaseReady)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/oidc-test-sessions-password-grant-negative-test.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
 			"--concierge-api-group-suffix", env.APIGroupSuffix,
@@ -700,6 +628,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
 			t.Skip("LDAP integration test requires connectivity to an LDAP server")
 		}
@@ -710,7 +640,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		setupClusterForEndToEndLDAPTest(t, expectedUsername, env)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/ldap-test-sessions.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
 
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
@@ -760,6 +690,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
 			t.Skip("LDAP integration test requires connectivity to an LDAP server")
 		}
@@ -770,7 +702,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		setupClusterForEndToEndLDAPTest(t, expectedUsername, env)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/ldap-test-with-env-vars-sessions.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
 
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
@@ -832,6 +764,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
 			t.Skip("Active Directory integration test requires connectivity to an LDAP server")
 		}
@@ -845,7 +779,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/ad-test-sessions.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
 
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
@@ -895,6 +829,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
 		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
 			t.Skip("ActiveDirectory integration test requires connectivity to an LDAP server")
 		}
@@ -909,7 +845,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 		setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env)
 
 		// Use a specific session cache for this test.
-		sessionCachePath := tempDir + "/ad-test-with-env-vars-sessions.yaml"
+		sessionCachePath := tempDir + "/test-sessions.yaml"
 
 		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
 			"get", "kubeconfig",
@@ -964,6 +900,214 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // nolint:gocyclo
 			expectedGroups,
 		)
 	})
+
+	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the browser flow.
+	t.Run("with Supervisor LDAP upstream IDP and browser flow with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
+		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+		t.Cleanup(cancel)
+
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
+		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
+		page := browsertest.Open(t)
+
+		expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
+		expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs
+
+		setupClusterForEndToEndLDAPTest(t, expectedUsername, env)
+
+		// Use a specific session cache for this test.
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
+		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
+			"get", "kubeconfig",
+			"--concierge-api-group-suffix", env.APIGroupSuffix,
+			"--concierge-authenticator-type", "jwt",
+			"--concierge-authenticator-name", authenticator.Name,
+			"--oidc-skip-browser",
+			"--oidc-ca-bundle", testCABundlePath,
+			"--upstream-identity-provider-flow", "browser_authcode",
+			"--oidc-session-cache", sessionCachePath,
+		})
+
+		// Run "kubectl get namespaces" which should trigger a browser login via the plugin.
+		kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6")
+		kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...)
+
+		// Run the kubectl command, wait for the Pinniped CLI to print the authorization URL, and open it in the browser.
+		kubectlOutputChan := startKubectlAndOpenAuthorizationURLInBrowser(testCtx, t, kubectlCmd, page)
+
+		// Confirm that we got to the Supervisor's login page, fill out the form, and submit the form.
+		browsertest.LoginToUpstreamLDAP(t, page, downstream.Spec.Issuer,
+			expectedUsername, env.SupervisorUpstreamLDAP.TestUserPassword)
+
+		formpostExpectSuccessState(t, page)
+
+		requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan))
+
+		requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env,
+			downstream,
+			kubeconfigPath,
+			sessionCachePath,
+			pinnipedExe,
+			expectedUsername,
+			expectedGroups,
+		)
+	})
+
+	// Add an Active Directory upstream IDP and try using it to authenticate during kubectl commands, using the browser flow.
+	t.Run("with Supervisor Active Directory upstream IDP and browser flow with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
+		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+		t.Cleanup(cancel)
+
+		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
+
+		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
+		page := browsertest.Open(t)
+
+		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
+			t.Skip("Active Directory integration test requires connectivity to an LDAP server")
+		}
+		if env.SupervisorUpstreamActiveDirectory.Host == "" {
+			t.Skip("Active Directory hostname not specified")
+		}
+
+		expectedUsername := env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue
+		expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames
+
+		setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env)
+
+		// Use a specific session cache for this test.
+		sessionCachePath := tempDir + "/test-sessions.yaml"
+
+		kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
+			"get", "kubeconfig",
+			"--concierge-api-group-suffix", env.APIGroupSuffix,
+			"--concierge-authenticator-type", "jwt",
+			"--concierge-authenticator-name", authenticator.Name,
+			"--oidc-skip-browser",
+			"--oidc-ca-bundle", testCABundlePath,
+			"--upstream-identity-provider-flow", "browser_authcode",
+			"--oidc-session-cache", sessionCachePath,
+		})
+
+		// Run "kubectl get namespaces" which should trigger a browser login via the plugin.
+		kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6")
+		kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...)
+
+		// Run the kubectl command, wait for the Pinniped CLI to print the authorization URL, and open it in the browser.
+		kubectlOutputChan := startKubectlAndOpenAuthorizationURLInBrowser(testCtx, t, kubectlCmd, page)
+
+		// Confirm that we got to the Supervisor's login page, fill out the form, and submit the form.
+		browsertest.LoginToUpstreamLDAP(t, page, downstream.Spec.Issuer,
+			expectedUsername, env.SupervisorUpstreamActiveDirectory.TestUserPassword)
+
+		formpostExpectSuccessState(t, page)
+
+		requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan))
+
+		requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env,
+			downstream,
+			kubeconfigPath,
+			sessionCachePath,
+			pinnipedExe,
+			expectedUsername,
+			expectedGroups,
+		)
+	})
+}
+
+func startKubectlAndOpenAuthorizationURLInBrowser(testCtx context.Context, t *testing.T, kubectlCmd *exec.Cmd, page *agouti.Page) chan string {
+	// Wrap the stdout and stderr pipes with TeeReaders which will copy each incremental read to an
+	// in-memory buffer, so we can have the full output available to us at the end.
+	originalStderrPipe, err := kubectlCmd.StderrPipe()
+	require.NoError(t, err)
+	originalStdoutPipe, err := kubectlCmd.StdoutPipe()
+	require.NoError(t, err)
+	var stderrPipeBuf, stdoutPipeBuf bytes.Buffer
+	stderrPipe := io.TeeReader(originalStderrPipe, &stderrPipeBuf)
+	stdoutPipe := io.TeeReader(originalStdoutPipe, &stdoutPipeBuf)
+
+	t.Logf("starting kubectl subprocess")
+	require.NoError(t, kubectlCmd.Start())
+	t.Cleanup(func() {
+		// Consume readers so that the tee buffers will contain all the output so far.
+		_, stdoutReadAllErr := readAllCtx(testCtx, stdoutPipe)
+		_, stderrReadAllErr := readAllCtx(testCtx, stderrPipe)
+
+		// Note that Wait closes the stdout/stderr pipes, so we don't need to close them ourselves.
+		waitErr := kubectlCmd.Wait()
+		t.Logf("kubectl subprocess exited with code %d", kubectlCmd.ProcessState.ExitCode())
+
+		// Upon failure, print the full output so far of the kubectl command.
+		var testAlreadyFailedErr error
+		if t.Failed() {
+			testAlreadyFailedErr = errors.New("test failed prior to clean up function")
+		}
+		cleanupErrs := utilerrors.NewAggregate([]error{waitErr, stdoutReadAllErr, stderrReadAllErr, testAlreadyFailedErr})
+
+		if cleanupErrs != nil {
+			t.Logf("kubectl stdout was:\n----start of stdout\n%s\n----end of stdout", stdoutPipeBuf.String())
+			t.Logf("kubectl stderr was:\n----start of stderr\n%s\n----end of stderr", stderrPipeBuf.String())
+		}
+		require.NoErrorf(t, cleanupErrs, "kubectl process did not exit cleanly and/or the test failed. "+
+			"Note: if kubectl's first call to the Pinniped CLI results in the Pinniped CLI returning an error, "+
+			"then kubectl may call the Pinniped CLI again, which may hang because it will wait for the user "+
+			"to finish the login. This test will kill the kubectl process after a timeout. In this case, the "+
+			" kubectl output printed above will include multiple prompts for the user to enter their authcode.",
+		)
+	})
+
+	// Start a background goroutine to read stderr from the CLI and parse out the login URL.
+	loginURLChan := make(chan string, 1)
+	spawnTestGoroutine(testCtx, t, func() error {
+		reader := bufio.NewReader(testlib.NewLoggerReader(t, "stderr", stderrPipe))
+		scanner := bufio.NewScanner(reader)
+		for scanner.Scan() {
+			loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
+			if err == nil && loginURL.Scheme == "https" {
+				loginURLChan <- loginURL.String() // this channel is buffered so this will not block
+				return nil
+			}
+		}
+		return fmt.Errorf("expected stderr to contain login URL")
+	})
+
+	// Start a background goroutine to read stdout from kubectl and return the result as a string.
+	kubectlOutputChan := make(chan string, 1)
+	spawnTestGoroutine(testCtx, t, func() error {
+		output, err := readAllCtx(testCtx, stdoutPipe)
+		if err != nil {
+			return err
+		}
+		t.Logf("kubectl output:\n%s\n", output)
+		kubectlOutputChan <- string(output) // this channel is buffered so this will not block
+		return nil
+	})
+
+	// Wait for the CLI to print out the login URL and open the browser to it.
+	t.Logf("waiting for CLI to output login URL")
+	var loginURL string
+	select {
+	case <-time.After(1 * time.Minute):
+		require.Fail(t, "timed out waiting for login URL")
+	case loginURL = <-loginURLChan:
+	}
+	t.Logf("navigating to login page: %q", loginURL)
+	require.NoError(t, page.Navigate(loginURL))
+
+	return kubectlOutputChan
+}
+
+func waitForKubectlOutput(t *testing.T, kubectlOutputChan chan string) string {
+	t.Logf("waiting for kubectl output")
+	var kubectlOutput string
+	select {
+	case <-time.After(1 * time.Minute):
+		require.Fail(t, "timed out waiting for kubectl output")
+	case kubectlOutput = <-kubectlOutputChan:
+	}
+	return kubectlOutput
 }
 
 func setupClusterForEndToEndLDAPTest(t *testing.T, username string, env *testlib.TestEnv) {

test/integration/supervisor_warnings_test.go

@@ -448,7 +448,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 		require.NoError(t, page.Navigate(loginURL))
 
 		// Expect to be redirected to the upstream provider and log in.
-		browsertest.LoginToUpstream(t, page, env.SupervisorUpstreamOIDC)
+		browsertest.LoginToUpstreamOIDC(t, page, env.SupervisorUpstreamOIDC)
 
 		// Expect to be redirected to the downstream callback which is serving the form_post HTML.
 		t.Logf("waiting for response page %s", downstream.Spec.Issuer)

test/testlib/browsertest/browsertest.go

@@ -112,7 +112,7 @@ func WaitForURL(t *testing.T, page *agouti.Page, pat *regexp.Regexp) {
 		func(requireEventually *require.Assertions) {
 			url, err := page.URL()
 			if url != lastURL {
-				t.Logf("saw URL %s", url)
+				t.Logf("saw URL %s", testlib.MaskTokens(url))
 				lastURL = url
 			}
 			requireEventually.NoError(err)
@@ -125,9 +125,9 @@ func WaitForURL(t *testing.T, page *agouti.Page, pat *regexp.Regexp) {
 	)
 }
 
-// LoginToUpstream expects the page to be redirected to one of several known upstream IDPs.
+// LoginToUpstreamOIDC expects the page to be redirected to one of several known upstream IDPs.
 // It knows how to enter the test username/password and submit the upstream login form.
-func LoginToUpstream(t *testing.T, page *agouti.Page, upstream testlib.TestOIDCUpstream) {
+func LoginToUpstreamOIDC(t *testing.T, page *agouti.Page, upstream testlib.TestOIDCUpstream) {
 	t.Helper()
 
 	type config struct {
@@ -182,3 +182,45 @@ func LoginToUpstream(t *testing.T, page *agouti.Page, upstream testlib.TestOIDCU
 	require.NoError(t, page.First(cfg.PasswordSelector).Fill(upstream.Password))
 	require.NoError(t, page.First(cfg.LoginButtonSelector).Click())
 }
+
+// LoginToUpstreamLDAP expects the page to be redirected to the Supervisor's login UI for an LDAP/AD IDP.
+// It knows how to enter the test username/password and submit the upstream login form.
+func LoginToUpstreamLDAP(t *testing.T, page *agouti.Page, issuer, username, password string) {
+	t.Helper()
+
+	loginURLRegexp, err := regexp.Compile(`\A` + regexp.QuoteMeta(issuer+"/login") + `\?state=.+\z`)
+	require.NoError(t, err)
+
+	// Expect to be redirected to the login page.
+	t.Logf("waiting for redirect to %s/login page", issuer)
+	WaitForURL(t, page, loginURLRegexp)
+
+	// Wait for the login page to be rendered.
+	WaitForVisibleElements(t, page, "#username", "#password", "#submit")
+
+	// Fill in the username and password and click "submit".
+	SubmitUpstreamLDAPLoginForm(t, page, username, password)
+}
+
+func SubmitUpstreamLDAPLoginForm(t *testing.T, page *agouti.Page, username string, password string) {
+	t.Helper()
+
+	// Fill in the username and password and click "submit".
+	t.Logf("logging in via Supervisor's upstream LDAP/AD login UI page")
+	require.NoError(t, page.First("#username").Fill(username))
+	require.NoError(t, page.First("#password").Fill(password))
+	require.NoError(t, page.First("#submit").Click())
+}
+
+func WaitForUpstreamLDAPLoginPageWithError(t *testing.T, page *agouti.Page, issuer string) {
+	t.Helper()
+
+	// Wait for redirect back to the login page again with an error.
+	t.Logf("waiting for redirect to back to login page with error message")
+	loginURLRegexp, err := regexp.Compile(`\A` + regexp.QuoteMeta(issuer+"/login") + `\?err=login_error&state=.+\z`)
+	require.NoError(t, err)
+	WaitForURL(t, page, loginURLRegexp)
+
+	// Wait for the login page to be rendered again, this time also with an error message.
+	WaitForVisibleElements(t, page, "#username", "#password", "#submit", "#alert")
+}

test/testlib/iotest.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testlib
@@ -36,7 +36,7 @@ func (l *testlogReader) Read(p []byte) (n int, err error) {
 }
 
 // MaskTokens makes a best-effort attempt to mask out things that look like secret tokens in test output.
-// The goal is more to have readable test output than for any security reason.
+// Provides more readable test output, but also obscures sensitive state params and authcodes from public test output.
 func MaskTokens(in string) string {
 	var tokenLike = regexp.MustCompile(`(?mi)[a-zA-Z0-9._-]{30,}|[a-zA-Z0-9]{20,}`)
 	return tokenLike.ReplaceAllStringFunc(in, func(t string) string {