feat: allow cross-origin requests between users' own apps (#7688)

This commit is contained in:
Asher
2023-06-07 11:08:14 -08:00
committed by GitHub
parent 125e9ef00e
commit f0c5201617
5 changed files with 195 additions and 32 deletions

View File

@ -408,7 +408,6 @@ func New(options *Options) *API {
prometheusMW := httpmw.Prometheus(options.PrometheusRegistry) prometheusMW := httpmw.Prometheus(options.PrometheusRegistry)
r.Use( r.Use(
cors,
httpmw.Recover(api.Logger), httpmw.Recover(api.Logger),
tracing.StatusWriterMiddleware, tracing.StatusWriterMiddleware,
tracing.Middleware(api.TracerProvider), 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 // SubdomainAppMW checks if the first subdomain is a valid app URL. If
// it is, it will serve that application. // it is, it will serve that application.
// //
// Workspace apps do their own auth and must be BEFORE the auth // Workspace apps do their own auth and CORS and must be BEFORE the auth
// middleware. // and CORS middleware.
api.workspaceAppServer.HandleSubdomain(apiRateLimiter), api.workspaceAppServer.HandleSubdomain(apiRateLimiter),
cors,
// Build-Version is helpful for debugging. // Build-Version is helpful for debugging.
func(next http.Handler) http.Handler { func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

View File

@ -2,8 +2,12 @@ package httpmw
import ( import (
"net/http" "net/http"
"net/url"
"regexp"
"github.com/go-chi/cors" "github.com/go-chi/cors"
"github.com/coder/coder/coderd/httpapi"
) )
//nolint:revive //nolint:revive
@ -25,3 +29,33 @@ func Cors(allowAll bool, origins ...string) func(next http.Handler) http.Handler
AllowCredentials: false, 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,
})
}

130
coderd/httpmw/cors_test.go Normal file
View File

@ -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)
}
}
})
}
}

View File

@ -361,35 +361,34 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler)
return return
} }
if !s.handleAPIKeySmuggling(rw, r, AccessMethodSubdomain) { // Use the passed in app middlewares before checking authentication and
return // passing to the proxy app.
} mws := chi.Middlewares(append(middlewares, httpmw.WorkspaceAppCors(s.HostnameRegex, app)))
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)
mws.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { 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) s.proxyWorkspaceApp(rw, r, *token, r.URL.Path)
})).ServeHTTP(rw, r.WithContext(ctx)) })).ServeHTTP(rw, r.WithContext(ctx))
}) })

View File

@ -1170,7 +1170,7 @@ when required by your organization's security policy.`,
// ☢️ Dangerous settings // ☢️ Dangerous settings
{ {
Name: "DANGEROUS: Allow all CORs requests", 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", Flag: "dangerous-allow-cors-requests",
Env: "CODER_DANGEROUS_ALLOW_CORS_REQUESTS", Env: "CODER_DANGEROUS_ALLOW_CORS_REQUESTS",
Hidden: true, // Hidden, should only be used by yarn dev server Hidden: true, // Hidden, should only be used by yarn dev server