Merge pull request #1179 from vmware-tanzu/auth_handler_form_post_csp

Fix bug in certain error handling for authorize endpoint when response_mode=form_post is requested

internal/oidc/auth/auth_handler.go

@@ -24,6 +24,7 @@ import (
 	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/login"
 	"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"
@@ -46,7 +47,7 @@ func NewHandler(
 	upstreamStateEncoder oidc.Encoder,
 	cookieCodec oidc.Codec,
 ) http.Handler {
-	return securityheader.Wrap(httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+	handler := httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 		if r.Method != http.MethodPost && r.Method != http.MethodGet {
 			// https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
 			// Authorization Servers MUST support the use of the HTTP GET and POST methods defined in
@@ -105,7 +106,12 @@ func NewHandler(
 			upstreamStateEncoder,
 			cookieCodec,
 		)
-	}))
+	})
+
+	// During a response_mode=form_post auth request using the browser flow, the custom form_post html page may
+	// be used to post certain errors back to the CLI from this handler's response, so allow the form_post
+	// page's CSS and JS to run.
+	return securityheader.WrapWithCustomCSP(handler, formposthtml.ContentSecurityPolicy())
 }
 
 func handleAuthRequestForLDAPUpstreamCLIFlow(

internal/oidc/auth/auth_handler_test.go

@@ -521,6 +521,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		wantStatus                             int
 		wantContentType                        string
 		wantBodyString                         string
+		wantBodyRegex                          string
 		wantBodyJSON                           string
 		wantCSRFValueInCookieHeader            string
 		wantBodyStringWithLocationInHref       bool
@@ -1537,6 +1538,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:       "",
 		},
+		{
+			name:            "form_post page is used to send errors to client using OIDC upstream browser flow with response_mode=form_post",
+			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			generateCSRF:    happyCSRFGenerator,
+			generatePKCE:    happyPKCEGenerator,
+			generateNonce:   happyNonceGenerator,
+			stateEncoder:    happyStateEncoder,
+			cookieEncoder:   happyCookieEncoder,
+			method:          http.MethodGet,
+			path:            modifiedHappyGetRequestPath(map[string]string{"response_mode": "form_post", "scope": "openid profile email tuna"}),
+			wantStatus:      http.StatusOK,
+			wantContentType: htmlContentType,
+			wantBodyRegex:   `<input type="hidden" name="encoded_params" value="error=invalid_scope&amp;error_description=The&#43;requested&#43;scope&#43;is&#43;invalid`,
+		},
 		{
 			name:                 "downstream scopes do not match what is configured for client using LDAP upstream",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
@@ -2578,7 +2593,9 @@ func TestAuthorizationEndpoint(t *testing.T) {
 
 		require.Equal(t, test.wantStatus, rsp.Code)
 		testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType)
-		testutil.RequireSecurityHeadersWithoutCustomCSPs(t, rsp)
+
+		// Use form_post page's CSPs because sometimes errors are sent to the client via the form_post page.
+		testutil.RequireSecurityHeadersWithFormPostPageCSPs(t, rsp)
 
 		if test.wantPasswordGrantCall != nil {
 			test.wantPasswordGrantCall.args.Ctx = reqContext
@@ -2645,6 +2662,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			default:
 				t.Errorf("unexpected response code: %v", code)
 			}
+		case test.wantBodyRegex != "":
+			require.Regexp(t, test.wantBodyRegex, rsp.Body.String())
 		default:
 			require.Equal(t, test.wantBodyString, rsp.Body.String())
 		}

test/integration/supervisor_login_test.go

@@ -1809,7 +1809,15 @@ func doTokenExchange(t *testing.T, config *oauth2.Config, tokenResponse *oauth2.
 
 func expectSecurityHeaders(t *testing.T, response *http.Response, expectFositeToOverrideSome bool) {
 	h := response.Header
-	assert.Equal(t, "default-src 'none'; frame-ancestors 'none'", h.Get("Content-Security-Policy"))
+
+	cspHeader := h.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 *")
+	require.Contains(t, cspHeader, "default-src 'none'")
+	require.Contains(t, cspHeader, "frame-ancestors 'none'")
+
 	assert.Equal(t, "DENY", h.Get("X-Frame-Options"))
 	assert.Equal(t, "1; mode=block", h.Get("X-XSS-Protection"))
 	assert.Equal(t, "nosniff", h.Get("X-Content-Type-Options"))