coder · code-asher · Jun 7, 2023 · May 25, 2023 · May 31, 2023 · May 31, 2023
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -408,7 +408,6 @@ func New(options *Options) *API {
 	prometheusMW := httpmw.Prometheus(options.PrometheusRegistry)
 
 	r.Use(
-		cors,
 		httpmw.Recover(api.Logger),
 		tracing.StatusWriterMiddleware,
 		tracing.Middleware(api.TracerProvider),
@@ -419,9 +418,10 @@ func New(options *Options) *API {
 		// SubdomainAppMW checks if the first subdomain is a valid app URL. If
 		// it is, it will serve that application.
 		//
-		// Workspace apps do their own auth and must be BEFORE the auth
-		// middleware.
+		// Workspace apps do their own auth and CORS and must be BEFORE the auth
+		// and CORS middleware.
 		api.workspaceAppServer.HandleSubdomain(apiRateLimiter),
+		cors,
 		// Build-Version is helpful for debugging.
 		func(next http.Handler) http.Handler {
 			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

diff --git a/coderd/httpmw/cors.go b/coderd/httpmw/cors.go
@@ -2,8 +2,12 @@ package httpmw
 
 import (
 	"net/http"
+	"net/url"
+	"regexp"
 
 	"github.com/go-chi/cors"
+
+	"github.com/coder/coder/coderd/httpapi"
 )
 
 //nolint:revive
@@ -25,3 +29,33 @@ func Cors(allowAll bool, origins ...string) func(next http.Handler) http.Handler
 		AllowCredentials: false,
 	})
 }
+
+func WorkspaceAppCors(regex *regexp.Regexp, app httpapi.ApplicationURL) func(next http.Handler) http.Handler {
+	return cors.Handler(cors.Options{
+		AllowOriginFunc: func(r *http.Request, rawOrigin string) bool {
+			origin, err := url.Parse(rawOrigin)
+			if rawOrigin == "" || origin.Host == "" || err != nil {
+				return false
+			}
+			subdomain, ok := httpapi.ExecuteHostnamePattern(regex, origin.Host)
+			if !ok {
+				return false
+			}
+			originApp, err := httpapi.ParseSubdomainAppURL(subdomain)
+			if err != nil {
+				return false
+			}
+			return ok && originApp.Username == app.Username
+		},
+		AllowedMethods: []string{
+			http.MethodHead,
+			http.MethodGet,
+			http.MethodPost,
+			http.MethodPut,
+			http.MethodPatch,
+			http.MethodDelete,
+		},
+		AllowedHeaders:   []string{"*"},
+		AllowCredentials: true,
+	})
+}
diff --git a/coderd/httpmw/cors_test.go b/coderd/httpmw/cors_test.go
@@ -0,0 +1,130 @@
+package httpmw_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/coderd/httpapi"
+	"github.com/coder/coder/coderd/httpmw"
+)
+
+func TestWorkspaceAppCors(t *testing.T) {
+	t.Parallel()
+
+	regex, err := httpapi.CompileHostnamePattern("*--apps.dev.coder.com")
+	require.NoError(t, err)
+
+	methods := []string{
+		http.MethodOptions,
+		http.MethodHead,
+		http.MethodGet,
+		http.MethodPost,
+		http.MethodPut,
+		http.MethodPatch,
+		http.MethodDelete,
+	}
+
+	tests := []struct {
+		name    string
+		origin  string
+		app     httpapi.ApplicationURL
+		allowed bool
+	}{
+		{
+			name:   "Self",
+			origin: "https://3000--agent--ws--user--apps.dev.coder.com",
+			app: httpapi.ApplicationURL{
+				AppSlugOrPort: "3000",
+				AgentName:     "agent",
+				WorkspaceName: "ws",
+				Username:      "user",
+			},
+			allowed: true,
+		},
+		{
+			name:   "SameWorkspace",
+			origin: "https://8000--agent--ws--user--apps.dev.coder.com",
+			app: httpapi.ApplicationURL{
+				AppSlugOrPort: "3000",
+				AgentName:     "agent",
+				WorkspaceName: "ws",
+				Username:      "user",
+			},
+			allowed: true,
+		},
+		{
+			name:   "SameUser",
+			origin: "https://8000--agent2--ws2--user--apps.dev.coder.com",
+			app: httpapi.ApplicationURL{
+				AppSlugOrPort: "3000",
+				AgentName:     "agent",
+				WorkspaceName: "ws",
+				Username:      "user",
+			},
+			allowed: true,
+		},
+		{
+			name:   "DifferentOriginOwner",
+			origin: "https://3000--agent--ws--user2--apps.dev.coder.com",
+			app: httpapi.ApplicationURL{
+				AppSlugOrPort: "3000",
+				AgentName:     "agent",
+				WorkspaceName: "ws",
+				Username:      "user",
+			},
+			allowed: false,
+		},
+		{
+			name:   "DifferentHostOwner",
+			origin: "https://3000--agent--ws--user--apps.dev.coder.com",
+			app: httpapi.ApplicationURL{
+				AppSlugOrPort: "3000",
+				AgentName:     "agent",
+				WorkspaceName: "ws",
+				Username:      "user2",
+			},
+			allowed: false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			for _, method := range methods {
+				r := httptest.NewRequest(method, "http://localhost", nil)
+				r.Header.Set("Origin", test.origin)
+				rw := httptest.NewRecorder()
+
+				// Preflight requests need to know what method will be requested.
+				if method == http.MethodOptions {
+					r.Header.Set("Access-Control-Request-Method", method)
+				}
+
+				handler := httpmw.WorkspaceAppCors(regex, test.app)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+					rw.WriteHeader(http.StatusNoContent)
+				}))
+
+				handler.ServeHTTP(rw, r)
+
+				if test.allowed {
+					require.Equal(t, test.origin, rw.Header().Get("Access-Control-Allow-Origin"))
+				} else {
+					require.Equal(t, "", rw.Header().Get("Access-Control-Allow-Origin"))
+				}
+
+				// For options we should never get to our handler as the middleware
+				// short-circuits with a 200.
+				if method == http.MethodOptions {
+					require.Equal(t, http.StatusOK, rw.Code)
+				} else {
+					require.Equal(t, http.StatusNoContent, rw.Code)
+				}
+			}
+		})
+	}
+}
diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go
@@ -361,35 +361,34 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler)
 				return
 			}
 
-			if !s.handleAPIKeySmuggling(rw, r, AccessMethodSubdomain) {
-				return
-			}
-
-			token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
-				Logger:              s.Logger,
-				SignedTokenProvider: s.SignedTokenProvider,
-				DashboardURL:        s.DashboardURL,
-				PathAppBaseURL:      s.AccessURL,
-				AppHostname:         s.Hostname,
-				AppRequest: Request{
-					AccessMethod:      AccessMethodSubdomain,
-					BasePath:          "/",
-					UsernameOrID:      app.Username,
-					WorkspaceNameOrID: app.WorkspaceName,
-					AgentNameOrID:     app.AgentName,
-					AppSlugOrPort:     app.AppSlugOrPort,
-				},
-				AppPath:  r.URL.Path,
-				AppQuery: r.URL.RawQuery,
-			})
-			if !ok {
-				return
-			}
-
-			// Use the passed in app middlewares before passing to the proxy
-			// app.
-			mws := chi.Middlewares(middlewares)
+			// Use the passed in app middlewares before checking authentication and
+			// passing to the proxy app.
+			mws := chi.Middlewares(append(middlewares, httpmw.WorkspaceAppCors(s.HostnameRegex, app)))
 			mws.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+				if !s.handleAPIKeySmuggling(rw, r, AccessMethodSubdomain) {
+					return
+				}
+
+				token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
+					Logger:              s.Logger,
+					SignedTokenProvider: s.SignedTokenProvider,
+					DashboardURL:        s.DashboardURL,
+					PathAppBaseURL:      s.AccessURL,
+					AppHostname:         s.Hostname,
+					AppRequest: Request{
+						AccessMethod:      AccessMethodSubdomain,
+						BasePath:          "/",
+						UsernameOrID:      app.Username,
+						WorkspaceNameOrID: app.WorkspaceName,
+						AgentNameOrID:     app.AgentName,
+						AppSlugOrPort:     app.AppSlugOrPort,
+					},
+					AppPath:  r.URL.Path,
+					AppQuery: r.URL.RawQuery,
+				})
+				if !ok {
+					return
+				}
 				s.proxyWorkspaceApp(rw, r, *token, r.URL.Path)
 			})).ServeHTTP(rw, r.WithContext(ctx))
 		})

diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -1170,7 +1170,7 @@ when required by your organization's security policy.`,
 		// ☢️ Dangerous settings
 		{
 			Name:        "DANGEROUS: Allow all CORs requests",
-			Description: "For security reasons, CORs requests are blocked. If external requests are required, setting this to true will set all cors headers as '*'. This should never be used in production.",
+			Description: "For security reasons, CORs requests are blocked except between workspace apps owned by the same user. If external requests are required, setting this to true will set all cors headers as '*'. This should never be used in production.",
 			Flag:        "dangerous-allow-cors-requests",
 			Env:         "CODER_DANGEROUS_ALLOW_CORS_REQUESTS",
 			Hidden:      true, // Hidden, should only be used by yarn dev server