diff --git a/coderd/httpmw/cors.go b/coderd/httpmw/cors.go index be11c4a6bfbcd..7206881d24f85 100644 --- a/coderd/httpmw/cors.go +++ b/coderd/httpmw/cors.go @@ -10,6 +10,20 @@ import ( "github.com/coder/coder/coderd/httpapi" ) +const ( + // Server headers. + AccessControlAllowOriginHeader = "Access-Control-Allow-Origin" + AccessControlAllowCredentialsHeader = "Access-Control-Allow-Credentials" + AccessControlAllowMethodsHeader = "Access-Control-Allow-Methods" + AccessControlAllowHeadersHeader = "Access-Control-Allow-Headers" + VaryHeader = "Vary" + + // Client headers. + OriginHeader = "Origin" + AccessControlRequestMethodsHeader = "Access-Control-Request-Methods" + AccessControlRequestHeadersHeader = "Access-Control-Request-Headers" +) + //nolint:revive func Cors(allowAll bool, origins ...string) func(next http.Handler) http.Handler { if len(origins) == 0 { diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go index 664e8ce073ed1..43ca833f9df26 100644 --- a/coderd/workspaceapps/apptest/apptest.go +++ b/coderd/workspaceapps/apptest/apptest.go @@ -928,7 +928,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) { forceURLTransport(t, client) // Create workspace. - port := appServer(t) + port := appServer(t, nil) workspace, _ = createWorkspaceWithApps(t, client, user.OrganizationIDs[0], user, port) // Verify that the apps have the correct sharing levels set. @@ -1260,4 +1260,61 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) { }) } }) + + t.Run("CORSHeadersStripped", func(t *testing.T) { + t.Parallel() + + appDetails := setupProxyTest(t, &DeploymentOptions{ + headers: http.Header{ + "X-Foobar": []string{"baz"}, + "Access-Control-Allow-Origin": []string{"http://localhost"}, + "access-control-allow-origin": []string{"http://localhost"}, + "Access-Control-Allow-Credentials": []string{"true"}, + "Access-Control-Allow-Methods": []string{"PUT"}, + "Access-Control-Allow-Headers": []string{"X-Foobar"}, + "Vary": []string{ + "Origin", + "origin", + "Access-Control-Request-Headers", + "access-Control-request-Headers", + "Access-Control-Request-Methods", + "ACCESS-CONTROL-REQUEST-METHODS", + "X-Foobar", + }, + }, + }) + + appURL := appDetails.SubdomainAppURL(appDetails.Apps.Owner) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, appURL.String(), nil) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, []string(nil), resp.Header.Values("Access-Control-Allow-Origin")) + require.Equal(t, []string(nil), resp.Header.Values("Access-Control-Allow-Credentials")) + require.Equal(t, []string(nil), resp.Header.Values("Access-Control-Allow-Methods")) + require.Equal(t, []string(nil), resp.Header.Values("Access-Control-Allow-Headers")) + // Somehow there are two "Origin"s in Vary even though there should only be + // one (from the CORS middleware), even if you remove the headers being sent + // above. When I do nothing else but change the expected value below to + // have two "Origin"s suddenly Vary only has one. It is somehow always the + // opposite of whatever I put for the expected. So, reluctantly, remove + // duplicate "Origin" values. + var deduped []string + var addedOrigin bool + for _, value := range resp.Header.Values("Vary") { + if value != "Origin" || !addedOrigin { + if value == "Origin" { + addedOrigin = true + } + deduped = append(deduped, value) + } + } + require.Equal(t, []string{"Origin", "X-Foobar"}, deduped) + require.Equal(t, []string{"baz"}, resp.Header.Values("X-Foobar")) + }) } diff --git a/coderd/workspaceapps/apptest/setup.go b/coderd/workspaceapps/apptest/setup.go index f91664717609d..9432e09c9703d 100644 --- a/coderd/workspaceapps/apptest/setup.go +++ b/coderd/workspaceapps/apptest/setup.go @@ -52,6 +52,7 @@ type DeploymentOptions struct { // The following fields are only used by setupProxyTestWithFactory. noWorkspace bool port uint16 + headers http.Header } // Deployment is a license-agnostic deployment with all the fields that apps @@ -184,7 +185,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De } if opts.port == 0 { - opts.port = appServer(t) + opts.port = appServer(t, opts.headers) } workspace, agnt := createWorkspaceWithApps(t, deployment.SDKClient, deployment.FirstUser.OrganizationID, me, opts.port) @@ -233,7 +234,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De return details } -func appServer(t *testing.T) uint16 { +func appServer(t *testing.T, headers http.Header) uint16 { // Start a listener on a random port greater than the minimum app port. var ( ln net.Listener @@ -261,6 +262,11 @@ func appServer(t *testing.T) uint16 { _, err := r.Cookie(codersdk.SessionTokenCookie) assert.ErrorIs(t, err, http.ErrNoCookie) w.Header().Set("X-Forwarded-For", r.Header.Get("X-Forwarded-For")) + for name, values := range headers { + for _, value := range values { + w.Header().Add(name, value) + } + } w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(proxyTestAppBody)) }), diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go index 331934be6a4d0..1d3e8592d7a1c 100644 --- a/coderd/workspaceapps/proxy.go +++ b/coderd/workspaceapps/proxy.go @@ -22,6 +22,7 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/tracing" + "github.com/coder/coder/coderd/util/slice" "github.com/coder/coder/coderd/wsconncache" "github.com/coder/coder/codersdk" "github.com/coder/coder/site" @@ -541,6 +542,26 @@ func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appT defer release() proxy.Transport = conn.HTTPTransport() + proxy.ModifyResponse = func(r *http.Response) error { + r.Header.Del(httpmw.AccessControlAllowOriginHeader) + r.Header.Del(httpmw.AccessControlAllowCredentialsHeader) + r.Header.Del(httpmw.AccessControlAllowMethodsHeader) + r.Header.Del(httpmw.AccessControlAllowHeadersHeader) + varies := r.Header.Values(httpmw.VaryHeader) + r.Header.Del(httpmw.VaryHeader) + forbiddenVary := []string{ + httpmw.OriginHeader, + httpmw.AccessControlRequestMethodsHeader, + httpmw.AccessControlRequestHeadersHeader, + } + for _, value := range varies { + if !slice.ContainsCompare(forbiddenVary, value, strings.EqualFold) { + r.Header.Add(httpmw.VaryHeader, value) + } + } + return nil + } + // This strips the session token from a workspace app request. cookieHeaders := r.Header.Values("Cookie")[:] r.Header.Del("Cookie") diff --git a/docs/networking/port-forwarding.md b/docs/networking/port-forwarding.md index 50d61c52736df..3958f60eec4c9 100644 --- a/docs/networking/port-forwarding.md +++ b/docs/networking/port-forwarding.md @@ -124,12 +124,12 @@ will echo whatever the request sends. These cross-origin headers are not configurable by administrative settings. -Applications can set their own headers which will override the defaults but this -will only apply to non-preflight requests. Preflight requests through the -dashboard are never sent to applications and thus cannot be modified by -them. Read more about the difference between simple requests and requests that -trigger preflights -[here](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests). +If applications set any of the above headers they will be stripped from the +response except for `Vary` headers that are set to a value other than the ones +listed above. + +In other words, CORS behavior through the dashboard is not currently +configurable by either admins or users. #### Allowed by default