From b99c4773a2df7fc8a2fed9a2e4fc72c86fddf211 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 2 Jun 2022 09:23:34 -0700 Subject: [PATCH 1/2] Use CSP headers in auth handler response When response_mode=form_post is requested, some error cases will be returned to the client using the form_post web page to POST the result back to the client's redirect URL. --- internal/oidc/auth/auth_handler.go | 10 ++++++++-- internal/oidc/auth/auth_handler_test.go | 21 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index b4b9fccd3..698ea7f38 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/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( diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index dc93c42ac..11431a0b0 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/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: ` Date: Tue, 7 Jun 2022 11:20:59 -0700 Subject: [PATCH 2/2] Update CSP header expectations in TestSupervisorLogin_Browser int test --- test/integration/supervisor_login_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 3713175a0..3c2f045a4 100644 --- a/test/integration/supervisor_login_test.go +++ b/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"))