coder · code-asher · Jun 21, 2023 · Jun 20, 2023 · code-asher · Jun 16, 2023
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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