Show an IDP chooser UI when appropriate from authorize endpoint

internal/federationdomain/endpoints/auth/auth_handler.go

@@ -89,6 +89,19 @@ func NewHandler(
 		// oidcapi.AuthorizeUpstreamIDPTypeParamName query (or form) params to request a certain upstream IDP.
 		// The Pinniped CLI has been sending these params since v0.9.0.
 		idpNameQueryParamValue := r.Form.Get(oidcapi.AuthorizeUpstreamIDPNameParamName)
+
+		// Check if we are in a special case where we should inject an interstitial page to ask the user
+		// which IDP they would like to use.
+		if shouldShowIDPChooser(idpFinder, idpNameQueryParamValue, requestedBrowserlessFlow) {
+			// Redirect to the IDP chooser page with all the same query/form params. When the user chooses an IDP,
+			// it will redirect back to here with all the same params again, with the pinniped_idp_name param added.
+			http.Redirect(w, r,
+				fmt.Sprintf("%s%s?%s", downstreamIssuer, oidc.ChooseIDPEndpointPath, r.Form.Encode()),
+				http.StatusSeeOther,
+			)
+			return nil
+		}
+
 		oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpNameQueryParamValue, idpFinder)
 		if err != nil {
 			oidc.WriteAuthorizeError(r, w,
@@ -153,6 +166,20 @@ func NewHandler(
 	return securityheader.WrapWithCustomCSP(handler, formposthtml.ContentSecurityPolicy())
 }
 
+func shouldShowIDPChooser(
+	idpFinder federationdomainproviders.FederationDomainIdentityProvidersFinderI,
+	idpNameQueryParamValue string,
+	requestedBrowserlessFlow bool,
+) bool {
+	clientDidNotRequestSpecificIDP := len(idpNameQueryParamValue) == 0
+	clientRequestedBrowserBasedFlow := !requestedBrowserlessFlow
+	inBackwardsCompatMode := idpFinder.HasDefaultIDP()
+	federationDomainSpecHasSomeValidIDPs := idpFinder.IDPCount() > 0
+
+	return clientDidNotRequestSpecificIDP && clientRequestedBrowserBasedFlow &&
+		!inBackwardsCompatMode && federationDomainSpecHasSomeValidIDPs
+}
+
 func handleAuthRequestForLDAPUpstreamCLIFlow(
 	r *http.Request,
 	w http.ResponseWriter,

internal/federationdomain/endpoints/auth/auth_handler_test.go

@@ -731,6 +731,25 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name: "with multiple IDPs available, request does not choose which IDP to use",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).
+				WithLDAP(upstreamLDAPIdentityProviderBuilder().Build()),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   happyGetRequestPath, // does not include pinniped_idp_name param
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            "", // there should not be a CSRF cookie set on the response
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/choose_identity_provider", happyGetRequestQueryMap),
+			wantUpstreamStateParamInLocationHeader: false, // it should copy the params of the original request, not add a new state param
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name: "with multiple IDPs available, request chooses to use OIDC browser flow",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().
@@ -3303,6 +3322,17 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 			wantContentType: plainContentType,
 			wantBodyString:  `{"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. 'pinniped_idp_name' param error: did not find IDP with name 'some-ldap-idp'"}`,
 		},
+		{
+			name:                 "with multiple IDPs, when using browserless flow, when pinniped_idp_name param is not specified, should be an error (browerless flows do not use IDP chooser page)",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().WithAllowPasswordGrant(true).Build()),
+			method:               http.MethodGet,
+			path:                 happyGetRequestPath,
+			customUsernameHeader: ptr.To(oidcUpstreamUsername),
+			customPasswordHeader: ptr.To(oidcUpstreamPassword),
+			wantStatus:           http.StatusBadRequest,
+			wantContentType:      plainContentType,
+			wantBodyString:       `{"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. 'pinniped_idp_name' param error: identity provider not found: this federation domain does not have a default identity provider"}`,
+		},
 		{
 			name:            "post with invalid form in the body",
 			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),

internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go

@@ -0,0 +1,77 @@
+// Copyright 2023 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package chooseidp
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"sort"
+
+	"go.pinniped.dev/generated/latest/apis/supervisor/oidc"
+	"go.pinniped.dev/internal/federationdomain/endpoints/chooseidp/chooseidphtml"
+	"go.pinniped.dev/internal/federationdomain/federationdomainproviders"
+	"go.pinniped.dev/internal/httputil/httperr"
+	"go.pinniped.dev/internal/httputil/securityheader"
+)
+
+// NewHandler returns a http.Handler that serves an IDP chooser web page. The authorization endpoint may redirect
+// to this page, copying all the same parameters from the original authorization request. Each button on this page
+// simply adds the IDP's name as an additional request parameter to the original authorization request's parameters,
+// and sends the user back to the authorization endpoint, where the authorization flow can start from scratch using
+// the original params with the extra pinniped_idp_name param added.
+func NewHandler(authURL string, upstreamIDPs federationdomainproviders.FederationDomainIdentityProvidersListerI) http.Handler {
+	handler := httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+		if r.Method != http.MethodGet {
+			return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET)", r.Method)
+		}
+
+		// This is just a sanity check that it appears to be an authorize request.
+		// Actual enforcement of parameters will happen at the authorization endpoint.
+		query := r.URL.Query()
+		if !(query.Has("client_id") && query.Has("redirect_uri") && query.Has("scope") && query.Has("response_type")) {
+			return httperr.New(http.StatusBadRequest, "missing required query params (must include client_id, redirect_uri, scope, and response_type)")
+		}
+
+		newIDPForPageData := func(displayName string) chooseidphtml.IdentityProvider {
+			return chooseidphtml.IdentityProvider{
+				DisplayName: displayName,
+				URL: fmt.Sprintf("%s?%s&%s=%s",
+					authURL, r.URL.Query().Encode(), oidc.AuthorizeUpstreamIDPNameParamName, url.QueryEscape(displayName)),
+			}
+		}
+
+		var idps []chooseidphtml.IdentityProvider
+		for _, p := range upstreamIDPs.GetOIDCIdentityProviders() {
+			idps = append(idps, newIDPForPageData(p.DisplayName))
+		}
+		for _, p := range upstreamIDPs.GetLDAPIdentityProviders() {
+			idps = append(idps, newIDPForPageData(p.DisplayName))
+		}
+		for _, p := range upstreamIDPs.GetActiveDirectoryIdentityProviders() {
+			idps = append(idps, newIDPForPageData(p.DisplayName))
+		}
+
+		sort.SliceStable(idps, func(i, j int) bool {
+			return idps[i].DisplayName < idps[j].DisplayName
+		})
+
+		if len(idps) == 0 {
+			// This shouldn't normally happen in practice because the auth endpoint would not have redirected to here.
+			return httperr.New(http.StatusInternalServerError,
+				"please check the server's configuration: no valid identity providers found for this FederationDomain")
+		}
+
+		return chooseidphtml.Template().Execute(w, &chooseidphtml.PageData{IdentityProviders: idps})
+	})
+
+	return wrapSecurityHeaders(handler)
+}
+
+func wrapSecurityHeaders(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		wrapped := securityheader.WrapWithCustomCSP(handler, chooseidphtml.ContentSecurityPolicy())
+		wrapped.ServeHTTP(w, r)
+	})
+}

internal/federationdomain/endpoints/chooseidp/choose_idp_handler_test.go

@@ -0,0 +1,150 @@
+// Copyright 2023 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package chooseidp
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/federationdomain/endpoints/chooseidp/chooseidphtml"
+	"go.pinniped.dev/internal/federationdomain/federationdomainproviders"
+	"go.pinniped.dev/internal/federationdomain/oidc"
+	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/oidctestutil"
+)
+
+func TestChooseIDPHandler(t *testing.T) {
+	const testIssuer = "https://pinniped.dev/issuer"
+
+	testReqQuery := url.Values{
+		"client_id":     []string{"foo"},
+		"redirect_uri":  []string{"bar"},
+		"scope":         []string{"baz"},
+		"response_type": []string{"bat"},
+	}
+	testIssuerWithTestReqQuery := testIssuer + "?" + testReqQuery.Encode()
+
+	tests := []struct {
+		name string
+
+		method    string
+		reqTarget string
+		idps      federationdomainproviders.FederationDomainIdentityProvidersListerI
+
+		wantStatus      int
+		wantContentType string
+		wantBodyString  string
+	}{
+		{
+			name:      "happy path",
+			method:    http.MethodGet,
+			reqTarget: "/some/path" + oidc.ChooseIDPEndpointPath + "?" + testReqQuery.Encode(),
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("oidc2").Build()).
+				WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("ldap1").Build()).
+				WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("z-ad1").Build()).
+				WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("ldap2").Build()).
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("oidc1").Build()).
+				WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName("ad2").Build()).
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusOK,
+			wantContentType: "text/html; charset=utf-8",
+			wantBodyString: testutil.ExpectedChooseIDPPageHTML(chooseidphtml.CSS(), chooseidphtml.JS(), []testutil.ChooseIDPPageExpectedValue{
+				// Should be sorted alphabetically by displayName.
+				{DisplayName: "ad2", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=ad2"},
+				{DisplayName: "ldap1", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=ldap1"},
+				{DisplayName: "ldap2", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=ldap2"},
+				{DisplayName: "oidc1", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=oidc1"},
+				{DisplayName: "oidc2", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=oidc2"},
+				{DisplayName: "z-ad1", URL: testIssuerWithTestReqQuery + "&pinniped_idp_name=z-ad1"},
+			}),
+		},
+		{
+			name:      "happy path when there are special characters in the IDP name",
+			method:    http.MethodGet,
+			reqTarget: "/some/path" + oidc.ChooseIDPEndpointPath + "?" + testReqQuery.Encode(),
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName(`This is Ryan's IDP 👍\~!@#$%^&*()-+[]{}\|;'"<>,.?`).Build()).
+				WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().WithName(`This is Josh's IDP 🦭`).Build()).
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusOK,
+			wantContentType: "text/html; charset=utf-8",
+			wantBodyString: testutil.ExpectedChooseIDPPageHTML(chooseidphtml.CSS(), chooseidphtml.JS(), []testutil.ChooseIDPPageExpectedValue{
+				// Should be sorted alphabetically by displayName.
+				{
+					DisplayName: `This is Josh's IDP 🦭`,
+					URL:         testIssuerWithTestReqQuery + `&pinniped_idp_name=` + url.QueryEscape(`This is Josh's IDP 🦭`),
+				},
+				{
+					DisplayName: `This is Ryan's IDP 👍\~!@#$%^&*()-+[]{}\|;'"<>,.?`,
+					URL:         testIssuerWithTestReqQuery + `&pinniped_idp_name=` + url.QueryEscape(`This is Ryan's IDP 👍\~!@#$%^&*()-+[]{}\|;'"<>,.?`),
+				},
+			}),
+		},
+		{
+			name:      "no valid IDPs are configured on the FederationDomain",
+			method:    http.MethodGet,
+			reqTarget: "/some/path" + oidc.ChooseIDPEndpointPath + "?" + testReqQuery.Encode(),
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusInternalServerError,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Internal Server Error: please check the server's configuration: no valid identity providers found for this FederationDomain\n",
+		},
+		{
+			name:      "no query params on the request",
+			method:    http.MethodGet,
+			reqTarget: "/some/path" + oidc.ChooseIDPEndpointPath,
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("x-some-idp").Build()).
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Bad Request: missing required query params (must include client_id, redirect_uri, scope, and response_type)\n",
+		},
+		{
+			name:      "missing required query param(s) on the request",
+			method:    http.MethodGet,
+			reqTarget: "/some/path" + oidc.ChooseIDPEndpointPath + "?client_id=foo",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("x-some-idp").Build()).
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Bad Request: missing required query params (must include client_id, redirect_uri, scope, and response_type)\n",
+		},
+		{
+			name:      "bad request method",
+			method:    http.MethodPost,
+			reqTarget: oidc.ChooseIDPEndpointPath,
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().WithName("x-some-idp").Build()).
+				BuildFederationDomainIdentityProvidersListerFinder(),
+			wantStatus:      http.StatusMethodNotAllowed,
+			wantContentType: "text/plain; charset=utf-8",
+			wantBodyString:  "Method Not Allowed: POST (try GET)\n",
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			handler := NewHandler(testIssuer, test.idps)
+
+			req := httptest.NewRequest(test.method, test.reqTarget, nil)
+			rsp := httptest.NewRecorder()
+			handler.ServeHTTP(rsp, req)
+
+			require.Equal(t, test.wantStatus, rsp.Code)
+			require.Equal(t, test.wantContentType, rsp.Header().Get("Content-Type"))
+			require.Equal(t, test.wantBodyString, rsp.Body.String())
+			testutil.RequireSecurityHeadersWithIDPChooserPageCSPs(t, rsp)
+		})
+	}
+}

internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.css

@@ -0,0 +1,71 @@
+/* Copyright 2023 the Pinniped contributors. All Rights Reserved. */
+/* SPDX-License-Identifier: Apache-2.0 */
+
+html {
+    height: 100%;
+}
+
+/* The form for this page is styled to be the same as the form from login_form.css */
+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;
+}
+
+/* Buttons for this page are styled to be the same as the form submit button in login_form.css */
+button {
+    color: inherit;
+    font: inherit;
+    border: 0;
+    margin: 0;
+    outline: 0;
+    padding: 0;
+}
+
+.form-field {
+    display: flex;
+    margin-bottom: 30px;
+}
+
+.form-field button {
+    width: 100%;
+    padding: 1em;
+    background-color: #218fcf; /* this is a color from the Pinniped logo :) */
+    color: #eee;
+    font-weight: bold;
+    cursor: pointer;
+    transition: all .3s;
+}
+
+.form-field button:focus, .form-field button:hover {
+    background-color: #1abfd3; /* this is a color from the Pinniped logo :) */
+}
+
+.form-field button:active {
+    transform: scale(.99);
+}

internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.js

@@ -0,0 +1,11 @@
+// Copyright 2023 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+window.onload = () => {
+    Array.from(document.querySelectorAll('button')).forEach(btn => {
+        btn.onclick = () => window.location.href = btn.dataset.url;
+    });
+    // Initially hidden to allow noscript tag to be the only visible content in the form in case Javascript is disabled.
+    // Make it visible whenever Javascript is enabled.
+    document.getElementById("choose-idp-form-buttons").hidden = false;
+};

internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go

@@ -0,0 +1,74 @@
+// Copyright 2023 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package chooseidphtml
+
+import (
+	_ "embed" // Needed to trigger //go:embed directives below.
+	"html/template"
+	"strings"
+
+	"github.com/tdewolff/minify/v2/minify"
+
+	"go.pinniped.dev/internal/federationdomain/csp"
+)
+
+//nolint:gochecknoglobals // This package uses globals to ensure that all parsing and minifying happens at init.
+var (
+	//go:embed choose_idp.css
+	rawCSS      string
+	minifiedCSS = panicOnError(minify.CSS(rawCSS))
+
+	//go:embed choose_idp.js
+	rawJS      string
+	minifiedJS = panicOnError(minify.JS(rawJS))
+
+	//go:embed choose_idp.gohtml
+	rawHTMLTemplate string
+
+	// Parse the Go templated HTML and inject functions providing the minified inline CSS and JS.
+	parsedHTMLTemplate = template.Must(template.New("choose_idp.gohtml").Funcs(template.FuncMap{
+		"minifiedCSS": func() template.CSS { return template.CSS(CSS()) },
+		"minifiedJS":  func() template.JS { return template.JS(JS()) }, //nolint:gosec // This is 100% static input, not attacker-controlled.
+	}).Parse(rawHTMLTemplate))
+
+	// Generate the CSP header value once since it's effectively constant.
+	cspValue = strings.Join([]string{
+		`default-src 'none'`,
+		`script-src '` + csp.Hash(minifiedJS) + `'`,
+		`style-src '` + csp.Hash(minifiedCSS) + `'`,
+		`img-src data:`,
+		`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 }
+
+// JS returns the minified JS that will be embedded into the page template.
+func JS() string { return minifiedJS }
+
+type IdentityProvider struct {
+	DisplayName string
+	URL         string
+}
+
+// PageData represents the inputs to the template.
+type PageData struct {
+	IdentityProviders []IdentityProvider
+}

internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml_test.go

@@ -0,0 +1,70 @@
+// Copyright 2023 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package chooseidphtml
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/testutil"
+)
+
+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}button{color:inherit;font:inherit;border:0;margin:0;outline:0;padding:0}.form-field{display:flex;margin-bottom:30px}.form-field button{width:100%;padding:1em;background-color:#218fcf;color:#eee;font-weight:700;cursor:pointer;transition:all .3s}.form-field button:focus,.form-field button:hover{background-color:#1abfd3}.form-field button:active{transform:scale(.99)}`
+
+	testExpectedJS = `window.onload=()=>{Array.from(document.querySelectorAll("button")).forEach(e=>{e.onclick=()=>window.location.href=e.dataset.url}),document.getElementById("choose-idp-form-buttons").hidden=!1}`
+
+	// 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'; ` +
+		`script-src 'sha256-eyuE+qQfuMn4WbDizGOp1wSGReaMYRYmRMXpyEo+8ps='; ` +
+		`style-src 'sha256-SgeTG5HEbHNFgjH+EvLrC+VKZRZQ6iAI3oFnW7i/Tm4='; ` +
+		`img-src data:; ` +
+		`frame-ancestors 'none'`
+)
+
+func TestTemplate(t *testing.T) {
+	const (
+		testUpstreamName1 = "test-idp-name1"
+		testUpstreamName2 = "test-idp-name2"
+		testURL1          = "https://pinniped.dev/path1?query=value"
+		testURL2          = "https://pinniped.dev/path2?query=value"
+	)
+
+	pageInputs := &PageData{
+		IdentityProviders: []IdentityProvider{
+			{DisplayName: testUpstreamName1, URL: testURL1},
+			{DisplayName: testUpstreamName2, URL: testURL2},
+		},
+	}
+
+	expectedHTML := testutil.ExpectedChooseIDPPageHTML(testExpectedCSS, testExpectedJS, []testutil.ChooseIDPPageExpectedValue{
+		{DisplayName: testUpstreamName1, URL: testURL1},
+		{DisplayName: testUpstreamName2, URL: testURL2},
+	})
+
+	var buf bytes.Buffer
+	require.NoError(t, Template().Execute(&buf, pageInputs))
+	require.Equal(t, expectedHTML, buf.String())
+}
+
+func TestContentSecurityPolicy(t *testing.T) {
+	require.Equal(t, testExpectedCSP, ContentSecurityPolicy())
+}
+
+func TestCSS(t *testing.T) {
+	require.Equal(t, testExpectedCSS, CSS())
+}
+
+func TestJS(t *testing.T) {
+	require.Equal(t, testExpectedJS, JS())
+}
+
+func TestHelpers(t *testing.T) {
+	require.Equal(t, "test", panicOnError("test", nil))
+	require.PanicsWithError(t, "some error", func() { panicOnError("", fmt.Errorf("some error")) })
+}

internal/federationdomain/endpointsmanager/manager.go

@@ -15,6 +15,7 @@ import (
 	"go.pinniped.dev/internal/federationdomain/dynamiccodec"
 	"go.pinniped.dev/internal/federationdomain/endpoints/auth"
 	"go.pinniped.dev/internal/federationdomain/endpoints/callback"
+	"go.pinniped.dev/internal/federationdomain/endpoints/chooseidp"
 	"go.pinniped.dev/internal/federationdomain/endpoints/discovery"
 	"go.pinniped.dev/internal/federationdomain/endpoints/idpdiscovery"
 	"go.pinniped.dev/internal/federationdomain/endpoints/jwks"
@@ -152,6 +153,11 @@ func (m *Manager) SetFederationDomains(federationDomains ...*federationdomainpro
 			issuerURL+oidc.CallbackEndpointPath,
 		)
 
+		m.providerHandlers[(issuerHostWithPath + oidc.ChooseIDPEndpointPath)] = chooseidp.NewHandler(
+			issuerURL+oidc.AuthorizationEndpointPath,
+			idpLister,
+		)
+
 		m.providerHandlers[(issuerHostWithPath + oidc.TokenEndpointPath)] = token.NewHandler(
 			idpLister,
 			oauthHelperWithKubeStorage,

internal/federationdomain/endpointsmanager/manager_test.go

@@ -123,6 +123,32 @@ func TestManager(t *testing.T) {
 			)
 		}
 
+		requirePinnipedIDPChooserRequestToBeHandled := func(requestIssuer string, expectedIDPNames []string) {
+			recorder := httptest.NewRecorder()
+
+			requiredParams := url.Values{
+				"client_id":     []string{"foo"},
+				"redirect_uri":  []string{"bar"},
+				"scope":         []string{"baz"},
+				"response_type": []string{"bat"},
+			}
+
+			subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.ChooseIDPEndpointPath+"?"+requiredParams.Encode()))
+
+			r.False(fallbackHandlerWasCalled)
+
+			// Minimal check to ensure that the right endpoint was called
+			r.Equal(http.StatusOK, recorder.Code, "unexpected response:", recorder)
+			r.Equal("text/html; charset=utf-8", recorder.Header().Get("Content-Type"))
+			responseBody, err := io.ReadAll(recorder.Body)
+			r.NoError(err)
+			// Should have some buttons whose URLs include the pinniped_idp_name param.
+			r.Contains(string(responseBody), "<button ")
+			for _, expectedIDPName := range expectedIDPNames {
+				r.Contains(string(responseBody), fmt.Sprintf("pinniped_idp_name=%s", url.QueryEscape(expectedIDPName)))
+			}
+		}
+
 		requireAuthorizationRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedRedirectLocationPrefix string) (string, string) {
 			recorder := httptest.NewRecorder()
 
@@ -377,6 +403,11 @@ func TestManager(t *testing.T) {
 			requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2KeyID)
 			requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2KeyID)
 
+			requirePinnipedIDPChooserRequestToBeHandled(issuer1, []string{upstreamIDPDisplayName1, upstreamIDPDisplayName2})
+			requirePinnipedIDPChooserRequestToBeHandled(issuer2, []string{upstreamIDPDisplayName1, upstreamIDPDisplayName2})
+			requirePinnipedIDPChooserRequestToBeHandled(issuer1DifferentCaseHostname, []string{upstreamIDPDisplayName1, upstreamIDPDisplayName2})
+			requirePinnipedIDPChooserRequestToBeHandled(issuer2DifferentCaseHostname, []string{upstreamIDPDisplayName1, upstreamIDPDisplayName2})
+
 			authRequestParamsIDP1 := "?" + url.Values{
 				"pinniped_idp_name":     []string{upstreamIDPDisplayName1},
 				"response_type":         []string{"code"},

internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder.go

@@ -36,6 +36,9 @@ type FederationDomainIdentityProvidersFinderI interface {
 		*resolvedprovider.FederationDomainResolvedLDAPIdentityProvider,
 		error,
 	)
+
+	HasDefaultIDP() bool
+	IDPCount() int
 }
 
 type FederationDomainIdentityProvidersListerI interface {
@@ -71,8 +74,10 @@ type FederationDomainIdentityProvidersListerFinder struct {
 // federationDomainIssuer parameter's IdentityProviders() list must have a unique DisplayName.
 // Note that a single underlying IDP UID may be used by multiple FederationDomainIdentityProvider in the parameter.
 // The wrapped lister should contain all valid upstream providers that are defined in the Supervisor, and is expected to
-// be thread-safe and to change its contents over time. The FederationDomainIdentityProvidersListerFinder will filter out the
+// be thread-safe and to change its contents over time. (Note that it should not contain any invalid or unready identity
-// ones that don't apply to this federation domain.
+// providers because the controllers that fill this cache should not put invalid or unready providers into the cache.)
+// The FederationDomainIdentityProvidersListerFinder will filter out the ones that don't apply to this federation
+// domain.
 func NewFederationDomainIdentityProvidersListerFinder(
 	federationDomainIssuer *FederationDomainIssuer,
 	wrappedLister idplister.UpstreamIdentityProvidersLister,
@@ -99,6 +104,10 @@ func NewFederationDomainIdentityProvidersListerFinder(
 	}
 }
 
+func (u *FederationDomainIdentityProvidersListerFinder) IDPCount() int {
+	return len(u.GetOIDCIdentityProviders()) + len(u.GetLDAPIdentityProviders()) + len(u.GetActiveDirectoryIdentityProviders())
+}
+
 // FindUpstreamIDPByDisplayName selects either an OIDC, LDAP, or ActiveDirectory IDP, or returns an error.
 // It only considers the allowed IDPs while doing the lookup by display name.
 // Note that ActiveDirectory and LDAP IDPs both return the same type, but with different SessionProviderType values.
@@ -131,6 +140,10 @@ func (u *FederationDomainIdentityProvidersListerFinder) FindUpstreamIDPByDisplay
 	return nil, nil, fmt.Errorf("identity provider not available: %q", upstreamIDPDisplayName)
 }
 
+func (u *FederationDomainIdentityProvidersListerFinder) HasDefaultIDP() bool {
+	return u.defaultIdentityProvider != nil
+}
+
 // FindDefaultIDP works like FindUpstreamIDPByDisplayName, but finds the default IDP instead of finding by name.
 // If there is no default IDP for this federation domain, then FindDefaultIDP will return an error.
 // This can be used to handle the backwards compatibility mode where an authorization request could be made
@@ -141,7 +154,7 @@ func (u *FederationDomainIdentityProvidersListerFinder) FindDefaultIDP() (
 	*resolvedprovider.FederationDomainResolvedLDAPIdentityProvider,
 	error,
 ) {
-	if u.defaultIdentityProvider == nil {
+	if !u.HasDefaultIDP() {
 		return nil, nil, fmt.Errorf("identity provider not found: this federation domain does not have a default identity provider")
 	}
 	return u.FindUpstreamIDPByDisplayName(u.defaultIdentityProvider.DisplayName)

internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder_test.go

@@ -99,7 +99,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	fdIssuerWithIDPwithLostUID, err := NewFederationDomainIssuer(fakeIssuerURL, []*FederationDomainIdentityProvider{
+	fdIssuerWithIDPWithLostUID, err := NewFederationDomainIssuer(fakeIssuerURL, []*FederationDomainIdentityProvider{
 		{DisplayName: "my-idp", UID: "you-cant-find-my-uid"},
 	})
 	require.NoError(t, err)
@@ -244,7 +244,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 			findIDPByDisplayName: "my-idp",
 			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
 				BuildDynamicUpstreamIDPProvider(),
-			federationDomainIssuer: fdIssuerWithIDPwithLostUID,
+			federationDomainIssuer: fdIssuerWithIDPWithLostUID,
 			wantError:              `identity provider not available: "my-idp"`,
 		},
 	}
@@ -263,10 +263,10 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 				require.NoError(t, err)
 			}
 			if tt.wantOIDCIDPByDisplayName != nil {
-				require.Equal(t, foundOIDCIDP, tt.wantOIDCIDPByDisplayName)
+				require.Equal(t, tt.wantOIDCIDPByDisplayName, foundOIDCIDP)
 			}
 			if tt.wantLDAPIDPByDisplayName != nil {
-				require.Equal(t, foundLDAPIDP, tt.wantLDAPIDPByDisplayName)
+				require.Equal(t, tt.wantLDAPIDPByDisplayName, foundLDAPIDP)
 			}
 		})
 	}
@@ -339,10 +339,10 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 				require.NoError(t, err)
 			}
 			if tt.wantDefaultOIDCIDP != nil {
-				require.Equal(t, foundOIDCIDP, tt.wantDefaultOIDCIDP)
+				require.Equal(t, tt.wantDefaultOIDCIDP, foundOIDCIDP)
 			}
 			if tt.wantDefaultLDAPIDP != nil {
-				require.Equal(t, foundLDAPIDP, tt.wantDefaultLDAPIDP)
+				require.Equal(t, tt.wantDefaultLDAPIDP, foundLDAPIDP)
 			}
 		})
 	}
@@ -406,7 +406,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 			subject := NewFederationDomainIdentityProvidersListerFinder(tt.federationDomainIssuer, tt.wrappedLister)
 			idps := subject.GetOIDCIdentityProviders()
 
-			require.Equal(t, idps, tt.wantIDPs)
+			require.Equal(t, tt.wantIDPs, idps)
 		})
 	}
 
@@ -467,7 +467,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 			subject := NewFederationDomainIdentityProvidersListerFinder(tt.federationDomainIssuer, tt.wrappedLister)
 			idps := subject.GetLDAPIdentityProviders()
 
-			require.Equal(t, idps, tt.wantIDPs)
+			require.Equal(t, tt.wantIDPs, idps)
 		})
 	}
 
@@ -529,7 +529,110 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) {
 			subject := NewFederationDomainIdentityProvidersListerFinder(tt.federationDomainIssuer, tt.wrappedLister)
 			idps := subject.GetActiveDirectoryIdentityProviders()
 
-			require.Equal(t, idps, tt.wantIDPs)
+			require.Equal(t, tt.wantIDPs, idps)
+		})
+	}
+
+	testIDPCount := []struct {
+		name                   string
+		wrappedLister          idplister.UpstreamIdentityProvidersLister
+		federationDomainIssuer *FederationDomainIssuer
+		wantCount              int
+	}{
+		{
+			name: "IDPCount when there are none to be found",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs,
+			wantCount:              0,
+		},
+		{
+			name: "IDPCount when there are various types of IDP to be found",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(myOIDCIDP1).
+				WithOIDC(myOIDCIDP2).
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
+					WithName("my-oidc-idp-that-isnt-in-fd-issuer").
+					WithResourceUID("my-oidc-idp-that-isnt-in-fd-issuer").
+					Build()).
+				WithLDAP(myLDAPIDP1).
+				WithLDAP(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().
+					WithName("my-ldap-idp-that-isnt-in-fd-issuer").
+					WithResourceUID("my-ldap-idp-that-isnt-in-fd-issuer").
+					Build()).
+				WithActiveDirectory(myADIDP1).
+				WithActiveDirectory(myADIDP2).
+				WithActiveDirectory(oidctestutil.NewTestUpstreamLDAPIdentityProviderBuilder().
+					WithName("my-ad-idp-that-isnt-in-fd-issuer").
+					WithResourceUID("my-ad-idp-that-isnt-in-fd-issuer").
+					Build()).
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs,
+			wantCount:              5,
+		},
+	}
+
+	for _, tt := range testIDPCount {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			subject := NewFederationDomainIdentityProvidersListerFinder(tt.federationDomainIssuer, tt.wrappedLister)
+
+			require.Equal(t, tt.wantCount, subject.IDPCount())
+		})
+	}
+
+	testHasDefaultIDP := []struct {
+		name                   string
+		wrappedLister          idplister.UpstreamIdentityProvidersLister
+		federationDomainIssuer *FederationDomainIssuer
+		wantHasDefaultIDP      bool
+	}{
+		{
+			name: "HasDefaultIDP when there is an OIDC provider set as default",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(myDefaultOIDCIDP).
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithDefaultOIDCIDP,
+			wantHasDefaultIDP:      true,
+		},
+		{
+			name: "HasDefaultIDP when there is an LDAP provider set as default",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithLDAP(myDefaultLDAPIDP).
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithDefaultLDAPIDP,
+			wantHasDefaultIDP:      true,
+		},
+		{
+			name: "HasDefaultIDP when there is one set even if it cannot be found",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithOIDC(oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
+					WithName("my-oidc-idp-that-isnt-in-fd-issuer").
+					WithResourceUID("my-oidc-idp-that-isnt-in-fd-issuer").
+					Build()).
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithDefaultOIDCIDP,
+			wantHasDefaultIDP:      true,
+		},
+		{
+			name: "HasDefaultIDP when there is none set",
+			wrappedLister: oidctestutil.NewUpstreamIDPListerBuilder().
+				BuildDynamicUpstreamIDPProvider(),
+			federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs,
+			wantHasDefaultIDP:      false,
+		},
+	}
+
+	for _, tt := range testHasDefaultIDP {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			subject := NewFederationDomainIdentityProvidersListerFinder(tt.federationDomainIssuer, tt.wrappedLister)
+
+			require.Equal(t, tt.wantHasDefaultIDP, subject.HasDefaultIDP())
 		})
 	}
 }

internal/federationdomain/oidc/oidc.go

@@ -37,6 +37,7 @@ const (
 	AuthorizationEndpointPath = "/oauth2/authorize"
 	TokenEndpointPath         = "/oauth2/token" //nolint:gosec // ignore lint warning that this is a credential
 	CallbackEndpointPath      = "/callback"
+	ChooseIDPEndpointPath     = "/choose_identity_provider"
 	JWKSEndpointPath          = "/jwks.json"
 	PinnipedIDPsPathV1Alpha1  = "/v1alpha1/pinniped_identity_providers"
 	PinnipedLoginPath         = "/login"

internal/testutil/assertions.go

@@ -99,6 +99,19 @@ func RequireSecurityHeadersWithLoginPageCSPs(t *testing.T, response *httptest.Re
 	requireSecurityHeaders(t, response)
 }
 
+func RequireSecurityHeadersWithIDPChooserPageCSPs(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.Contains(t, cspHeader, "script-src '") // loose assertion
+	require.Contains(t, cspHeader, "style-src '")  // loose assertion
+	require.Contains(t, cspHeader, "img-src data:")
+	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")

internal/testutil/oidctestutil/oidctestutil.go

@@ -467,6 +467,14 @@ type TestFederationDomainIdentityProvidersListerFinder struct {
 	defaultIDPDisplayName                    string
 }
 
+func (t *TestFederationDomainIdentityProvidersListerFinder) HasDefaultIDP() bool {
+	return t.defaultIDPDisplayName != ""
+}
+
+func (t *TestFederationDomainIdentityProvidersListerFinder) IDPCount() int {
+	return len(t.upstreamOIDCIdentityProviders) + len(t.upstreamLDAPIdentityProviders) + len(t.upstreamActiveDirectoryIdentityProviders)
+}
+
 func (t *TestFederationDomainIdentityProvidersListerFinder) GetOIDCIdentityProviders() []*resolvedprovider.FederationDomainResolvedOIDCIdentityProvider {
 	fdIDPs := make([]*resolvedprovider.FederationDomainResolvedOIDCIdentityProvider, len(t.upstreamOIDCIdentityProviders))
 	for i, testIDP := range t.upstreamOIDCIdentityProviders {

site/content/docs/howto/configure-auth-for-webapps.md

@@ -46,11 +46,16 @@ framework (e.g. Spring, Rails, Django, etc.) to implement authentication. The Su
 - Clients must use `query` as the
   [response_mode](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest) at the authorization endpoint,
   or not specify the `response_mode` param, which defaults to `query`.
-- If the Supervisor's FederationDomain was configured with explicit `identityProviders` in its spec, then the
+- The client may optionally send an extra parameter on the initial authorization request to indicate which identity
-  client must send an extra parameter on the initial authorization request to indicate which identity provider
+  provider the user would like to use when authenticating. This parameter is called `pinniped_idp_name` and the value
-  the user would like to use when authenticating. This parameter is called `pinniped_idp_name` and the value
   of the parameter should be set to the `displayName` of the identity provider as it was configured on the
-  FederationDomain.
+  FederationDomain. When this parameter is not included, and when the FederationDomain was configured with explicit
+  `identityProviders` in its spec, then the user will be prompted to choose an identity provider from the list of
+  available identity providers by an interstitial web page during their login flow. The value of this parameter
+  should be considered a hint and not a hard requirement, since the user could choose to alter or remove this
+  query param from the authorization URL, and thus could use a different available identity provider from the
+  FederationDomain to log in. This is not a security concern, since any successful login using any available identity
+  provider from the FederationDomain's configuration is a valid and allowed user.
 
 Most web application frameworks offer all these capabilities in their OAuth2/OIDC libraries.
 

test/integration/supervisor_login_test.go

@@ -235,7 +235,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 		createIDP func(t *testing.T) string
 
 		// Optionally specify the identityProviders part of the FederationDomain's spec by returning it from this function.
-		// Also return the displayName of the IDP that should be used during authentication.
+		// Also return the displayName of the IDP that should be used during authentication (or empty string for no IDP name in the auth request).
 		// This function takes the name of the IDP CR which was returned by createIDP() as as argument.
 		federationDomainIDPs func(t *testing.T, idpName string) (idps []configv1alpha1.FederationDomainIdentityProvider, useIDPDisplayName string)
 
@@ -1430,6 +1430,51 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 			wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" },
 			wantDownstreamIDTokenGroups:          env.SupervisorUpstreamOIDC.ExpectedGroups,
 		},
+		{
+			name:      "oidc upstream with downstream dynamic client happy path, requesting all scopes, using the IDP chooser page",
+			maybeSkip: skipNever,
+			createIDP: func(t *testing.T) string {
+				spec := basicOIDCIdentityProviderSpec()
+				spec.Claims = idpv1alpha1.OIDCClaims{
+					Username: env.SupervisorUpstreamOIDC.UsernameClaim,
+					Groups:   env.SupervisorUpstreamOIDC.GroupsClaim,
+				}
+				spec.AuthorizationConfig = idpv1alpha1.OIDCAuthorizationConfig{
+					AdditionalScopes: env.SupervisorUpstreamOIDC.AdditionalScopes,
+				}
+				return testlib.CreateTestOIDCIdentityProvider(t, spec, idpv1alpha1.PhaseReady).Name
+			},
+			federationDomainIDPs: func(t *testing.T, idpName string) ([]configv1alpha1.FederationDomainIdentityProvider, string) {
+				displayName := "my oidc idp"
+				return []configv1alpha1.FederationDomainIdentityProvider{
+						{
+							DisplayName: displayName,
+							ObjectRef: v1.TypedLocalObjectReference{
+								APIGroup: ptr.To("idp.supervisor." + env.APIGroupSuffix),
+								Kind:     "OIDCIdentityProvider",
+								Name:     idpName,
+							},
+						},
+					},
+					"" // return an empty string be used as the pinniped_idp_name param's value in the authorize request,
+				// which should cause the authorize endpoint to show the IDP chooser page
+			},
+			createOIDCClient: func(t *testing.T, callbackURL string) (string, string) {
+				return testlib.CreateOIDCClient(t, configv1alpha1.OIDCClientSpec{
+					AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(callbackURL)},
+					AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+					AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+				}, configv1alpha1.OIDCClientPhaseReady)
+			},
+			requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowOIDCWithIDPChooserPage,
+			wantDownstreamIDTokenSubjectToMatch: "^" +
+				regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer) +
+				regexp.QuoteMeta("?idpName="+url.QueryEscape("my oidc idp")) +
+				regexp.QuoteMeta("&sub=") + ".+" +
+				"$",
+			wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" },
+			wantDownstreamIDTokenGroups:          env.SupervisorUpstreamOIDC.ExpectedGroups,
+		},
 		{
 			name:      "oidc upstream with downstream dynamic client happy path, requesting all scopes, with additional claims",
 			maybeSkip: skipNever,
@@ -2727,9 +2772,8 @@ func requestAuthorizationAndExpectImmediateRedirectToCallback(t *testing.T, _, d
 	browser.WaitForURL(t, callbackURLPattern)
 }
 
-func requestAuthorizationUsingBrowserAuthcodeFlowOIDC(t *testing.T, _, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) {
+func openBrowserAndNavigateToAuthorizeURL(t *testing.T, downstreamAuthorizeURL string, httpClient *http.Client) *browsertest.Browser {
 	t.Helper()
-	env := testlib.IntegrationEnv(t)
 
 	ctx, cancelFunc := context.WithTimeout(context.Background(), time.Minute)
 	defer cancelFunc()
@@ -2742,13 +2786,45 @@ func requestAuthorizationUsingBrowserAuthcodeFlowOIDC(t *testing.T, _, downstrea
 	t.Logf("opening browser to downstream authorize URL %s", testlib.MaskTokens(downstreamAuthorizeURL))
 	browser.Navigate(t, downstreamAuthorizeURL)
 
+	return browser
+}
+
+func loginToUpstreamOIDCAndWaitForCallback(t *testing.T, b *browsertest.Browser, downstreamCallbackURL string) {
+	t.Helper()
+	env := testlib.IntegrationEnv(t)
+
 	// Expect to be redirected to the upstream provider and log in.
-	browsertest.LoginToUpstreamOIDC(t, browser, env.SupervisorUpstreamOIDC)
+	browsertest.LoginToUpstreamOIDC(t, b, env.SupervisorUpstreamOIDC)
 
 	// Wait for the login to happen and us be redirected back to a localhost callback.
 	t.Logf("waiting for redirect to callback")
 	callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(downstreamCallbackURL) + `\?.+\z`)
-	browser.WaitForURL(t, callbackURLPattern)
+	b.WaitForURL(t, callbackURLPattern)
+}
+
+func requestAuthorizationUsingBrowserAuthcodeFlowOIDC(t *testing.T, _, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) {
+	t.Helper()
+
+	browser := openBrowserAndNavigateToAuthorizeURL(t, downstreamAuthorizeURL, httpClient)
+
+	loginToUpstreamOIDCAndWaitForCallback(t, browser, downstreamCallbackURL)
+}
+
+func requestAuthorizationUsingBrowserAuthcodeFlowOIDCWithIDPChooserPage(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) {
+	t.Helper()
+
+	browser := openBrowserAndNavigateToAuthorizeURL(t, downstreamAuthorizeURL, httpClient)
+
+	t.Log("waiting for redirect to IDP chooser page")
+	browser.WaitForURL(t, regexp.MustCompile(fmt.Sprintf(`\A%s/choose_identity_provider.*\z`, downstreamIssuer)))
+
+	t.Log("waiting for any IDP chooser button to be visible")
+	browser.WaitForVisibleElements(t, "button")
+
+	t.Log("clicking the first IDP chooser button")
+	browser.ClickFirstMatch(t, "button")
+
+	loginToUpstreamOIDCAndWaitForCallback(t, browser, downstreamCallbackURL)
 }
 
 func requestAuthorizationUsingBrowserAuthcodeFlowLDAP(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client) {

test/testlib/browsertest/browsertest.go

@@ -184,38 +184,38 @@ func (b *Browser) Title(t *testing.T) string {
 	return title
 }
 
-func (b *Browser) WaitForVisibleElements(t *testing.T, selectors ...string) {
+func (b *Browser) WaitForVisibleElements(t *testing.T, cssSelectors ...string) {
 	t.Helper()
-	for _, s := range selectors {
+	for _, s := range cssSelectors {
-		b.runWithTimeout(t, b.timeout(), chromedp.WaitVisible(s))
+		b.runWithTimeout(t, b.timeout(), chromedp.WaitVisible(s, chromedp.ByQuery))
 	}
 }
 
-func (b *Browser) TextOfFirstMatch(t *testing.T, selector string) string {
+func (b *Browser) TextOfFirstMatch(t *testing.T, cssSelector string) string {
 	t.Helper()
 	var text string
-	b.runWithTimeout(t, b.timeout(), chromedp.Text(selector, &text, chromedp.NodeVisible))
+	b.runWithTimeout(t, b.timeout(), chromedp.Text(cssSelector, &text, chromedp.NodeVisible, chromedp.ByQuery))
 	return text
 }
 
-func (b *Browser) AttrValueOfFirstMatch(t *testing.T, selector string, attributeName string) string {
+func (b *Browser) AttrValueOfFirstMatch(t *testing.T, cssSelector string, attributeName string) string {
 	t.Helper()
 	var value string
 	var ok bool
-	b.runWithTimeout(t, b.timeout(), chromedp.AttributeValue(selector, attributeName, &value, &ok))
+	b.runWithTimeout(t, b.timeout(), chromedp.AttributeValue(cssSelector, attributeName, &value, &ok, chromedp.ByQuery))
-	require.Truef(t, ok, "did not find attribute named %q on first element returned by selector %q", attributeName, selector)
+	require.Truef(t, ok, "did not find attribute named %q on first element returned by selector %q", attributeName, cssSelector)
 	return value
 }
 
-func (b *Browser) SendKeysToFirstMatch(t *testing.T, selector string, runesToType string) {
+func (b *Browser) SendKeysToFirstMatch(t *testing.T, cssSelector string, runesToType string) {
 	t.Helper()
-	b.runWithTimeout(t, b.timeout(), chromedp.SendKeys(selector, runesToType, chromedp.NodeVisible, chromedp.NodeEnabled))
+	b.runWithTimeout(t, b.timeout(), chromedp.SendKeys(cssSelector, runesToType, chromedp.NodeVisible, chromedp.NodeEnabled, chromedp.ByQuery))
 }
 
-func (b *Browser) ClickFirstMatch(t *testing.T, selector string) string {
+func (b *Browser) ClickFirstMatch(t *testing.T, cssSelector string) string {
 	t.Helper()
 	var text string
-	b.runWithTimeout(t, b.timeout(), chromedp.Click(selector, chromedp.NodeVisible, chromedp.NodeEnabled))
+	b.runWithTimeout(t, b.timeout(), chromedp.Click(cssSelector, chromedp.NodeVisible, chromedp.NodeEnabled, chromedp.ByQuery))
 	return text
 }