diff --git a/internal/federationdomain/endpoints/auth/auth_handler.go b/internal/federationdomain/endpoints/auth/auth_handler.go index df58c543..7cc7b91e 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler.go +++ b/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, diff --git a/internal/federationdomain/endpoints/auth/auth_handler_test.go b/internal/federationdomain/endpoints/auth/auth_handler_test.go index a7b5a246..ffa11934 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler_test.go +++ b/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()), diff --git a/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go b/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go new file mode 100644 index 00000000..ef1ae70a --- /dev/null +++ b/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) + }) +} diff --git a/internal/federationdomain/endpoints/chooseidp/choose_idp_handler_test.go b/internal/federationdomain/endpoints/chooseidp/choose_idp_handler_test.go new file mode 100644 index 00000000..d759b1c4 --- /dev/null +++ b/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) + }) + } +} diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.css b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.css new file mode 100644 index 00000000..01d72aaf --- /dev/null +++ b/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); +} diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.gohtml b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.gohtml new file mode 100644 index 00000000..e0743d65 --- /dev/null +++ b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.gohtml @@ -0,0 +1,43 @@ + + + + Choose Identity Provider + + + + + + +
+
+

Choose an identity provider to log in

+
+ + +
+ + diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.js b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/choose_idp.js new file mode 100644 index 00000000..d6cd9546 --- /dev/null +++ b/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; +}; diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go new file mode 100644 index 00000000..e45e1bf3 --- /dev/null +++ b/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 +} diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml_test.go b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml_test.go new file mode 100644 index 00000000..23c490da --- /dev/null +++ b/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")) }) +} diff --git a/internal/federationdomain/endpointsmanager/manager.go b/internal/federationdomain/endpointsmanager/manager.go index fa96f3bb..55b6ceca 100644 --- a/internal/federationdomain/endpointsmanager/manager.go +++ b/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, diff --git a/internal/federationdomain/endpointsmanager/manager_test.go b/internal/federationdomain/endpointsmanager/manager_test.go index 819aa361..d87f4769 100644 --- a/internal/federationdomain/endpointsmanager/manager_test.go +++ b/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), "`) + + withNewline(spaces(12)+``) + + withNewline(spaces(8)) + } + buttons += withNewline(spaces(4) + ``) + + bottom := here.Doc(` + + + + `) + + return top + noscript + buttons + bottom +} diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index a3c55591..f1c78e30 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/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 { diff --git a/site/content/docs/howto/configure-auth-for-webapps.md b/site/content/docs/howto/configure-auth-for-webapps.md index c7c68fc4..f70cb8e6 100644 --- a/site/content/docs/howto/configure-auth-for-webapps.md +++ b/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 - client must send an extra parameter on the initial authorization request to indicate which identity provider - the user would like to use when authenticating. This parameter is called `pinniped_idp_name` and the value +- The client may optionally send an extra parameter on the initial authorization request to indicate which identity + provider 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. diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 0d4f2912..d6500ff6 100644 --- a/test/integration/supervisor_login_test.go +++ b/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) { diff --git a/test/testlib/browsertest/browsertest.go b/test/testlib/browsertest/browsertest.go index 4e537ad2..c561c8a2 100644 --- a/test/testlib/browsertest/browsertest.go +++ b/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 { - b.runWithTimeout(t, b.timeout(), chromedp.WaitVisible(s)) + for _, s := range cssSelectors { + 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)) - require.Truef(t, ok, "did not find attribute named %q on first element returned by selector %q", attributeName, selector) + 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, 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 }