From 35ee5d665650e84a6c091c17697519ef3fbf9952 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 26 Aug 2022 08:51:36 -0400 Subject: [PATCH 01/24] chore: Add subdomain parser for applications --- coderd/subdomain.go | 115 +++++++++++++++++++++++++++++++++++++++ coderd/subdomain_test.go | 108 ++++++++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 coderd/subdomain.go create mode 100644 coderd/subdomain_test.go diff --git a/coderd/subdomain.go b/coderd/subdomain.go new file mode 100644 index 0000000000000..61067f498af0b --- /dev/null +++ b/coderd/subdomain.go @@ -0,0 +1,115 @@ +package coderd + +import ( + "fmt" + "net/http" + "regexp" + "strings" + + "golang.org/x/xerrors" +) + +const ( + // XForwardedHostHeader is a header used by proxies to indicate the + // original host of the request. + XForwardedHostHeader = "X-Forwarded-Host" + xForwardedProto = "X-Forwarded-Proto" +) + +type Application struct { + AppURL string + AppName string + Workspace string + Agent string + User string + Path string + + // Domain is used to output the url to reach the app. + Domain string +} + +func (api *API) handleSubdomain(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + }) +} + +var ( + nameRegex = `[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*` + appURL = regexp.MustCompile(fmt.Sprintf( + // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} + `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, + nameRegex)) +) + +func ParseSubdomainAppURL(r *http.Request) (Application, error) { + host := RequestHost(r) + if host == "" { + return Application{}, xerrors.Errorf("no host header") + } + + subdomain, domain, err := SplitSubdomain(host) + if err != nil { + return Application{}, xerrors.Errorf("split host domain: %w", err) + } + + matches := appURL.FindAllStringSubmatch(subdomain, -1) + if len(matches) == 0 { + return Application{}, xerrors.Errorf("invalid application url format: %q", subdomain) + } + + if len(matches) > 1 { + return Application{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) + } + matchGroup := matches[0] + + return Application{ + AppURL: "", + AppName: matchGroup[appURL.SubexpIndex("AppName")], + Workspace: matchGroup[appURL.SubexpIndex("WorkspaceName")], + Agent: matchGroup[appURL.SubexpIndex("AgentName")], + User: matchGroup[appURL.SubexpIndex("UserName")], + Path: r.URL.Path, + Domain: domain, + }, nil +} + +// Parse parses a DevURL from the subdomain of r's Host header. +// If DevURL is not valid, returns a non-nil error. +// +// devurls can be in two forms, each field separate by 2 hypthens: +// 1) port-envname-user (eg. http://8080--myenv--johndoe.cdrdeploy.c8s.io) +// 2) name-user (eg. http://demosvc--johndoe.cdrdeploy.c8s.io) +// +// Note that envname itself can contain hyphens. +// If subdomain begins with a sequence of numbers, form 1 is assumed. +// Otherwise, form 2 is assumed. +//func Parse(r *http.Request, devurlSuffix string) (Application, error) { +// +// return d, nil +//} + +// RequestHost returns the name of the host from the request. It prioritizes +// 'X-Forwarded-Host' over r.Host since most requests are being proxied. +func RequestHost(r *http.Request) string { + host := r.Header.Get(XForwardedHostHeader) + if host != "" { + return host + } + + return r.Host +} + +// SplitSubdomain splits a subdomain from a domain. E.g.: +// - "foo.bar.com" becomes "foo", "bar.com" +// - "foo.bar.baz.com" becomes "foo", "bar.baz.com" +// +// An error is returned if the string doesn't contain a period. +func SplitSubdomain(hostname string) (string, string, error) { + toks := strings.SplitN(hostname, ".", 2) + if len(toks) < 2 { + return "", "", xerrors.Errorf("no domain") + } + + return toks[0], toks[1], nil +} diff --git a/coderd/subdomain_test.go b/coderd/subdomain_test.go new file mode 100644 index 0000000000000..5eadec8036cdb --- /dev/null +++ b/coderd/subdomain_test.go @@ -0,0 +1,108 @@ +package coderd_test + +import ( + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd" +) + +func TestParseSubdomainAppURL(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + URL string + Expected coderd.Application + ExpectedError string + }{ + { + Name: "Empty", + URL: "https://example.com", + Expected: coderd.Application{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Workspace.Agent+App", + URL: "https://workspace.agent--app.coder.com", + Expected: coderd.Application{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Workspace+App", + URL: "https://workspace--app.coder.com", + Expected: coderd.Application{}, + ExpectedError: "invalid application url format", + }, + // Correct + { + Name: "User+Workspace+App", + URL: "https://user--workspace--app.coder.com", + Expected: coderd.Application{ + AppURL: "", + AppName: "app", + Workspace: "workspace", + Agent: "", + User: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace+Port", + URL: "https://user--workspace--8080.coder.com", + Expected: coderd.Application{ + AppURL: "", + AppName: "8080", + Workspace: "workspace", + Agent: "", + User: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace.Agent+App", + URL: "https://user--workspace--agent--app.coder.com", + Expected: coderd.Application{ + AppURL: "", + AppName: "app", + Workspace: "workspace", + Agent: "agent", + User: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace.Agent+Port", + URL: "https://user--workspace--agent--8080.coder.com", + Expected: coderd.Application{ + AppURL: "", + AppName: "8080", + Workspace: "workspace", + Agent: "agent", + User: "user", + Path: "", + Domain: "coder.com", + }, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + r := httptest.NewRequest("GET", c.URL, nil) + + app, err := coderd.ParseSubdomainAppURL(r) + if c.ExpectedError == "" { + require.NoError(t, err) + require.Equal(t, c.Expected, app, "expected app") + } else { + require.ErrorContains(t, err, c.ExpectedError, "expected error") + } + }) + } +} From a7e5bd657cdfa8a7d7ad6398ec2c96942336fa2d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 26 Aug 2022 09:07:52 -0400 Subject: [PATCH 02/24] Add basic router for applications using same codepath --- coderd/subdomain.go | 82 ++++++++++++++++++++++++++++++++-------- coderd/subdomain_test.go | 56 +++++++++++++-------------- coderd/workspaceapps.go | 27 +++++++++++-- 3 files changed, 119 insertions(+), 46 deletions(-) diff --git a/coderd/subdomain.go b/coderd/subdomain.go index 61067f498af0b..df7816f97a552 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -1,11 +1,17 @@ package coderd import ( + "database/sql" "fmt" "net/http" "regexp" "strings" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/codersdk" + + "github.com/coder/coder/coderd/database" + "golang.org/x/xerrors" ) @@ -17,20 +23,66 @@ const ( ) type Application struct { - AppURL string - AppName string - Workspace string - Agent string - User string - Path string + AppURL string + AppName string + WorkspaceName string + Agent string + Username string + Path string // Domain is used to output the url to reach the app. Domain string } func (api *API) handleSubdomain(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + app, err := ParseSubdomainAppURL(r) + if err != nil { + // Not a Dev URL, proceed as usual. + // TODO: @emyrk we should probably catch invalid subdomains. Meaning + // an invalid devurl should not route to the coderd. + next.ServeHTTP(rw, r) + return + } + + user, err := api.Database.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{ + Username: app.Username, + }) + if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + httpapi.ResourceNotFound(rw) + return + } + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching user.", + Detail: err.Error(), + }) + return + } + + workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{ + OwnerID: user.ID, + Name: app.WorkspaceName, + }) + if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + httpapi.ResourceNotFound(rw) + return + } + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace.", + Detail: err.Error(), + }) + return + } + + api.proxyWorkspaceApplication(proxyApplication{ + Workspace: workspace, + // TODO: Fetch workspace agent + Agent: database.WorkspaceAgent{}, + AppName: app.AppName, + }, rw, r) }) } @@ -64,13 +116,13 @@ func ParseSubdomainAppURL(r *http.Request) (Application, error) { matchGroup := matches[0] return Application{ - AppURL: "", - AppName: matchGroup[appURL.SubexpIndex("AppName")], - Workspace: matchGroup[appURL.SubexpIndex("WorkspaceName")], - Agent: matchGroup[appURL.SubexpIndex("AgentName")], - User: matchGroup[appURL.SubexpIndex("UserName")], - Path: r.URL.Path, - Domain: domain, + AppURL: "", + AppName: matchGroup[appURL.SubexpIndex("AppName")], + WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], + Agent: matchGroup[appURL.SubexpIndex("AgentName")], + Username: matchGroup[appURL.SubexpIndex("UserName")], + Path: r.URL.Path, + Domain: domain, }, nil } diff --git a/coderd/subdomain_test.go b/coderd/subdomain_test.go index 5eadec8036cdb..b7fbec12ce47a 100644 --- a/coderd/subdomain_test.go +++ b/coderd/subdomain_test.go @@ -40,52 +40,52 @@ func TestParseSubdomainAppURL(t *testing.T) { Name: "User+Workspace+App", URL: "https://user--workspace--app.coder.com", Expected: coderd.Application{ - AppURL: "", - AppName: "app", - Workspace: "workspace", - Agent: "", - User: "user", - Path: "", - Domain: "coder.com", + AppURL: "", + AppName: "app", + WorkspaceName: "workspace", + Agent: "", + User: "user", + Path: "", + Domain: "coder.com", }, }, { Name: "User+Workspace+Port", URL: "https://user--workspace--8080.coder.com", Expected: coderd.Application{ - AppURL: "", - AppName: "8080", - Workspace: "workspace", - Agent: "", - User: "user", - Path: "", - Domain: "coder.com", + AppURL: "", + AppName: "8080", + WorkspaceName: "workspace", + Agent: "", + User: "user", + Path: "", + Domain: "coder.com", }, }, { Name: "User+Workspace.Agent+App", URL: "https://user--workspace--agent--app.coder.com", Expected: coderd.Application{ - AppURL: "", - AppName: "app", - Workspace: "workspace", - Agent: "agent", - User: "user", - Path: "", - Domain: "coder.com", + AppURL: "", + AppName: "app", + WorkspaceName: "workspace", + Agent: "agent", + User: "user", + Path: "", + Domain: "coder.com", }, }, { Name: "User+Workspace.Agent+Port", URL: "https://user--workspace--agent--8080.coder.com", Expected: coderd.Application{ - AppURL: "", - AppName: "8080", - Workspace: "workspace", - Agent: "agent", - User: "user", - Path: "", - Domain: "coder.com", + AppURL: "", + AppName: "8080", + WorkspaceName: "workspace", + Agent: "agent", + User: "user", + Path: "", + Domain: "coder.com", }, }, } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 4f9ef2d6ae6b8..fc2581977c44f 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -96,9 +96,30 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) } } + api.proxyWorkspaceApplication(proxyApplication{ + Workspace: workspace, + Agent: agent, + AppName: chi.URLParam(r, "workspaceapp"), + }, rw, r) +} + +// proxyApplication are the required fields to proxy a workspace application. +type proxyApplication struct { + Workspace database.Workspace + Agent database.WorkspaceAgent + + AppName string +} + +func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { + if !api.Authorize(r, rbac.ActionCreate, proxyApp.Workspace.ExecutionRBAC()) { + httpapi.ResourceNotFound(rw) + return + } + app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ - AgentID: agent.ID, - Name: chi.URLParam(r, "workspaceapp"), + AgentID: proxyApp.Agent.ID, + Name: proxyApp.AppName, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusNotFound, codersdk.Response{ @@ -161,7 +182,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) } r.URL.Path = path - conn, release, err := api.workspaceAgentCache.Acquire(r, agent.ID) + conn, release, err := api.workspaceAgentCache.Acquire(r, proxyApp.Agent.ID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to dial workspace agent.", From 3e30cddefd4c015ca3435b11868d59e4ba6d2eae Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 29 Aug 2022 14:23:50 -0400 Subject: [PATCH 03/24] Handle ports as app names --- coderd/coderd.go | 8 +++- coderd/subdomain.go | 86 ++++++++++++++++------------------------ coderd/subdomain_test.go | 21 ++++++++-- coderd/workspaceapps.go | 56 +++++++++++++++----------- 4 files changed, 92 insertions(+), 79 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 640c4b5f52cb6..d83cbdf3729c6 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -131,7 +131,13 @@ func New(options *Options) *API { }) }, httpmw.Prometheus(options.PrometheusRegistry), - api.handleSubdomain, + // Handle all subdomain requests + api.handleSubdomain( + httpmw.RateLimitPerMinute(options.APIRateLimit), + httpmw.ExtractAPIKey(options.Database, oauthConfigs, false), + httpmw.ExtractUserParam(api.Database), + httpmw.ExtractWorkspaceAndAgentParam(api.Database), + ), ) apps := func(r chi.Router) { diff --git a/coderd/subdomain.go b/coderd/subdomain.go index df7816f97a552..215b7d3f17e9a 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -1,16 +1,14 @@ package coderd import ( - "database/sql" "fmt" "net/http" "regexp" "strings" - "github.com/coder/coder/coderd/httpapi" - "github.com/coder/coder/codersdk" + "github.com/coder/coder/coderd/httpmw" - "github.com/coder/coder/coderd/database" + "github.com/go-chi/chi/v5" "golang.org/x/xerrors" ) @@ -34,56 +32,42 @@ type Application struct { Domain string } -func (api *API) handleSubdomain(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - app, err := ParseSubdomainAppURL(r) - if err != nil { - // Not a Dev URL, proceed as usual. - // TODO: @emyrk we should probably catch invalid subdomains. Meaning - // an invalid devurl should not route to the coderd. - next.ServeHTTP(rw, r) - return - } - - user, err := api.Database.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{ - Username: app.Username, - }) - if err != nil { - if xerrors.Is(err, sql.ErrNoRows) { - httpapi.ResourceNotFound(rw) +func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + app, err := ParseSubdomainAppURL(r) + + if err != nil { + // Not a Dev URL, proceed as usual. + // TODO: @emyrk we should probably catch invalid subdomains. Meaning + // an invalid devurl should not route to the coderd. + next.ServeHTTP(rw, r) return } - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching user.", - Detail: err.Error(), - }) - return - } - - workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{ - OwnerID: user.ID, - Name: app.WorkspaceName, - }) - if err != nil { - if xerrors.Is(err, sql.ErrNoRows) { - httpapi.ResourceNotFound(rw) - return + + workspaceAgentKey := app.WorkspaceName + if app.Agent != "" { + workspaceAgentKey += "." + app.Agent } - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace.", - Detail: err.Error(), - }) - return - } - - api.proxyWorkspaceApplication(proxyApplication{ - Workspace: workspace, - // TODO: Fetch workspace agent - Agent: database.WorkspaceAgent{}, - AppName: app.AppName, - }, rw, r) - }) + chiCtx := chi.RouteContext(ctx) + chiCtx.URLParams.Add("workspace_and_agent", workspaceAgentKey) + chiCtx.URLParams.Add("user", app.Username) + + // 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) { + workspace := httpmw.WorkspaceParam(r) + agent := httpmw.WorkspaceAgentParam(r) + + api.proxyWorkspaceApplication(proxyApplication{ + Workspace: workspace, + Agent: agent, + AppName: app.AppName, + }, rw, r) + })).ServeHTTP(rw, r.WithContext(ctx)) + }) + } } var ( diff --git a/coderd/subdomain_test.go b/coderd/subdomain_test.go index b7fbec12ce47a..4425cb7362109 100644 --- a/coderd/subdomain_test.go +++ b/coderd/subdomain_test.go @@ -44,7 +44,7 @@ func TestParseSubdomainAppURL(t *testing.T) { AppName: "app", WorkspaceName: "workspace", Agent: "", - User: "user", + Username: "user", Path: "", Domain: "coder.com", }, @@ -57,7 +57,7 @@ func TestParseSubdomainAppURL(t *testing.T) { AppName: "8080", WorkspaceName: "workspace", Agent: "", - User: "user", + Username: "user", Path: "", Domain: "coder.com", }, @@ -70,7 +70,7 @@ func TestParseSubdomainAppURL(t *testing.T) { AppName: "app", WorkspaceName: "workspace", Agent: "agent", - User: "user", + Username: "user", Path: "", Domain: "coder.com", }, @@ -83,7 +83,20 @@ func TestParseSubdomainAppURL(t *testing.T) { AppName: "8080", WorkspaceName: "workspace", Agent: "agent", - User: "user", + Username: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "HyphenatedNames", + URL: "https://admin-user--workspace-thing--agent-thing--app-name.coder.com", + Expected: coderd.Application{ + AppURL: "", + AppName: "app-name", + WorkspaceName: "workspace-thing", + Agent: "agent-thing", + Username: "admin-user", Path: "", Domain: "coder.com", }, diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index f529800ed2ed8..1fec64dd13fdd 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httputil" "net/url" + "strconv" "strings" "github.com/coder/coder/coderd/database" @@ -51,34 +52,43 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } - app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ - AgentID: proxyApp.Agent.ID, - Name: proxyApp.AppName, - }) - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(rw, http.StatusNotFound, codersdk.Response{ - Message: "Application not found.", - }) - return - } - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace application.", - Detail: err.Error(), - }) - return - } - if !app.Url.Valid { - httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Application %s does not have a url.", app.Name), + var internalURL string + + num, err := strconv.Atoi(proxyApp.AppName) + if err == nil && num <= 65535 { + // TODO: @emyrk we should probably allow changing the schema? + internalURL = "http://localhost:" + proxyApp.AppName + } else { + app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ + AgentID: proxyApp.Agent.ID, + Name: proxyApp.AppName, }) - return + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(rw, http.StatusNotFound, codersdk.Response{ + Message: "Application not found.", + }) + return + } + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace application.", + Detail: err.Error(), + }) + return + } + if !app.Url.Valid { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Application %s does not have a url.", app.Name), + }) + return + } + internalURL = app.Url.String } - appURL, err := url.Parse(app.Url.String) + appURL, err := url.Parse(internalURL) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: fmt.Sprintf("App url %q must be a valid url.", app.Url.String), + Message: fmt.Sprintf("App url %q must be a valid url.", internalURL), Detail: err.Error(), }) return From 83973061a8d636e0b2f74202a41e66be998bde6b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 08:46:05 -0400 Subject: [PATCH 04/24] Add comments --- coderd/coderd.go | 4 +++- coderd/subdomain.go | 19 ++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index d83cbdf3729c6..1a4b257d611da 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -131,8 +131,10 @@ func New(options *Options) *API { }) }, httpmw.Prometheus(options.PrometheusRegistry), - // Handle all subdomain requests + // handleSubdomain checks if the first subdomain is a valid app url. + // If it is, it will serve that application. api.handleSubdomain( + // Middleware to impose on the served application. httpmw.RateLimitPerMinute(options.APIRateLimit), httpmw.ExtractAPIKey(options.Database, oauthConfigs, false), httpmw.ExtractUserParam(api.Database), diff --git a/coderd/subdomain.go b/coderd/subdomain.go index 215b7d3f17e9a..b22498ee27534 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -78,6 +78,10 @@ var ( nameRegex)) ) +// ParseSubdomainAppURL parses an application from the subdomain of r's Host header. +// If the application string is not valid, returns a non-nil error. +// 1) {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT/AppName}} +// (eg. http://admin--myenv--main--8080.cdrdeploy.c8s.io) func ParseSubdomainAppURL(r *http.Request) (Application, error) { host := RequestHost(r) if host == "" { @@ -110,21 +114,6 @@ func ParseSubdomainAppURL(r *http.Request) (Application, error) { }, nil } -// Parse parses a DevURL from the subdomain of r's Host header. -// If DevURL is not valid, returns a non-nil error. -// -// devurls can be in two forms, each field separate by 2 hypthens: -// 1) port-envname-user (eg. http://8080--myenv--johndoe.cdrdeploy.c8s.io) -// 2) name-user (eg. http://demosvc--johndoe.cdrdeploy.c8s.io) -// -// Note that envname itself can contain hyphens. -// If subdomain begins with a sequence of numbers, form 1 is assumed. -// Otherwise, form 2 is assumed. -//func Parse(r *http.Request, devurlSuffix string) (Application, error) { -// -// return d, nil -//} - // RequestHost returns the name of the host from the request. It prioritizes // 'X-Forwarded-Host' over r.Host since most requests are being proxied. func RequestHost(r *http.Request) string { From f994ec3bdb214fc6a764438e8a48c482e9da7075 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 09:31:13 -0400 Subject: [PATCH 05/24] Cleanup --- coderd/coderd.go | 2 ++ coderd/subdomain.go | 24 ++++++++++----------- coderd/subdomain_test.go | 23 ++++++++------------ coderd/workspaceapps.go | 45 ++++++++++++++++++++-------------------- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 1a4b257d611da..7ebaa76827e74 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -136,6 +136,8 @@ func New(options *Options) *API { api.handleSubdomain( // Middleware to impose on the served application. httpmw.RateLimitPerMinute(options.APIRateLimit), + // This should extract the application specific API key when we + // implement a scoped token. httpmw.ExtractAPIKey(options.Database, oauthConfigs, false), httpmw.ExtractUserParam(api.Database), httpmw.ExtractWorkspaceAndAgentParam(api.Database), diff --git a/coderd/subdomain.go b/coderd/subdomain.go index b22498ee27534..25def19d1c786 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -17,11 +17,10 @@ const ( // XForwardedHostHeader is a header used by proxies to indicate the // original host of the request. XForwardedHostHeader = "X-Forwarded-Host" - xForwardedProto = "X-Forwarded-Proto" ) -type Application struct { - AppURL string +// ApplicationURL is a parsed application url into it's components +type ApplicationURL struct { AppName string WorkspaceName string Agent string @@ -39,9 +38,11 @@ func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) app, err := ParseSubdomainAppURL(r) if err != nil { - // Not a Dev URL, proceed as usual. + // Subdomain is not a valid application url. Pass through. // TODO: @emyrk we should probably catch invalid subdomains. Meaning - // an invalid devurl should not route to the coderd. + // an invalid application should not route to the coderd. + // To do this we would need to know the list of valid access urls + // though? next.ServeHTTP(rw, r) return } @@ -82,29 +83,28 @@ var ( // If the application string is not valid, returns a non-nil error. // 1) {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT/AppName}} // (eg. http://admin--myenv--main--8080.cdrdeploy.c8s.io) -func ParseSubdomainAppURL(r *http.Request) (Application, error) { +func ParseSubdomainAppURL(r *http.Request) (ApplicationURL, error) { host := RequestHost(r) if host == "" { - return Application{}, xerrors.Errorf("no host header") + return ApplicationURL{}, xerrors.Errorf("no host header") } subdomain, domain, err := SplitSubdomain(host) if err != nil { - return Application{}, xerrors.Errorf("split host domain: %w", err) + return ApplicationURL{}, xerrors.Errorf("split host domain: %w", err) } matches := appURL.FindAllStringSubmatch(subdomain, -1) if len(matches) == 0 { - return Application{}, xerrors.Errorf("invalid application url format: %q", subdomain) + return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) } if len(matches) > 1 { - return Application{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) + return ApplicationURL{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) } matchGroup := matches[0] - return Application{ - AppURL: "", + return ApplicationURL{ AppName: matchGroup[appURL.SubexpIndex("AppName")], WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], Agent: matchGroup[appURL.SubexpIndex("AgentName")], diff --git a/coderd/subdomain_test.go b/coderd/subdomain_test.go index 4425cb7362109..ac835f292b8df 100644 --- a/coderd/subdomain_test.go +++ b/coderd/subdomain_test.go @@ -14,33 +14,32 @@ func TestParseSubdomainAppURL(t *testing.T) { testCases := []struct { Name string URL string - Expected coderd.Application + Expected coderd.ApplicationURL ExpectedError string }{ { Name: "Empty", URL: "https://example.com", - Expected: coderd.Application{}, + Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Workspace.Agent+App", URL: "https://workspace.agent--app.coder.com", - Expected: coderd.Application{}, + Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Workspace+App", URL: "https://workspace--app.coder.com", - Expected: coderd.Application{}, + Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, // Correct { Name: "User+Workspace+App", URL: "https://user--workspace--app.coder.com", - Expected: coderd.Application{ - AppURL: "", + Expected: coderd.ApplicationURL{ AppName: "app", WorkspaceName: "workspace", Agent: "", @@ -52,8 +51,7 @@ func TestParseSubdomainAppURL(t *testing.T) { { Name: "User+Workspace+Port", URL: "https://user--workspace--8080.coder.com", - Expected: coderd.Application{ - AppURL: "", + Expected: coderd.ApplicationURL{ AppName: "8080", WorkspaceName: "workspace", Agent: "", @@ -65,8 +63,7 @@ func TestParseSubdomainAppURL(t *testing.T) { { Name: "User+Workspace.Agent+App", URL: "https://user--workspace--agent--app.coder.com", - Expected: coderd.Application{ - AppURL: "", + Expected: coderd.ApplicationURL{ AppName: "app", WorkspaceName: "workspace", Agent: "agent", @@ -78,8 +75,7 @@ func TestParseSubdomainAppURL(t *testing.T) { { Name: "User+Workspace.Agent+Port", URL: "https://user--workspace--agent--8080.coder.com", - Expected: coderd.Application{ - AppURL: "", + Expected: coderd.ApplicationURL{ AppName: "8080", WorkspaceName: "workspace", Agent: "agent", @@ -91,8 +87,7 @@ func TestParseSubdomainAppURL(t *testing.T) { { Name: "HyphenatedNames", URL: "https://admin-user--workspace-thing--agent-thing--app-name.coder.com", - Expected: coderd.Application{ - AppURL: "", + Expected: coderd.ApplicationURL{ AppName: "app-name", WorkspaceName: "workspace-thing", Agent: "agent-thing", diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 1fec64dd13fdd..94f96b06769cb 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -54,28 +54,15 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res var internalURL string - num, err := strconv.Atoi(proxyApp.AppName) - if err == nil && num <= 65535 { - // TODO: @emyrk we should probably allow changing the schema? - internalURL = "http://localhost:" + proxyApp.AppName - } else { - app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ - AgentID: proxyApp.Agent.ID, - Name: proxyApp.AppName, - }) - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(rw, http.StatusNotFound, codersdk.Response{ - Message: "Application not found.", - }) - return - } - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace application.", - Detail: err.Error(), - }) - return - } + // Fetch the app from the database. If the app does not exist, check if + // the app is a port number + num, numError := strconv.Atoi(proxyApp.AppName) + app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ + AgentID: proxyApp.Agent.ID, + Name: proxyApp.AppName, + }) + switch { + case err == nil: if !app.Url.Valid { httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ Message: fmt.Sprintf("Application %s does not have a url.", app.Name), @@ -83,6 +70,20 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } internalURL = app.Url.String + case err != nil && errors.Is(err, sql.ErrNoRows) && numError == nil && num <= 65535: + // If the app does not exist, but the app name is a port number, then + // route to the port as an anonymous app. + + // Anonymous apps will default to `http`. If the user wants to configure + // particular app settings, they will have to name it. + internalURL = "http://localhost:" + proxyApp.AppName + case err != nil: + // All other db errors, return an error. + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace application.", + Detail: err.Error(), + }) + return } appURL, err := url.Parse(internalURL) From fda80b88f71c55392e59b9f2cb72b363f186ecee Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 09:48:33 -0400 Subject: [PATCH 06/24] Linting --- coderd/subdomain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/subdomain.go b/coderd/subdomain.go index 25def19d1c786..db10bfe382a70 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -130,7 +130,7 @@ func RequestHost(r *http.Request) string { // - "foo.bar.baz.com" becomes "foo", "bar.baz.com" // // An error is returned if the string doesn't contain a period. -func SplitSubdomain(hostname string) (string, string, error) { +func SplitSubdomain(hostname string) (subdomain string, domain string, err error) { toks := strings.SplitN(hostname, ".", 2) if len(toks) < 2 { return "", "", xerrors.Errorf("no domain") From 6b09a0f040297777e47df5856f9e7f58335a5c46 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 10:01:33 -0400 Subject: [PATCH 07/24] Push cookies to subdomains on the access url as well --- coderd/subdomain.go | 14 ++++++++++++++ coderd/userauth.go | 2 ++ coderd/users.go | 2 ++ 3 files changed, 18 insertions(+) diff --git a/coderd/subdomain.go b/coderd/subdomain.go index db10bfe382a70..3dc404e8d61cb 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -138,3 +138,17 @@ func SplitSubdomain(hostname string) (subdomain string, domain string, err error return toks[0], toks[1], nil } + +// applicationCookie is a helper function to copy the auth cookie to also +// support subdomains. Until we support creating authentication cookies that can +// only do application authentication, we will just reuse the original token. +// This code should be temporary and be replaced with something that creates +// a unique session_token. +func (api *API) applicationCookie(authCookie *http.Cookie) *http.Cookie { + appCookie := *authCookie + // We only support setting this cookie on the access url subdomains. + // This is to ensure we don't accidentally leak the auth cookie to subdomains + // on another hostname. + appCookie.Domain = "." + api.AccessURL.Hostname() + return &appCookie +} diff --git a/coderd/userauth.go b/coderd/userauth.go index ff45846de369c..28b1fca7d677d 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -162,6 +162,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { } http.SetCookie(rw, cookie) + http.SetCookie(rw, api.applicationCookie(cookie)) redirect := state.Redirect if redirect == "" { @@ -274,6 +275,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } http.SetCookie(rw, cookie) + http.SetCookie(rw, api.applicationCookie(cookie)) redirect := state.Redirect if redirect == "" { diff --git a/coderd/users.go b/coderd/users.go index 08175455b312e..5fdfbbdc8cd93 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -796,6 +796,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { } http.SetCookie(rw, cookie) + http.SetCookie(rw, api.applicationCookie(cookie)) httpapi.Write(rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ SessionToken: cookie.Value, @@ -874,6 +875,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { } http.SetCookie(rw, cookie) + http.SetCookie(rw, api.applicationCookie(cookie)) // Delete the session token from database. apiKey := httpmw.APIKey(r) From 634cd2e04445b8ca72b8b72e95fd2d43540a4512 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 10:26:05 -0400 Subject: [PATCH 08/24] Fix unit test --- coderd/users_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index e2752987f6951..699b3ab534a4e 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -281,9 +281,10 @@ func TestPostLogout(t *testing.T) { require.Equal(t, http.StatusOK, res.StatusCode) cookies := res.Cookies() - require.Len(t, cookies, 1, "Exactly one cookie should be returned") + require.Len(t, cookies, 2, "Exactly two cookies should be returned") - require.Equal(t, codersdk.SessionTokenKey, cookies[0].Name, "Cookie should be the auth cookie") + require.Equal(t, codersdk.SessionTokenKey, cookies[0].Name, "Cookie should be the auth & app cookie") + require.Equal(t, codersdk.SessionTokenKey, cookies[1].Name, "Cookie should be the auth & app cookie") require.Equal(t, -1, cookies[0].MaxAge, "Cookie should be set to delete") _, err = client.GetAPIKey(ctx, admin.UserID.String(), keyID) From 82df6f183321dd27eed748da39dd1b8ff951abb7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 10:32:06 -0400 Subject: [PATCH 09/24] Fix comment --- coderd/subdomain.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/coderd/subdomain.go b/coderd/subdomain.go index 3dc404e8d61cb..8f70526aa6025 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -26,9 +26,7 @@ type ApplicationURL struct { Agent string Username string Path string - - // Domain is used to output the url to reach the app. - Domain string + Domain string } func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { From 49084e21620dcfe1d69efb82df45e1a240da838d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 Aug 2022 15:51:28 -0400 Subject: [PATCH 10/24] Reuse regex from validation --- coderd/httpapi/username.go | 4 ++++ coderd/subdomain.go | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/coderd/httpapi/username.go b/coderd/httpapi/username.go index f6e5fcc28dca3..470f0e2d0fc0a 100644 --- a/coderd/httpapi/username.go +++ b/coderd/httpapi/username.go @@ -12,6 +12,10 @@ var ( usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*") ) +func UsernameValidRegexString() string { + return usernameValid.String() +} + // UsernameValid returns whether the input string is a valid username. func UsernameValid(str string) bool { if len(str) > 32 { diff --git a/coderd/subdomain.go b/coderd/subdomain.go index 8f70526aa6025..f02dbbfd26952 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -6,6 +6,8 @@ import ( "regexp" "strings" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/httpmw" "github.com/go-chi/chi/v5" @@ -70,7 +72,8 @@ func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) } var ( - nameRegex = `[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*` + // Remove the "starts with" and "ends with" regex components. + nameRegex = strings.Trim(httpapi.UsernameValidRegexString(), "^$") appURL = regexp.MustCompile(fmt.Sprintf( // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, From 4696bf9f4bad11c8b325eed48c87932518471070 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 10:57:50 -0400 Subject: [PATCH 11/24] Export valid name regex --- coderd/httpapi/username.go | 10 +++------- coderd/subdomain.go | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/coderd/httpapi/username.go b/coderd/httpapi/username.go index 470f0e2d0fc0a..f41e0d96d3205 100644 --- a/coderd/httpapi/username.go +++ b/coderd/httpapi/username.go @@ -8,14 +8,10 @@ import ( ) var ( - usernameValid = regexp.MustCompile("^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$") - usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*") + UsernameValidRegex = regexp.MustCompile("^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$") + usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*") ) -func UsernameValidRegexString() string { - return usernameValid.String() -} - // UsernameValid returns whether the input string is a valid username. func UsernameValid(str string) bool { if len(str) > 32 { @@ -24,7 +20,7 @@ func UsernameValid(str string) bool { if len(str) < 1 { return false } - return usernameValid.MatchString(str) + return UsernameValidRegex.MatchString(str) } // UsernameFrom returns a best-effort username from the provided string. diff --git a/coderd/subdomain.go b/coderd/subdomain.go index f02dbbfd26952..ebd105af21f9a 100644 --- a/coderd/subdomain.go +++ b/coderd/subdomain.go @@ -73,7 +73,7 @@ func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) var ( // Remove the "starts with" and "ends with" regex components. - nameRegex = strings.Trim(httpapi.UsernameValidRegexString(), "^$") + nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") appURL = regexp.MustCompile(fmt.Sprintf( // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, From b5d1f6aa8d6ed1c3acd39d4737d718152ef2c693 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 11:00:57 -0400 Subject: [PATCH 12/24] Move to workspaceapps.go --- coderd/httpapi/host.go | 20 +++++ coderd/subdomain.go | 155 ----------------------------------- coderd/subdomain_test.go | 116 -------------------------- coderd/workspaceapps.go | 125 ++++++++++++++++++++++++++++ coderd/workspaceapps_test.go | 108 ++++++++++++++++++++++++ 5 files changed, 253 insertions(+), 271 deletions(-) create mode 100644 coderd/httpapi/host.go delete mode 100644 coderd/subdomain.go delete mode 100644 coderd/subdomain_test.go diff --git a/coderd/httpapi/host.go b/coderd/httpapi/host.go new file mode 100644 index 0000000000000..1723684af210f --- /dev/null +++ b/coderd/httpapi/host.go @@ -0,0 +1,20 @@ +package httpapi + +import "net/http" + +const ( + // XForwardedHostHeader is a header used by proxies to indicate the + // original host of the request. + XForwardedHostHeader = "X-Forwarded-Host" +) + +// RequestHost returns the name of the host from the request. It prioritizes +// 'X-Forwarded-Host' over r.Host since most requests are being proxied. +func RequestHost(r *http.Request) string { + host := r.Header.Get(XForwardedHostHeader) + if host != "" { + return host + } + + return r.Host +} diff --git a/coderd/subdomain.go b/coderd/subdomain.go deleted file mode 100644 index ebd105af21f9a..0000000000000 --- a/coderd/subdomain.go +++ /dev/null @@ -1,155 +0,0 @@ -package coderd - -import ( - "fmt" - "net/http" - "regexp" - "strings" - - "github.com/coder/coder/coderd/httpapi" - - "github.com/coder/coder/coderd/httpmw" - - "github.com/go-chi/chi/v5" - - "golang.org/x/xerrors" -) - -const ( - // XForwardedHostHeader is a header used by proxies to indicate the - // original host of the request. - XForwardedHostHeader = "X-Forwarded-Host" -) - -// ApplicationURL is a parsed application url into it's components -type ApplicationURL struct { - AppName string - WorkspaceName string - Agent string - Username string - Path string - Domain string -} - -func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - app, err := ParseSubdomainAppURL(r) - - if err != nil { - // Subdomain is not a valid application url. Pass through. - // TODO: @emyrk we should probably catch invalid subdomains. Meaning - // an invalid application should not route to the coderd. - // To do this we would need to know the list of valid access urls - // though? - next.ServeHTTP(rw, r) - return - } - - workspaceAgentKey := app.WorkspaceName - if app.Agent != "" { - workspaceAgentKey += "." + app.Agent - } - chiCtx := chi.RouteContext(ctx) - chiCtx.URLParams.Add("workspace_and_agent", workspaceAgentKey) - chiCtx.URLParams.Add("user", app.Username) - - // 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) { - workspace := httpmw.WorkspaceParam(r) - agent := httpmw.WorkspaceAgentParam(r) - - api.proxyWorkspaceApplication(proxyApplication{ - Workspace: workspace, - Agent: agent, - AppName: app.AppName, - }, rw, r) - })).ServeHTTP(rw, r.WithContext(ctx)) - }) - } -} - -var ( - // Remove the "starts with" and "ends with" regex components. - nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") - appURL = regexp.MustCompile(fmt.Sprintf( - // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} - `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, - nameRegex)) -) - -// ParseSubdomainAppURL parses an application from the subdomain of r's Host header. -// If the application string is not valid, returns a non-nil error. -// 1) {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT/AppName}} -// (eg. http://admin--myenv--main--8080.cdrdeploy.c8s.io) -func ParseSubdomainAppURL(r *http.Request) (ApplicationURL, error) { - host := RequestHost(r) - if host == "" { - return ApplicationURL{}, xerrors.Errorf("no host header") - } - - subdomain, domain, err := SplitSubdomain(host) - if err != nil { - return ApplicationURL{}, xerrors.Errorf("split host domain: %w", err) - } - - matches := appURL.FindAllStringSubmatch(subdomain, -1) - if len(matches) == 0 { - return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) - } - - if len(matches) > 1 { - return ApplicationURL{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) - } - matchGroup := matches[0] - - return ApplicationURL{ - AppName: matchGroup[appURL.SubexpIndex("AppName")], - WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], - Agent: matchGroup[appURL.SubexpIndex("AgentName")], - Username: matchGroup[appURL.SubexpIndex("UserName")], - Path: r.URL.Path, - Domain: domain, - }, nil -} - -// RequestHost returns the name of the host from the request. It prioritizes -// 'X-Forwarded-Host' over r.Host since most requests are being proxied. -func RequestHost(r *http.Request) string { - host := r.Header.Get(XForwardedHostHeader) - if host != "" { - return host - } - - return r.Host -} - -// SplitSubdomain splits a subdomain from a domain. E.g.: -// - "foo.bar.com" becomes "foo", "bar.com" -// - "foo.bar.baz.com" becomes "foo", "bar.baz.com" -// -// An error is returned if the string doesn't contain a period. -func SplitSubdomain(hostname string) (subdomain string, domain string, err error) { - toks := strings.SplitN(hostname, ".", 2) - if len(toks) < 2 { - return "", "", xerrors.Errorf("no domain") - } - - return toks[0], toks[1], nil -} - -// applicationCookie is a helper function to copy the auth cookie to also -// support subdomains. Until we support creating authentication cookies that can -// only do application authentication, we will just reuse the original token. -// This code should be temporary and be replaced with something that creates -// a unique session_token. -func (api *API) applicationCookie(authCookie *http.Cookie) *http.Cookie { - appCookie := *authCookie - // We only support setting this cookie on the access url subdomains. - // This is to ensure we don't accidentally leak the auth cookie to subdomains - // on another hostname. - appCookie.Domain = "." + api.AccessURL.Hostname() - return &appCookie -} diff --git a/coderd/subdomain_test.go b/coderd/subdomain_test.go deleted file mode 100644 index ac835f292b8df..0000000000000 --- a/coderd/subdomain_test.go +++ /dev/null @@ -1,116 +0,0 @@ -package coderd_test - -import ( - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/coder/coder/coderd" -) - -func TestParseSubdomainAppURL(t *testing.T) { - t.Parallel() - testCases := []struct { - Name string - URL string - Expected coderd.ApplicationURL - ExpectedError string - }{ - { - Name: "Empty", - URL: "https://example.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Workspace.Agent+App", - URL: "https://workspace.agent--app.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Workspace+App", - URL: "https://workspace--app.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - // Correct - { - Name: "User+Workspace+App", - URL: "https://user--workspace--app.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app", - WorkspaceName: "workspace", - Agent: "", - Username: "user", - Path: "", - Domain: "coder.com", - }, - }, - { - Name: "User+Workspace+Port", - URL: "https://user--workspace--8080.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "8080", - WorkspaceName: "workspace", - Agent: "", - Username: "user", - Path: "", - Domain: "coder.com", - }, - }, - { - Name: "User+Workspace.Agent+App", - URL: "https://user--workspace--agent--app.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app", - WorkspaceName: "workspace", - Agent: "agent", - Username: "user", - Path: "", - Domain: "coder.com", - }, - }, - { - Name: "User+Workspace.Agent+Port", - URL: "https://user--workspace--agent--8080.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "8080", - WorkspaceName: "workspace", - Agent: "agent", - Username: "user", - Path: "", - Domain: "coder.com", - }, - }, - { - Name: "HyphenatedNames", - URL: "https://admin-user--workspace-thing--agent-thing--app-name.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app-name", - WorkspaceName: "workspace-thing", - Agent: "agent-thing", - Username: "admin-user", - Path: "", - Domain: "coder.com", - }, - }, - } - - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() - r := httptest.NewRequest("GET", c.URL, nil) - - app, err := coderd.ParseSubdomainAppURL(r) - if c.ExpectedError == "" { - require.NoError(t, err) - require.Equal(t, c.Expected, app, "expected app") - } else { - require.ErrorContains(t, err, c.ExpectedError, "expected error") - } - }) - } -} diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 94f96b06769cb..199c96a914e96 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -7,9 +7,12 @@ import ( "net/http" "net/http/httputil" "net/url" + "regexp" "strconv" "strings" + "golang.org/x/xerrors" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -38,6 +41,46 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) }, rw, r) } +func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + app, err := ParseSubdomainAppURL(r) + + if err != nil { + // Subdomain is not a valid application url. Pass through. + // TODO: @emyrk we should probably catch invalid subdomains. Meaning + // an invalid application should not route to the coderd. + // To do this we would need to know the list of valid access urls + // though? + next.ServeHTTP(rw, r) + return + } + + workspaceAgentKey := app.WorkspaceName + if app.Agent != "" { + workspaceAgentKey += "." + app.Agent + } + chiCtx := chi.RouteContext(ctx) + chiCtx.URLParams.Add("workspace_and_agent", workspaceAgentKey) + chiCtx.URLParams.Add("user", app.Username) + + // 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) { + workspace := httpmw.WorkspaceParam(r) + agent := httpmw.WorkspaceAgentParam(r) + + api.proxyWorkspaceApplication(proxyApplication{ + Workspace: workspace, + Agent: agent, + AppName: app.AppName, + }, rw, r) + })).ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} + // proxyApplication are the required fields to proxy a workspace application. type proxyApplication struct { Workspace database.Workspace @@ -150,3 +193,85 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res proxy.ServeHTTP(rw, r) } + +var ( + // Remove the "starts with" and "ends with" regex components. + nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") + appURL = regexp.MustCompile(fmt.Sprintf( + // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} + `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, + nameRegex)) +) + +// ApplicationURL is a parsed application url into it's components +type ApplicationURL struct { + AppName string + WorkspaceName string + Agent string + Username string + Path string + Domain string +} + +// ParseSubdomainAppURL parses an application from the subdomain of r's Host header. +// If the application string is not valid, returns a non-nil error. +// 1) {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT/AppName}} +// (eg. http://admin--myenv--main--8080.cdrdeploy.c8s.io) +func ParseSubdomainAppURL(r *http.Request) (ApplicationURL, error) { + host := httpapi.RequestHost(r) + if host == "" { + return ApplicationURL{}, xerrors.Errorf("no host header") + } + + subdomain, domain, err := SplitSubdomain(host) + if err != nil { + return ApplicationURL{}, xerrors.Errorf("split host domain: %w", err) + } + + matches := appURL.FindAllStringSubmatch(subdomain, -1) + if len(matches) == 0 { + return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) + } + + if len(matches) > 1 { + return ApplicationURL{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) + } + matchGroup := matches[0] + + return ApplicationURL{ + AppName: matchGroup[appURL.SubexpIndex("AppName")], + WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], + Agent: matchGroup[appURL.SubexpIndex("AgentName")], + Username: matchGroup[appURL.SubexpIndex("UserName")], + Path: r.URL.Path, + Domain: domain, + }, nil +} + +// SplitSubdomain splits a subdomain from a domain. E.g.: +// - "foo.bar.com" becomes "foo", "bar.com" +// - "foo.bar.baz.com" becomes "foo", "bar.baz.com" +// +// An error is returned if the string doesn't contain a period. +func SplitSubdomain(hostname string) (subdomain string, domain string, err error) { + toks := strings.SplitN(hostname, ".", 2) + if len(toks) < 2 { + return "", "", xerrors.Errorf("no domain") + } + + return toks[0], toks[1], nil +} + +// applicationCookie is a helper function to copy the auth cookie to also +// support subdomains. Until we support creating authentication cookies that can +// only do application authentication, we will just reuse the original token. +// This code should be temporary and be replaced with something that creates +// a unique session_token. +func (api *API) applicationCookie(authCookie *http.Cookie) *http.Cookie { + appCookie := *authCookie + // We only support setting this cookie on the access url subdomains. + // This is to ensure we don't accidentally leak the auth cookie to subdomains + // on another hostname. + appCookie.Domain = "." + api.AccessURL.Hostname() + return &appCookie +} diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index d17a5e95ccbbb..3622b128880ae 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -3,9 +3,11 @@ package coderd_test import ( "context" "fmt" + "github.com/coder/coder/coderd" "io" "net" "net/http" + "net/http/httptest" "testing" "time" @@ -165,3 +167,109 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) }) } + +func TestParseSubdomainAppURL(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + URL string + Expected coderd.ApplicationURL + ExpectedError string + }{ + { + Name: "Empty", + URL: "https://example.com", + Expected: coderd.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Workspace.Agent+App", + URL: "https://workspace.agent--app.coder.com", + Expected: coderd.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Workspace+App", + URL: "https://workspace--app.coder.com", + Expected: coderd.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + // Correct + { + Name: "User+Workspace+App", + URL: "https://user--workspace--app.coder.com", + Expected: coderd.ApplicationURL{ + AppName: "app", + WorkspaceName: "workspace", + Agent: "", + Username: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace+Port", + URL: "https://user--workspace--8080.coder.com", + Expected: coderd.ApplicationURL{ + AppName: "8080", + WorkspaceName: "workspace", + Agent: "", + Username: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace.Agent+App", + URL: "https://user--workspace--agent--app.coder.com", + Expected: coderd.ApplicationURL{ + AppName: "app", + WorkspaceName: "workspace", + Agent: "agent", + Username: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "User+Workspace.Agent+Port", + URL: "https://user--workspace--agent--8080.coder.com", + Expected: coderd.ApplicationURL{ + AppName: "8080", + WorkspaceName: "workspace", + Agent: "agent", + Username: "user", + Path: "", + Domain: "coder.com", + }, + }, + { + Name: "HyphenatedNames", + URL: "https://admin-user--workspace-thing--agent-thing--app-name.coder.com", + Expected: coderd.ApplicationURL{ + AppName: "app-name", + WorkspaceName: "workspace-thing", + Agent: "agent-thing", + Username: "admin-user", + Path: "", + Domain: "coder.com", + }, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + r := httptest.NewRequest("GET", c.URL, nil) + + app, err := coderd.ParseSubdomainAppURL(r) + if c.ExpectedError == "" { + require.NoError(t, err) + require.Equal(t, c.Expected, app, "expected app") + } else { + require.ErrorContains(t, err, c.ExpectedError, "expected error") + } + }) + } +} From 0578588bb7c4e9b8cb4bb627e097296735132bba Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 11:05:02 -0400 Subject: [PATCH 13/24] Change app url name order --- coderd/workspaceapps.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 199c96a914e96..abba0911e062f 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -198,8 +198,8 @@ var ( // Remove the "starts with" and "ends with" regex components. nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") appURL = regexp.MustCompile(fmt.Sprintf( - // {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT}} - `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, + // AppName--WorkspaceName--AgentName--UserName + `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, nameRegex)) ) From 77d3452ae5aa7ce6016bd8ab3a10212237fa4050 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 11:14:24 -0400 Subject: [PATCH 14/24] Import order --- coderd/workspaceapps.go | 2 +- coderd/workspaceapps_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index abba0911e062f..8d596396875b6 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" + "github.com/go-chi/chi/v5" "golang.org/x/xerrors" "github.com/coder/coder/coderd/database" @@ -20,7 +21,6 @@ import ( "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/codersdk" "github.com/coder/coder/site" - "github.com/go-chi/chi/v5" ) // workspaceAppsProxyPath proxies requests to a workspace application diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 3622b128880ae..35c217dd9edf7 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -3,7 +3,6 @@ package coderd_test import ( "context" "fmt" - "github.com/coder/coder/coderd" "io" "net" "net/http" @@ -17,6 +16,7 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" + "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" From 54f2bdd4cc4cc92e250b49890c02bfe1ba1ae405 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 11:33:44 -0400 Subject: [PATCH 15/24] Deleted duplicate code --- coderd/httpapi/host.go | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 coderd/httpapi/host.go diff --git a/coderd/httpapi/host.go b/coderd/httpapi/host.go deleted file mode 100644 index 1723684af210f..0000000000000 --- a/coderd/httpapi/host.go +++ /dev/null @@ -1,20 +0,0 @@ -package httpapi - -import "net/http" - -const ( - // XForwardedHostHeader is a header used by proxies to indicate the - // original host of the request. - XForwardedHostHeader = "X-Forwarded-Host" -) - -// RequestHost returns the name of the host from the request. It prioritizes -// 'X-Forwarded-Host' over r.Host since most requests are being proxied. -func RequestHost(r *http.Request) string { - host := r.Header.Get(XForwardedHostHeader) - if host != "" { - return host - } - - return r.Host -} From f1d7670af634df34b974111a27ea751580997b4a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Aug 2022 11:48:46 -0400 Subject: [PATCH 16/24] Rename subdomain handler --- coderd/coderd.go | 4 ++-- coderd/workspaceapps.go | 2 +- coderd/workspaceapps_test.go | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index d014b0f512344..cf641dfa4ae47 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -130,9 +130,9 @@ func New(options *Options) *API { httpmw.Recover(api.Logger), httpmw.Logger(api.Logger), httpmw.Prometheus(options.PrometheusRegistry), - // handleSubdomain checks if the first subdomain is a valid app url. + // handleSubdomainApplications checks if the first subdomain is a valid app url. // If it is, it will serve that application. - api.handleSubdomain( + api.handleSubdomainApplications( // Middleware to impose on the served application. httpmw.RateLimitPerMinute(options.APIRateLimit), // This should extract the application specific API key when we diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 8d596396875b6..0ce806afb817c 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -41,7 +41,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) }, rw, r) } -func (api *API) handleSubdomain(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { +func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 35c217dd9edf7..beded446feda0 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -197,7 +197,7 @@ func TestParseSubdomainAppURL(t *testing.T) { // Correct { Name: "User+Workspace+App", - URL: "https://user--workspace--app.coder.com", + URL: "https://app--workspace--user.coder.com", Expected: coderd.ApplicationURL{ AppName: "app", WorkspaceName: "workspace", @@ -209,7 +209,7 @@ func TestParseSubdomainAppURL(t *testing.T) { }, { Name: "User+Workspace+Port", - URL: "https://user--workspace--8080.coder.com", + URL: "https://8080--workspace--user.coder.com", Expected: coderd.ApplicationURL{ AppName: "8080", WorkspaceName: "workspace", @@ -221,7 +221,7 @@ func TestParseSubdomainAppURL(t *testing.T) { }, { Name: "User+Workspace.Agent+App", - URL: "https://user--workspace--agent--app.coder.com", + URL: "https://app--workspace--agent--user.coder.com", Expected: coderd.ApplicationURL{ AppName: "app", WorkspaceName: "workspace", @@ -233,7 +233,7 @@ func TestParseSubdomainAppURL(t *testing.T) { }, { Name: "User+Workspace.Agent+Port", - URL: "https://user--workspace--agent--8080.coder.com", + URL: "https://8080--workspace--agent--user.coder.com", Expected: coderd.ApplicationURL{ AppName: "8080", WorkspaceName: "workspace", @@ -245,7 +245,7 @@ func TestParseSubdomainAppURL(t *testing.T) { }, { Name: "HyphenatedNames", - URL: "https://admin-user--workspace-thing--agent-thing--app-name.coder.com", + URL: "https://app-name--workspace-thing--agent-thing--admin-user.coder.com", Expected: coderd.ApplicationURL{ AppName: "app-name", WorkspaceName: "workspace-thing", From 56c1d0079a7fbf6ce46b544937d7813ee4816f58 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 Sep 2022 16:42:52 +0000 Subject: [PATCH 17/24] Change the devurl syntax to app--agent--workspace--user --- coderd/workspaceapps.go | 132 +++++++++++++++++++---------------- coderd/workspaceapps_test.go | 86 +++++++++++------------ 2 files changed, 112 insertions(+), 106 deletions(-) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index d4d987333958e..b6268253bc2b9 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -1,8 +1,6 @@ package coderd import ( - "database/sql" - "errors" "fmt" "net/http" "net/http/httputil" @@ -45,10 +43,19 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - app, err := ParseSubdomainAppURL(r) + host := httpapi.RequestHost(r) + if host == "" { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: "Could not determine request Host.", + }) + return + } + + app, err := ParseSubdomainAppURL(host) if err != nil { - // Subdomain is not a valid application url. Pass through. + // Subdomain is not a valid application url. Pass through to the + // rest of the app. // TODO: @emyrk we should probably catch invalid subdomains. Meaning // an invalid application should not route to the coderd. // To do this we would need to know the list of valid access urls @@ -57,10 +64,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - workspaceAgentKey := app.WorkspaceName - if app.Agent != "" { - workspaceAgentKey += "." + app.Agent - } + workspaceAgentKey := fmt.Sprintf("%s.%s", app.WorkspaceName, app.AgentName) chiCtx := chi.RouteContext(ctx) chiCtx.URLParams.Add("workspace_and_agent", workspaceAgentKey) chiCtx.URLParams.Add("user", app.Username) @@ -75,6 +79,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht Workspace: workspace, Agent: agent, AppName: app.AppName, + Port: app.Port, }, rw, r) })).ServeHTTP(rw, r.WithContext(ctx)) }) @@ -86,7 +91,9 @@ type proxyApplication struct { Workspace database.Workspace Agent database.WorkspaceAgent + // Either AppName or Port must be set, but not both. AppName string + Port uint16 } func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { @@ -95,17 +102,26 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } - var internalURL string - - // Fetch the app from the database. If the app does not exist, check if - // the app is a port number - num, numError := strconv.Atoi(proxyApp.AppName) - app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ - AgentID: proxyApp.Agent.ID, - Name: proxyApp.AppName, - }) - switch { - case err == nil: + // If the app does not exist, but the app name is a port number, then + // route to the port as an "anonymous app". We only support HTTP for + // port-based URLs. + internalURL := fmt.Sprintf("http://127.0.0.1:%d", proxyApp.Port) + + // If the app name was used instead, fetch the app from the database so we + // can get the internal URL. + if proxyApp.AppName != "" { + app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ + AgentID: proxyApp.Agent.ID, + Name: proxyApp.AppName, + }) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace application.", + Detail: err.Error(), + }) + return + } + if !app.Url.Valid { httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ Message: fmt.Sprintf("Application %s does not have a url.", app.Name), @@ -113,20 +129,6 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } internalURL = app.Url.String - case err != nil && errors.Is(err, sql.ErrNoRows) && numError == nil && num <= 65535: - // If the app does not exist, but the app name is a port number, then - // route to the port as an anonymous app. - - // Anonymous apps will default to `http`. If the user wants to configure - // particular app settings, they will have to name it. - internalURL = "http://localhost:" + proxyApp.AppName - case err != nil: - // All other db errors, return an error. - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace application.", - Detail: err.Error(), - }) - return } appURL, err := url.Parse(internalURL) @@ -198,65 +200,71 @@ var ( // Remove the "starts with" and "ends with" regex components. nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") appURL = regexp.MustCompile(fmt.Sprintf( - // AppName--WorkspaceName--AgentName--UserName - `^(?P%[1]s)--(?P%[1]s)(--(?P%[1]s))?--(?P%[1]s)$`, + // {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} + `^(?P%[1]s)--(?P%[1]s)--(?P%[1]s)--(?P%[1]s)$`, nameRegex)) ) -// ApplicationURL is a parsed application url into it's components +// ApplicationURL is a parsed application URL hostname. type ApplicationURL struct { + // Only one of AppName or Port will be set. AppName string + Port uint16 + AgentName string WorkspaceName string - Agent string Username string - Path string - Domain string + // BaseHostname is the rest of the hostname minus the application URL part + // and the first dot. + BaseHostname string } -// ParseSubdomainAppURL parses an application from the subdomain of r's Host header. -// If the application string is not valid, returns a non-nil error. -// 1. {USERNAME}--{WORKSPACE_NAME}}--{{AGENT_NAME}}--{{PORT/AppName}} -// (eg. http://admin--myenv--main--8080.cdrdeploy.c8s.io) -func ParseSubdomainAppURL(r *http.Request) (ApplicationURL, error) { - host := httpapi.RequestHost(r) - if host == "" { - return ApplicationURL{}, xerrors.Errorf("no host header") - } - - subdomain, domain, err := SplitSubdomain(host) +// ParseSubdomainAppURL parses an application from the subdomain of r's Host +// header. If the subdomain is not a valid application URL hostname, returns a +// non-nil error. +// +// Subdomains should be in the form: +// +// {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} +// (eg. http://8080--main--dev--dean.hi.c8s.io) +func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { + subdomain, rest, err := SplitSubdomain(hostname) if err != nil { - return ApplicationURL{}, xerrors.Errorf("split host domain: %w", err) + return ApplicationURL{}, xerrors.Errorf("split host domain %q: %w", hostname, err) } matches := appURL.FindAllStringSubmatch(subdomain, -1) if len(matches) == 0 { return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) } + matchGroup := matches[0] - if len(matches) > 1 { - return ApplicationURL{}, xerrors.Errorf("multiple matches (%d) for application url: %q", len(matches), subdomain) + appName := matchGroup[appURL.SubexpIndex("AppName")] + port, err := strconv.ParseUint(appName, 10, 16) + if err != nil || port == 0 { + port = 0 + } else { + appName = "" } - matchGroup := matches[0] return ApplicationURL{ - AppName: matchGroup[appURL.SubexpIndex("AppName")], + AppName: appName, + Port: uint16(port), + AgentName: matchGroup[appURL.SubexpIndex("AgentName")], WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], - Agent: matchGroup[appURL.SubexpIndex("AgentName")], - Username: matchGroup[appURL.SubexpIndex("UserName")], - Path: r.URL.Path, - Domain: domain, + Username: matchGroup[appURL.SubexpIndex("Username")], + BaseHostname: rest, }, nil } -// SplitSubdomain splits a subdomain from a domain. E.g.: +// SplitSubdomain splits a subdomain from the rest of the hostname. E.g.: // - "foo.bar.com" becomes "foo", "bar.com" // - "foo.bar.baz.com" becomes "foo", "bar.baz.com" // // An error is returned if the string doesn't contain a period. -func SplitSubdomain(hostname string) (subdomain string, domain string, err error) { +func SplitSubdomain(hostname string) (subdomain string, rest string, err error) { toks := strings.SplitN(hostname, ".", 2) if len(toks) < 2 { - return "", "", xerrors.Errorf("no domain") + return "", "", xerrors.New("no subdomain") } return toks[0], toks[1], nil diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 9871b641a92ac..2809ff9f5fc93 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -6,7 +6,6 @@ import ( "io" "net" "net/http" - "net/http/httptest" "testing" "time" @@ -175,87 +174,87 @@ func TestParseSubdomainAppURL(t *testing.T) { t.Parallel() testCases := []struct { Name string - URL string + Host string Expected coderd.ApplicationURL ExpectedError string }{ { - Name: "Empty", - URL: "https://example.com", + Name: "Invalid_Empty", + Host: "example.com", Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, { - Name: "Workspace.Agent+App", - URL: "https://workspace.agent--app.coder.com", + Name: "Invalid_Workspace.Agent--App", + Host: "workspace.agent--app.coder.com", Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, { - Name: "Workspace+App", - URL: "https://workspace--app.coder.com", + Name: "Invalid_Workspace--App", + Host: "workspace--app.coder.com", + Expected: coderd.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_App--Workspace--User", + Host: "app--workspace--user.coder.com", + Expected: coderd.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_TooManyComponents", + Host: "1--2--3--4--5.coder.com", Expected: coderd.ApplicationURL{}, ExpectedError: "invalid application url format", }, // Correct { - Name: "User+Workspace+App", - URL: "https://app--workspace--user.coder.com", + Name: "AppName--Agent--Workspace--User", + Host: "app--agent--workspace--user.coder.com", Expected: coderd.ApplicationURL{ AppName: "app", + Port: 0, + AgentName: "agent", WorkspaceName: "workspace", - Agent: "", Username: "user", - Path: "", - Domain: "coder.com", + BaseHostname: "coder.com", }, }, { - Name: "User+Workspace+Port", - URL: "https://8080--workspace--user.coder.com", + Name: "Port--Agent--Workspace--User", + Host: "8080--agent--workspace--user.coder.com", Expected: coderd.ApplicationURL{ - AppName: "8080", + AppName: "", + Port: 8080, + AgentName: "agent", WorkspaceName: "workspace", - Agent: "", Username: "user", - Path: "", - Domain: "coder.com", + BaseHostname: "coder.com", }, }, { - Name: "User+Workspace.Agent+App", - URL: "https://app--workspace--agent--user.coder.com", + Name: "DeepSubdomain", + Host: "app--agent--workspace--user.dev.dean-was-here.coder.com", Expected: coderd.ApplicationURL{ AppName: "app", + Port: 0, + AgentName: "agent", WorkspaceName: "workspace", - Agent: "agent", - Username: "user", - Path: "", - Domain: "coder.com", - }, - }, - { - Name: "User+Workspace.Agent+Port", - URL: "https://8080--workspace--agent--user.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "8080", - WorkspaceName: "workspace", - Agent: "agent", Username: "user", - Path: "", - Domain: "coder.com", + BaseHostname: "dev.dean-was-here.coder.com", }, }, { Name: "HyphenatedNames", - URL: "https://app-name--workspace-thing--agent-thing--admin-user.coder.com", + Host: "app-name--agent-name--workspace-name--user-name.coder.com", Expected: coderd.ApplicationURL{ AppName: "app-name", - WorkspaceName: "workspace-thing", - Agent: "agent-thing", - Username: "admin-user", - Path: "", - Domain: "coder.com", + Port: 0, + AgentName: "agent-name", + WorkspaceName: "workspace-name", + Username: "user-name", + BaseHostname: "coder.com", }, }, } @@ -264,9 +263,8 @@ func TestParseSubdomainAppURL(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() - r := httptest.NewRequest("GET", c.URL, nil) - app, err := coderd.ParseSubdomainAppURL(r) + app, err := coderd.ParseSubdomainAppURL(c.Host) if c.ExpectedError == "" { require.NoError(t, err) require.Equal(t, c.Expected, app, "expected app") From 25d776a2268dd8a777ae13cec4c271d8ad113e7c Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Fri, 9 Sep 2022 16:36:00 +0000 Subject: [PATCH 18/24] more devurls support stuff, everything should work now --- cli/server.go | 2 +- coderd/workspaceapps.go | 78 ++++++++++++++++--------- scripts/coder-dev.sh | 14 +++-- scripts/develop.sh | 18 +++--- site/src/components/AppLink/AppLink.tsx | 5 +- 5 files changed, 75 insertions(+), 42 deletions(-) diff --git a/cli/server.go b/cli/server.go index f1c654f9ac629..2585d9ad0cca3 100644 --- a/cli/server.go +++ b/cli/server.go @@ -687,7 +687,7 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command { cmd.Println("Waiting for WebSocket connections to close...") _ = coderAPI.Close() - cmd.Println("Done wainting for WebSocket connections") + cmd.Println("Done waiting for WebSocket connections") // Close tunnel after we no longer have in-flight connections. if tunnel { diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index b6268253bc2b9..d7ddba092dbc9 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -32,10 +32,21 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } + // Determine the real path that was hit. The * URL parameter in Chi will not + // include the leading slash if it was present, so we need to add it back. + chiPath := chi.URLParam(r, "*") + basePath := strings.TrimSuffix(r.URL.Path, chiPath) + if strings.HasSuffix(basePath, "/") { + chiPath = "/" + chiPath + } + + appName, port := AppNameOrPort(chi.URLParam(r, "workspaceapp")) api.proxyWorkspaceApplication(proxyApplication{ Workspace: workspace, Agent: agent, - AppName: chi.URLParam(r, "workspaceapp"), + AppName: appName, + Port: port, + Path: chiPath, }, rw, r) } @@ -80,6 +91,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht Agent: agent, AppName: app.AppName, Port: app.Port, + Path: r.URL.Path, }, rw, r) })).ServeHTTP(rw, r.WithContext(ctx)) }) @@ -94,6 +106,8 @@ type proxyApplication struct { // Either AppName or Port must be set, but not both. AppName string Port uint16 + // Path must either be empty or have a leading slash. + Path string } func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { @@ -134,33 +148,21 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res appURL, err := url.Parse(internalURL) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: fmt.Sprintf("App url %q must be a valid url.", internalURL), + Message: fmt.Sprintf("App URL %q is invalid.", internalURL), Detail: err.Error(), }) return } - proxy := httputil.NewSingleHostReverseProxy(appURL) - proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { - // This is a browser-facing route so JSON responses are not viable here. - // To pass friendly errors to the frontend, special meta tags are overridden - // in the index.html with the content passed here. - r = r.WithContext(site.WithAPIResponse(r.Context(), site.APIResponse{ - StatusCode: http.StatusBadGateway, - Message: err.Error(), - })) - api.siteHandler.ServeHTTP(w, r) - } - path := chi.URLParam(r, "*") - if !strings.HasSuffix(r.URL.Path, "/") && path == "" { + // Ensure path and query parameter correctness. + if proxyApp.Path == "" { // Web applications typically request paths relative to the // root URL. This allows for routing behind a proxy or subpath. // See https://github.com/coder/code-server/issues/241 for examples. - r.URL.Path += "/" - http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) + http.Redirect(rw, r, r.URL.Path+"/", http.StatusTemporaryRedirect) return } - if r.URL.RawQuery == "" && appURL.RawQuery != "" { + if proxyApp.Path == "/" && r.URL.RawQuery == "" && appURL.RawQuery != "" { // If the application defines a default set of query parameters, // we should always respect them. The reverse proxy will merge // query parameters for server-side requests, but sometimes @@ -170,7 +172,21 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) return } - r.URL.Path = path + + r.URL.Path = proxyApp.Path + appURL.RawQuery = "" + + proxy := httputil.NewSingleHostReverseProxy(appURL) + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + // This is a browser-facing route so JSON responses are not viable here. + // To pass friendly errors to the frontend, special meta tags are overridden + // in the index.html with the content passed here. + r = r.WithContext(site.WithAPIResponse(r.Context(), site.APIResponse{ + StatusCode: http.StatusBadGateway, + Message: err.Error(), + })) + api.siteHandler.ServeHTTP(w, r) + } conn, release, err := api.workspaceAgentCache.Acquire(r, proxyApp.Agent.ID) if err != nil { @@ -238,17 +254,10 @@ func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { } matchGroup := matches[0] - appName := matchGroup[appURL.SubexpIndex("AppName")] - port, err := strconv.ParseUint(appName, 10, 16) - if err != nil || port == 0 { - port = 0 - } else { - appName = "" - } - + appName, port := AppNameOrPort(matchGroup[appURL.SubexpIndex("AppName")]) return ApplicationURL{ AppName: appName, - Port: uint16(port), + Port: port, AgentName: matchGroup[appURL.SubexpIndex("AgentName")], WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], Username: matchGroup[appURL.SubexpIndex("Username")], @@ -270,6 +279,19 @@ func SplitSubdomain(hostname string) (subdomain string, rest string, err error) return toks[0], toks[1], nil } +// AppNameOrPort takes a string and returns either the input string or a port +// number. +func AppNameOrPort(val string) (string, uint16) { + port, err := strconv.ParseUint(val, 10, 16) + if err != nil || port == 0 { + port = 0 + } else { + val = "" + } + + return val, uint16(port) +} + // applicationCookie is a helper function to copy the auth cookie to also // support subdomains. Until we support creating authentication cookies that can // only do application authentication, we will just reuse the original token. diff --git a/scripts/coder-dev.sh b/scripts/coder-dev.sh index 5f9b2d02fba3e..dfba6261bab8c 100755 --- a/scripts/coder-dev.sh +++ b/scripts/coder-dev.sh @@ -10,15 +10,19 @@ source "${SCRIPT_DIR}/lib.sh" GOOS="$(go env GOOS)" GOARCH="$(go env GOARCH)" -CODER_DEV_BIN="build/coder_${GOOS}_${GOARCH}" +RELATIVE_BINARY_PATH="build/coder_${GOOS}_${GOARCH}" -cdroot -mkdir -p ./.coderv2 -CODER_DEV_DIR="$(realpath ./.coderv2)" +# To preserve the CWD when running the binary, we need to use pushd and popd to +# get absolute paths to everything. +pushd "$PROJECT_ROOT" + mkdir -p ./.coderv2 + CODER_DEV_BIN="$(realpath "$RELATIVE_BINARY_PATH")" + CODER_DEV_DIR="$(realpath ./.coderv2)" +popd if [[ ! -x "${CODER_DEV_BIN}" ]]; then echo "Run this command first:" - echo " make $CODER_DEV_BIN" + echo " make $RELATIVE_BINARY_PATH" exit 1 fi diff --git a/scripts/develop.sh b/scripts/develop.sh index 49939e42dacd8..3dc00b4b9056f 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -59,18 +59,22 @@ CODER_DEV_SHIM="${PROJECT_ROOT}/scripts/coder-dev.sh" # rather than leaving things in an inconsistent state. trap 'kill -TERM -$$' ERR cdroot - "${CODER_DEV_SHIM}" server --address 127.0.0.1:3000 --tunnel || kill -INT -$$ & + "${CODER_DEV_SHIM}" server --address 0.0.0.0:3000 --tunnel || kill -INT -$$ & echo '== Waiting for Coder to become ready' timeout 60s bash -c 'until curl -s --fail http://localhost:3000 > /dev/null 2>&1; do sleep 0.5; done' - # Try to create the initial admin user. - "${CODER_DEV_SHIM}" login http://127.0.0.1:3000 --username=admin --email=admin@coder.com --password="${password}" || - echo 'Failed to create admin user. To troubleshoot, try running this command manually.' + if [ ! -f "${PROJECT_ROOT}/.coderv2/developsh-did-first-setup" ]; then + # Try to create the initial admin user. + "${CODER_DEV_SHIM}" login http://127.0.0.1:3000 --username=admin --email=admin@coder.com --password="${password}" || + echo 'Failed to create admin user. To troubleshoot, try running this command manually.' - # Try to create a regular user. - "${CODER_DEV_SHIM}" users create --email=member@coder.com --username=member --password="${password}" || - echo 'Failed to create regular user. To troubleshoot, try running this command manually.' + # Try to create a regular user. + "${CODER_DEV_SHIM}" users create --email=member@coder.com --username=member --password="${password}" || + echo 'Failed to create regular user. To troubleshoot, try running this command manually.' + + touch "${PROJECT_ROOT}/.coderv2/developsh-did-first-setup" + fi # If we have docker available and the "docker" template doesn't already # exist, then let's try to create a template! diff --git a/site/src/components/AppLink/AppLink.tsx b/site/src/components/AppLink/AppLink.tsx index 5429e316681c2..a9d5410b15907 100644 --- a/site/src/components/AppLink/AppLink.tsx +++ b/site/src/components/AppLink/AppLink.tsx @@ -26,7 +26,10 @@ export const AppLink: FC> = ({ appIcon, }) => { const styles = useStyles() - const href = `/@${userName}/${workspaceName}.${agentName}/apps/${encodeURIComponent(appName)}` + + // The backend redirects if the trailing slash isn't included, so we add it + // here to avoid extra roundtrips. + const href = `/@${userName}/${workspaceName}.${agentName}/apps/${encodeURIComponent(appName)}/` return ( Date: Mon, 12 Sep 2022 15:57:32 +0000 Subject: [PATCH 19/24] devurls working + tests --- coderd/coderdtest/coderdtest.go | 9 +- coderd/httpmw/logger.go | 8 +- coderd/userauth.go | 6 +- coderd/users.go | 19 ++- coderd/users_test.go | 8 +- coderd/workspaceapps.go | 73 +++++++--- coderd/workspaceapps_test.go | 242 ++++++++++++++++++++++++++++++-- 7 files changed, 319 insertions(+), 46 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index f4f26dc05d642..dfc927d3c1e65 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -63,6 +63,7 @@ import ( ) type Options struct { + AccessURL *url.URL AWSCertificates awsidentity.Certificates Authorizer rbac.Authorizer AzureCertificates x509.VerifyOptions @@ -197,13 +198,19 @@ func newWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c _ = turnServer.Close() }) + accessURL := options.AccessURL + if accessURL == nil { + accessURL, err = url.Parse(srv.URL) + require.NoError(t, err) + } + // We set the handler after server creation for the access URL. coderAPI := options.APIBuilder(&coderd.Options{ AgentConnectionUpdateFrequency: 150 * time.Millisecond, // Force a long disconnection timeout to ensure // agents are not marked as disconnected during slow tests. AgentInactiveDisconnectTimeout: testutil.WaitShort, - AccessURL: serverURL, + AccessURL: accessURL, Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), CacheDir: t.TempDir(), Database: db, diff --git a/coderd/httpmw/logger.go b/coderd/httpmw/logger.go index 6f3a700bc56bf..80fecd8fdafd8 100644 --- a/coderd/httpmw/logger.go +++ b/coderd/httpmw/logger.go @@ -42,15 +42,13 @@ func Logger(log slog.Logger) func(next http.Handler) http.Handler { ) } + // We should not log at level ERROR for 5xx status codes because 5xx + // includes proxy errors etc. It also causes slogtest to fail + // instantly without an error message by default. logLevelFn := httplog.Debug if sw.Status >= 400 { logLevelFn = httplog.Warn } - if sw.Status >= 500 { - // Server errors should be treated as an ERROR - // log level. - logLevelFn = httplog.Error - } logLevelFn(r.Context(), r.Method) }) diff --git a/coderd/userauth.go b/coderd/userauth.go index da55f61f15dad..7d68526c030b1 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -162,8 +162,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) - http.SetCookie(rw, api.applicationCookie(cookie)) + api.setAuthCookie(rw, cookie) redirect := state.Redirect if redirect == "" { @@ -297,8 +296,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) - http.SetCookie(rw, api.applicationCookie(cookie)) + api.setAuthCookie(rw, cookie) redirect := state.Redirect if redirect == "" { diff --git a/coderd/users.go b/coderd/users.go index 6d153d18478aa..cf4c3aedb3a2f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -809,8 +809,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) - http.SetCookie(rw, api.applicationCookie(cookie)) + api.setAuthCookie(rw, cookie) httpapi.Write(rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ SessionToken: cookie.Value, @@ -887,9 +886,12 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { Name: codersdk.SessionTokenKey, Path: "/", } - http.SetCookie(rw, cookie) - http.SetCookie(rw, api.applicationCookie(cookie)) + + devurlCookie := api.applicationCookie(cookie) + if devurlCookie != nil { + http.SetCookie(rw, devurlCookie) + } // Delete the session token from database. apiKey := httpmw.APIKey(r) @@ -1069,6 +1071,15 @@ func (api *API) createUser(ctx context.Context, store database.Store, req create }) } +func (api *API) setAuthCookie(rw http.ResponseWriter, cookie *http.Cookie) { + http.SetCookie(rw, cookie) + + devurlCookie := api.applicationCookie(cookie) + if devurlCookie != nil { + http.SetCookie(rw, devurlCookie) + } +} + func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User { convertedUser := codersdk.User{ ID: user.ID, diff --git a/coderd/users_test.go b/coderd/users_test.go index c682862637a03..9b40e8e1f145c 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "sort" "strings" "testing" @@ -261,7 +262,12 @@ func TestPostLogout(t *testing.T) { t.Run("Logout", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, nil) + accessURL, err := url.Parse("http://somedomain.com") + require.NoError(t, err) + + client := coderdtest.New(t, &coderdtest.Options{ + AccessURL: accessURL, + }) admin := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index d7ddba092dbc9..96a055a1a58ff 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -2,6 +2,7 @@ package coderd import ( "fmt" + "net" "net/http" "net/http/httputil" "net/url" @@ -42,11 +43,12 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) appName, port := AppNameOrPort(chi.URLParam(r, "workspaceapp")) api.proxyWorkspaceApplication(proxyApplication{ - Workspace: workspace, - Agent: agent, - AppName: appName, - Port: port, - Path: chiPath, + Workspace: workspace, + Agent: agent, + AppName: appName, + Port: port, + Path: chiPath, + DashboardOnError: true, }, rw, r) } @@ -87,11 +89,12 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht agent := httpmw.WorkspaceAgentParam(r) api.proxyWorkspaceApplication(proxyApplication{ - Workspace: workspace, - Agent: agent, - AppName: app.AppName, - Port: app.Port, - Path: r.URL.Path, + Workspace: workspace, + Agent: agent, + AppName: app.AppName, + Port: app.Port, + Path: r.URL.Path, + DashboardOnError: false, }, rw, r) })).ServeHTTP(rw, r.WithContext(ctx)) }) @@ -108,6 +111,11 @@ type proxyApplication struct { Port uint16 // Path must either be empty or have a leading slash. Path string + + // DashboardOnError determines whether or not the dashboard should be + // rendered on error. This should be set for proxy path URLs but not + // hostname based URLs. + DashboardOnError bool } func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { @@ -178,14 +186,21 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res proxy := httputil.NewSingleHostReverseProxy(appURL) proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { - // This is a browser-facing route so JSON responses are not viable here. - // To pass friendly errors to the frontend, special meta tags are overridden - // in the index.html with the content passed here. - r = r.WithContext(site.WithAPIResponse(r.Context(), site.APIResponse{ - StatusCode: http.StatusBadGateway, - Message: err.Error(), - })) - api.siteHandler.ServeHTTP(w, r) + if proxyApp.DashboardOnError { + // To pass friendly errors to the frontend, special meta tags are + // overridden in the index.html with the content passed here. + r = r.WithContext(site.WithAPIResponse(r.Context(), site.APIResponse{ + StatusCode: http.StatusBadGateway, + Message: err.Error(), + })) + api.siteHandler.ServeHTTP(w, r) + return + } + + httpapi.Write(w, http.StatusBadGateway, codersdk.Response{ + Message: "Failed to proxy request to application.", + Detail: err.Error(), + }) } conn, release, err := api.workspaceAgentCache.Acquire(r, proxyApp.Agent.ID) @@ -234,6 +249,16 @@ type ApplicationURL struct { BaseHostname string } +// String returns the application URL hostname without scheme. +func (a ApplicationURL) String() string { + appNameOrPort := a.AppName + if a.Port != 0 { + appNameOrPort = strconv.Itoa(int(a.Port)) + } + + return fmt.Sprintf("%s--%s--%s--%s.%s", appNameOrPort, a.AgentName, a.WorkspaceName, a.Username, a.BaseHostname) +} + // ParseSubdomainAppURL parses an application from the subdomain of r's Host // header. If the subdomain is not a valid application URL hostname, returns a // non-nil error. @@ -297,11 +322,17 @@ func AppNameOrPort(val string) (string, uint16) { // only do application authentication, we will just reuse the original token. // This code should be temporary and be replaced with something that creates // a unique session_token. +// +// Returns nil if the access URL isn't a hostname. func (api *API) applicationCookie(authCookie *http.Cookie) *http.Cookie { + if net.ParseIP(api.AccessURL.Hostname()) != nil { + return nil + } + appCookie := *authCookie - // We only support setting this cookie on the access url subdomains. - // This is to ensure we don't accidentally leak the auth cookie to subdomains - // on another hostname. + // We only support setting this cookie on the access URL subdomains. This is + // to ensure we don't accidentally leak the auth cookie to subdomains on + // another hostname. appCookie.Domain = "." + api.AccessURL.Hostname() return &appCookie } diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 2809ff9f5fc93..a43046b8c505f 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -6,6 +6,7 @@ import ( "io" "net" "net/http" + "net/url" "testing" "time" @@ -17,14 +18,25 @@ import ( "github.com/coder/coder/agent" "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" "github.com/coder/coder/testutil" ) -func TestWorkspaceAppsProxyPath(t *testing.T) { - t.Parallel() +const ( + proxyTestAgentName = "agent-name" + proxyTestAppName = "example" + proxyTestAppQuery = "query=true" + proxyTestAppBody = "hello world" + proxyTestFakeAppName = "fake" +) + +// setupProxyTest creates a workspace with an agent and some apps. It returns a +// codersdk client, the workspace, and the port number the test listener is +// running on. +func setupProxyTest(t *testing.T) (*codersdk.Client, uuid.UUID, codersdk.Workspace, uint16) { // #nosec ln, err := net.Listen("tcp", ":0") require.NoError(t, err) @@ -34,6 +46,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { _, err := r.Cookie(codersdk.SessionTokenKey) assert.ErrorIs(t, err, http.ErrNoCookie) w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(proxyTestAppBody)) }), } t.Cleanup(func() { @@ -41,7 +54,8 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { _ = ln.Close() }) go server.Serve(ln) - tcpAddr, _ := ln.Addr().(*net.TCPAddr) + tcpAddr, ok := ln.Addr().(*net.TCPAddr) + require.True(t, ok) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, @@ -58,15 +72,16 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { Name: "example", Type: "aws_instance", Agents: []*proto.Agent{{ - Id: uuid.NewString(), + Id: uuid.NewString(), + Name: proxyTestAgentName, Auth: &proto.Agent_Token{ Token: authToken, }, Apps: []*proto.App{{ - Name: "example", - Url: fmt.Sprintf("http://127.0.0.1:%d?query=true", tcpAddr.Port), + Name: proxyTestAppName, + Url: fmt.Sprintf("http://127.0.0.1:%d?%s", tcpAddr.Port, proxyTestAppQuery), }, { - Name: "fake", + Name: proxyTestFakeAppName, Url: "http://127.0.0.2", }}, }}, @@ -92,9 +107,26 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { _ = agentCloser.Close() }) coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID) + + // Configure the HTTP client to not follow redirects and to route all + // requests regardless of hostname to the coderd test server. client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse } + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + require.True(t, ok) + transport := defaultTransport.Clone() + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, client.URL.Host) + } + client.HTTPClient.Transport = transport + + return client, user.OrganizationID, workspace, uint16(tcpAddr.Port) +} + +func TestWorkspaceAppsProxyPath(t *testing.T) { + t.Parallel() + client, orgID, workspace, port := setupProxyTest(t) t.Run("RedirectsWithoutAuth", func(t *testing.T) { t.Parallel() @@ -115,6 +147,22 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) }) + t.Run("NoAccessShould401", func(t *testing.T) { + t.Parallel() + + userClient := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + userClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + userClient.HTTPClient.Transport = client.HTTPClient.Transport + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := userClient.Request(ctx, http.MethodGet, "/@me/"+workspace.Name+"/apps/example", nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusNotFound, resp.StatusCode) + }) + t.Run("RedirectsWithSlash", func(t *testing.T) { t.Parallel() @@ -139,7 +187,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) loc, err := resp.Location() require.NoError(t, err) - require.Equal(t, "query=true", loc.RawQuery) + require.Equal(t, proxyTestAppQuery, loc.RawQuery) }) t.Run("Proxies", func(t *testing.T) { @@ -148,12 +196,28 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := client.Request(ctx, http.MethodGet, "/@me/"+workspace.Name+"/apps/example/?query=true", nil) + resp, err := client.Request(ctx, http.MethodGet, "/@me/"+workspace.Name+"/apps/example/?"+proxyTestAppQuery, nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, proxyTestAppBody, string(body)) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("ProxiesPort", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + path := fmt.Sprintf("/@me/%s/apps/%d/?%s", workspace.Name, port, proxyTestAppQuery) + resp, err := client.Request(ctx, http.MethodGet, path, nil) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) require.NoError(t, err) - require.Equal(t, "", string(body)) + require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) }) @@ -166,8 +230,166 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { resp, err := client.Request(ctx, http.MethodGet, "/@me/"+workspace.Name+"/apps/fake/", nil) require.NoError(t, err) defer resp.Body.Close() + // this is 200 OK because it returns a dashboard page + require.Equal(t, http.StatusOK, resp.StatusCode) + }) +} + +func TestWorkspaceAppsProxySubdomain(t *testing.T) { + t.Parallel() + client, orgID, workspace, port := setupProxyTest(t) + + // proxyURL generates a URL for the proxy subdomain. The default path is a + // slash. + proxyURL := func(t *testing.T, appNameOrPort interface{}, pathAndQuery ...string) string { + t.Helper() + + var ( + appName string + port uint16 + ) + if val, ok := appNameOrPort.(string); ok { + appName = val + } else { + port, ok = appNameOrPort.(uint16) + require.True(t, ok) + } + + me, err := client.User(context.Background(), codersdk.Me) + require.NoError(t, err, "get current user details") + + hostname := coderd.ApplicationURL{ + AppName: appName, + Port: port, + AgentName: proxyTestAgentName, + WorkspaceName: workspace.Name, + Username: me.Username, + BaseHostname: "test.coder.com", + }.String() + + actualPath := "/" + query := "" + if len(pathAndQuery) > 0 { + actualPath = pathAndQuery[0] + } + if len(pathAndQuery) > 1 { + query = pathAndQuery[1] + } + + return (&url.URL{ + Scheme: "http", + Host: hostname, + Path: actualPath, + RawQuery: query, + }).String() + } + + t.Run("NoAuthShould401", func(t *testing.T) { + t.Parallel() + unauthedClient := codersdk.New(client.URL) + unauthedClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + unauthedClient.HTTPClient.Transport = client.HTTPClient.Transport + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := unauthedClient.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppName), nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + }) + + t.Run("NoAccessShould401", func(t *testing.T) { + t.Parallel() + + userClient := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + userClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + userClient.HTTPClient.Transport = client.HTTPClient.Transport + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := userClient.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppName), nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusNotFound, resp.StatusCode) + }) + + t.Run("RedirectsWithSlash", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + slashlessURL := proxyURL(t, proxyTestAppName, "") + resp, err := client.Request(ctx, http.MethodGet, slashlessURL, nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + loc, err := resp.Location() + require.NoError(t, err) + require.Equal(t, slashlessURL+"/?"+proxyTestAppQuery, loc.String()) + }) + + t.Run("RedirectsWithQuery", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + querylessURL := proxyURL(t, proxyTestAppName, "/", "") + resp, err := client.Request(ctx, http.MethodGet, querylessURL, nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + loc, err := resp.Location() + require.NoError(t, err) + require.Equal(t, proxyTestAppQuery, loc.RawQuery) + }) + + t.Run("Proxies", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppName, "/", proxyTestAppQuery), nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, proxyTestAppBody, string(body)) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("ProxiesPort", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, port, "/", proxyTestAppQuery), nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) }) + + t.Run("ProxyError", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, proxyTestFakeAppName, "/", ""), nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusBadGateway, resp.StatusCode) + }) } func TestParseSubdomainAppURL(t *testing.T) { From 1f8a1f08f08640c0702916b001d6c50114cc0e78 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 12 Sep 2022 16:46:01 +0000 Subject: [PATCH 20/24] Move stuff to httpapi --- coderd/httpapi/url.go | 100 ++++++++++++++ coderd/httpapi/url_test.go | 253 +++++++++++++++++++++++++++++++++++ coderd/workspaceapps.go | 97 +------------- coderd/workspaceapps_test.go | 109 +-------------- 4 files changed, 357 insertions(+), 202 deletions(-) create mode 100644 coderd/httpapi/url.go create mode 100644 coderd/httpapi/url_test.go diff --git a/coderd/httpapi/url.go b/coderd/httpapi/url.go new file mode 100644 index 0000000000000..a7780f2b3b2b9 --- /dev/null +++ b/coderd/httpapi/url.go @@ -0,0 +1,100 @@ +package httpapi + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "golang.org/x/xerrors" +) + +var ( + // Remove the "starts with" and "ends with" regex components. + nameRegex = strings.Trim(UsernameValidRegex.String(), "^$") + appURL = regexp.MustCompile(fmt.Sprintf( + // {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} + `^(?P%[1]s)--(?P%[1]s)--(?P%[1]s)--(?P%[1]s)$`, + nameRegex)) +) + +// SplitSubdomain splits a subdomain from the rest of the hostname. E.g.: +// - "foo.bar.com" becomes "foo", "bar.com" +// - "foo.bar.baz.com" becomes "foo", "bar.baz.com" +// +// An error is returned if the string doesn't contain a period. +func SplitSubdomain(hostname string) (subdomain string, rest string, err error) { + toks := strings.SplitN(hostname, ".", 2) + if len(toks) < 2 { + return "", "", xerrors.New("no subdomain") + } + + return toks[0], toks[1], nil +} + +// ApplicationURL is a parsed application URL hostname. +type ApplicationURL struct { + // Only one of AppName or Port will be set. + AppName string + Port uint16 + AgentName string + WorkspaceName string + Username string + // BaseHostname is the rest of the hostname minus the application URL part + // and the first dot. + BaseHostname string +} + +// String returns the application URL hostname without scheme. +func (a ApplicationURL) String() string { + appNameOrPort := a.AppName + if a.Port != 0 { + appNameOrPort = strconv.Itoa(int(a.Port)) + } + + return fmt.Sprintf("%s--%s--%s--%s.%s", appNameOrPort, a.AgentName, a.WorkspaceName, a.Username, a.BaseHostname) +} + +// ParseSubdomainAppURL parses an ApplicationURL from the given hostname. If +// the subdomain is not a valid application URL hostname, returns a non-nil +// error. +// +// Subdomains should be in the form: +// +// {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} +// (eg. http://8080--main--dev--dean.hi.c8s.io) +func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { + subdomain, rest, err := SplitSubdomain(hostname) + if err != nil { + return ApplicationURL{}, xerrors.Errorf("split host domain %q: %w", hostname, err) + } + + matches := appURL.FindAllStringSubmatch(subdomain, -1) + if len(matches) == 0 { + return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) + } + matchGroup := matches[0] + + appName, port := AppNameOrPort(matchGroup[appURL.SubexpIndex("AppName")]) + return ApplicationURL{ + AppName: appName, + Port: port, + AgentName: matchGroup[appURL.SubexpIndex("AgentName")], + WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], + Username: matchGroup[appURL.SubexpIndex("Username")], + BaseHostname: rest, + }, nil +} + +// AppNameOrPort takes a string and returns either the input string or a port +// number. +func AppNameOrPort(val string) (string, uint16) { + port, err := strconv.ParseUint(val, 10, 16) + if err != nil || port == 0 { + port = 0 + } else { + val = "" + } + + return val, uint16(port) +} diff --git a/coderd/httpapi/url_test.go b/coderd/httpapi/url_test.go new file mode 100644 index 0000000000000..567074633e988 --- /dev/null +++ b/coderd/httpapi/url_test.go @@ -0,0 +1,253 @@ +package httpapi_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/httpapi" +) + +func TestSplitSubdomain(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + Host string + ExpectedSubdomain string + ExpectedRest string + ExpectedErr string + }{ + { + Name: "Empty", + Host: "", + ExpectedSubdomain: "", + ExpectedRest: "", + ExpectedErr: "no subdomain", + }, + { + Name: "NoSubdomain", + Host: "com", + ExpectedSubdomain: "", + ExpectedRest: "", + ExpectedErr: "no subdomain", + }, + { + Name: "Domain", + Host: "coder.com", + ExpectedSubdomain: "coder", + ExpectedRest: "com", + ExpectedErr: "", + }, + { + Name: "Subdomain", + Host: "subdomain.coder.com", + ExpectedSubdomain: "subdomain", + ExpectedRest: "coder.com", + ExpectedErr: "", + }, + { + Name: "DoubleSubdomain", + Host: "subdomain1.subdomain2.coder.com", + ExpectedSubdomain: "subdomain1", + ExpectedRest: "subdomain2.coder.com", + ExpectedErr: "", + }, + { + Name: "WithPort", + Host: "subdomain.coder.com:8080", + ExpectedSubdomain: "subdomain", + ExpectedRest: "coder.com:8080", + ExpectedErr: "", + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + + subdomain, rest, err := httpapi.SplitSubdomain(c.Host) + if c.ExpectedErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), c.ExpectedErr) + } else { + require.NoError(t, err) + } + require.Equal(t, c.ExpectedSubdomain, subdomain) + require.Equal(t, c.ExpectedRest, rest) + }) + } +} + +func TestApplicationURLString(t *testing.T) { + t.Parallel() + + testCases := []struct { + Name string + URL httpapi.ApplicationURL + Expected string + }{ + { + Name: "Empty", + URL: httpapi.ApplicationURL{}, + Expected: "------.", + }, + { + Name: "AppName", + URL: httpapi.ApplicationURL{ + AppName: "app", + Port: 0, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "coder.com", + }, + Expected: "app--agent--workspace--user.coder.com", + }, + { + Name: "Port", + URL: httpapi.ApplicationURL{ + AppName: "", + Port: 8080, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "coder.com", + }, + Expected: "8080--agent--workspace--user.coder.com", + }, + { + Name: "Both", + URL: httpapi.ApplicationURL{ + AppName: "app", + Port: 8080, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "coder.com", + }, + // Prioritizes port over app name. + Expected: "8080--agent--workspace--user.coder.com", + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, c.Expected, c.URL.String()) + }) + } +} + +func TestParseSubdomainAppURL(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + Host string + Expected httpapi.ApplicationURL + ExpectedError string + }{ + { + Name: "Invalid_Split", + Host: "com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "no subdomain", + }, + { + Name: "Invalid_Empty", + Host: "example.com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_Workspace.Agent--App", + Host: "workspace.agent--app.coder.com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_Workspace--App", + Host: "workspace--app.coder.com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_App--Workspace--User", + Host: "app--workspace--user.coder.com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + { + Name: "Invalid_TooManyComponents", + Host: "1--2--3--4--5.coder.com", + Expected: httpapi.ApplicationURL{}, + ExpectedError: "invalid application url format", + }, + // Correct + { + Name: "AppName--Agent--Workspace--User", + Host: "app--agent--workspace--user.coder.com", + Expected: httpapi.ApplicationURL{ + AppName: "app", + Port: 0, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "coder.com", + }, + }, + { + Name: "Port--Agent--Workspace--User", + Host: "8080--agent--workspace--user.coder.com", + Expected: httpapi.ApplicationURL{ + AppName: "", + Port: 8080, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "coder.com", + }, + }, + { + Name: "DeepSubdomain", + Host: "app--agent--workspace--user.dev.dean-was-here.coder.com", + Expected: httpapi.ApplicationURL{ + AppName: "app", + Port: 0, + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + BaseHostname: "dev.dean-was-here.coder.com", + }, + }, + { + Name: "HyphenatedNames", + Host: "app-name--agent-name--workspace-name--user-name.coder.com", + Expected: httpapi.ApplicationURL{ + AppName: "app-name", + Port: 0, + AgentName: "agent-name", + WorkspaceName: "workspace-name", + Username: "user-name", + BaseHostname: "coder.com", + }, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + + app, err := httpapi.ParseSubdomainAppURL(c.Host) + if c.ExpectedError == "" { + require.NoError(t, err) + require.Equal(t, c.Expected, app, "expected app") + } else { + require.ErrorContains(t, err, c.ExpectedError, "expected error") + } + }) + } +} diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 96a055a1a58ff..44807d5fa73db 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -6,12 +6,9 @@ import ( "net/http" "net/http/httputil" "net/url" - "regexp" - "strconv" "strings" "github.com/go-chi/chi/v5" - "golang.org/x/xerrors" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" @@ -41,7 +38,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) chiPath = "/" + chiPath } - appName, port := AppNameOrPort(chi.URLParam(r, "workspaceapp")) + appName, port := httpapi.AppNameOrPort(chi.URLParam(r, "workspaceapp")) api.proxyWorkspaceApplication(proxyApplication{ Workspace: workspace, Agent: agent, @@ -65,7 +62,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - app, err := ParseSubdomainAppURL(host) + app, err := httpapi.ParseSubdomainAppURL(host) if err != nil { // Subdomain is not a valid application url. Pass through to the // rest of the app. @@ -227,96 +224,6 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res proxy.ServeHTTP(rw, r) } -var ( - // Remove the "starts with" and "ends with" regex components. - nameRegex = strings.Trim(httpapi.UsernameValidRegex.String(), "^$") - appURL = regexp.MustCompile(fmt.Sprintf( - // {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} - `^(?P%[1]s)--(?P%[1]s)--(?P%[1]s)--(?P%[1]s)$`, - nameRegex)) -) - -// ApplicationURL is a parsed application URL hostname. -type ApplicationURL struct { - // Only one of AppName or Port will be set. - AppName string - Port uint16 - AgentName string - WorkspaceName string - Username string - // BaseHostname is the rest of the hostname minus the application URL part - // and the first dot. - BaseHostname string -} - -// String returns the application URL hostname without scheme. -func (a ApplicationURL) String() string { - appNameOrPort := a.AppName - if a.Port != 0 { - appNameOrPort = strconv.Itoa(int(a.Port)) - } - - return fmt.Sprintf("%s--%s--%s--%s.%s", appNameOrPort, a.AgentName, a.WorkspaceName, a.Username, a.BaseHostname) -} - -// ParseSubdomainAppURL parses an application from the subdomain of r's Host -// header. If the subdomain is not a valid application URL hostname, returns a -// non-nil error. -// -// Subdomains should be in the form: -// -// {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} -// (eg. http://8080--main--dev--dean.hi.c8s.io) -func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { - subdomain, rest, err := SplitSubdomain(hostname) - if err != nil { - return ApplicationURL{}, xerrors.Errorf("split host domain %q: %w", hostname, err) - } - - matches := appURL.FindAllStringSubmatch(subdomain, -1) - if len(matches) == 0 { - return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) - } - matchGroup := matches[0] - - appName, port := AppNameOrPort(matchGroup[appURL.SubexpIndex("AppName")]) - return ApplicationURL{ - AppName: appName, - Port: port, - AgentName: matchGroup[appURL.SubexpIndex("AgentName")], - WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], - Username: matchGroup[appURL.SubexpIndex("Username")], - BaseHostname: rest, - }, nil -} - -// SplitSubdomain splits a subdomain from the rest of the hostname. E.g.: -// - "foo.bar.com" becomes "foo", "bar.com" -// - "foo.bar.baz.com" becomes "foo", "bar.baz.com" -// -// An error is returned if the string doesn't contain a period. -func SplitSubdomain(hostname string) (subdomain string, rest string, err error) { - toks := strings.SplitN(hostname, ".", 2) - if len(toks) < 2 { - return "", "", xerrors.New("no subdomain") - } - - return toks[0], toks[1], nil -} - -// AppNameOrPort takes a string and returns either the input string or a port -// number. -func AppNameOrPort(val string) (string, uint16) { - port, err := strconv.ParseUint(val, 10, 16) - if err != nil || port == 0 { - port = 0 - } else { - val = "" - } - - return val, uint16(port) -} - // applicationCookie is a helper function to copy the auth cookie to also // support subdomains. Until we support creating authentication cookies that can // only do application authentication, we will just reuse the original token. diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index a43046b8c505f..153cf17d0edad 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -16,8 +16,8 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" - "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" @@ -258,7 +258,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { me, err := client.User(context.Background(), codersdk.Me) require.NoError(t, err, "get current user details") - hostname := coderd.ApplicationURL{ + hostname := httpapi.ApplicationURL{ AppName: appName, Port: port, AgentName: proxyTestAgentName, @@ -391,108 +391,3 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { require.Equal(t, http.StatusBadGateway, resp.StatusCode) }) } - -func TestParseSubdomainAppURL(t *testing.T) { - t.Parallel() - testCases := []struct { - Name string - Host string - Expected coderd.ApplicationURL - ExpectedError string - }{ - { - Name: "Invalid_Empty", - Host: "example.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Invalid_Workspace.Agent--App", - Host: "workspace.agent--app.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Invalid_Workspace--App", - Host: "workspace--app.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Invalid_App--Workspace--User", - Host: "app--workspace--user.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - { - Name: "Invalid_TooManyComponents", - Host: "1--2--3--4--5.coder.com", - Expected: coderd.ApplicationURL{}, - ExpectedError: "invalid application url format", - }, - // Correct - { - Name: "AppName--Agent--Workspace--User", - Host: "app--agent--workspace--user.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app", - Port: 0, - AgentName: "agent", - WorkspaceName: "workspace", - Username: "user", - BaseHostname: "coder.com", - }, - }, - { - Name: "Port--Agent--Workspace--User", - Host: "8080--agent--workspace--user.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "", - Port: 8080, - AgentName: "agent", - WorkspaceName: "workspace", - Username: "user", - BaseHostname: "coder.com", - }, - }, - { - Name: "DeepSubdomain", - Host: "app--agent--workspace--user.dev.dean-was-here.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app", - Port: 0, - AgentName: "agent", - WorkspaceName: "workspace", - Username: "user", - BaseHostname: "dev.dean-was-here.coder.com", - }, - }, - { - Name: "HyphenatedNames", - Host: "app-name--agent-name--workspace-name--user-name.coder.com", - Expected: coderd.ApplicationURL{ - AppName: "app-name", - Port: 0, - AgentName: "agent-name", - WorkspaceName: "workspace-name", - Username: "user-name", - BaseHostname: "coder.com", - }, - }, - } - - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() - - app, err := coderd.ParseSubdomainAppURL(c.Host) - if c.ExpectedError == "" { - require.NoError(t, err) - require.Equal(t, c.Expected, app, "expected app") - } else { - require.ErrorContains(t, err, c.ExpectedError, "expected error") - } - }) - } -} From 2c7bcc17e62e64316201823fa9a1b7bf565bbaa6 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 12 Sep 2022 16:56:37 +0000 Subject: [PATCH 21/24] fixup! Move stuff to httpapi --- coderd/users.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 0c35c90c97634..b2f97b6c19744 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -941,12 +941,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { Name: codersdk.SessionTokenKey, Path: "/", } - http.SetCookie(rw, cookie) - - devurlCookie := api.applicationCookie(cookie) - if devurlCookie != nil { - http.SetCookie(rw, devurlCookie) - } + api.setAuthCookie(rw, cookie) // Delete the session token from database. apiKey := httpmw.APIKey(r) From 58653d4aa4963b848b133af47f26fc4e22a5feed Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 12 Sep 2022 19:07:47 +0000 Subject: [PATCH 22/24] kyle comments --- coderd/coderd.go | 11 +++++++- coderd/coderdtest/coderdtest.go | 13 ++++----- coderd/httpmw/apikey.go | 50 ++++++++++++++++++++++++++++++--- coderd/users_test.go | 8 +----- coderd/workspaceapps_test.go | 25 ++++++++++++----- 5 files changed, 80 insertions(+), 27 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index b3d4066e43f30..8d90c934fb846 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -165,9 +165,18 @@ func New(options *Options) *API { api.handleSubdomainApplications( // Middleware to impose on the served application. httpmw.RateLimitPerMinute(options.APIRateLimit), + httpmw.UseLoginURL(func() *url.URL { + if options.AccessURL == nil { + return nil + } + + u := *options.AccessURL + u.Path = "/login" + return &u + }()), // This should extract the application specific API key when we // implement a scoped token. - httpmw.ExtractAPIKey(options.Database, oauthConfigs, false), + httpmw.ExtractAPIKey(options.Database, oauthConfigs, true), httpmw.ExtractUserParam(api.Database), httpmw.ExtractWorkspaceAndAgentParam(api.Database), ), diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 7d7fdbb889d82..646835d8fe532 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -64,7 +64,6 @@ import ( ) type Options struct { - AccessURL *url.URL AWSCertificates awsidentity.Certificates Authorizer rbac.Authorizer AzureCertificates x509.VerifyOptions @@ -183,8 +182,12 @@ func newWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c srv.Start() t.Cleanup(srv.Close) + tcpAddr, ok := srv.Listener.Addr().(*net.TCPAddr) + require.True(t, ok) + serverURL, err := url.Parse(srv.URL) require.NoError(t, err) + serverURL.Host = fmt.Sprintf("localhost:%d", tcpAddr.Port) derpPort, err := strconv.Atoi(serverURL.Port()) require.NoError(t, err) @@ -205,19 +208,13 @@ func newWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c features.Auditor = options.Auditor } - accessURL := options.AccessURL - if accessURL == nil { - accessURL, err = url.Parse(srv.URL) - require.NoError(t, err) - } - // We set the handler after server creation for the access URL. coderAPI := options.APIBuilder(&coderd.Options{ AgentConnectionUpdateFrequency: 150 * time.Millisecond, // Force a long disconnection timeout to ensure // agents are not marked as disconnected during slow tests. AgentInactiveDisconnectTimeout: testutil.WaitShort, - AccessURL: accessURL, + AccessURL: serverURL, Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), CacheDir: t.TempDir(), Database: db, diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index ed2a1e617f567..c2b748434f8d5 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -9,6 +9,7 @@ import ( "fmt" "net" "net/http" + "net/url" "strings" "time" @@ -58,6 +59,24 @@ const ( internalErrorMessage string = "An internal error occurred. Please try again or contact the system administrator." ) +type loginURLKey struct{} + +func getLoginURL(r *http.Request) (*url.URL, bool) { + val, ok := r.Context().Value(loginURLKey{}).(*url.URL) + return val, ok +} + +// UseLoginURL sets the login URL to use for the request for handlers like +// ExtractAPIKey. +func UseLoginURL(loginURL *url.URL) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), loginURLKey{}, loginURL) + next.ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} + // ExtractAPIKey requires authentication using a valid API key. // It handles extending an API key if it comes close to expiry, // updating the last used time in the database. @@ -70,14 +89,37 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool // pages like workspace applications. write := func(code int, response codersdk.Response) { if redirectToLogin { + var ( + u = &url.URL{ + Path: "/login", + } + redirectURL = func() string { + path := r.URL.Path + if r.URL.RawQuery != "" { + path += "?" + r.URL.RawQuery + } + return path + }() + ) + if loginURL, ok := getLoginURL(r); ok { + u = loginURL + // Don't redirect to the current page, as it may be on + // a different domain and we have issues determining the + // scheme to redirect to. + redirectURL = "" + } + q := r.URL.Query() q.Add("message", response.Message) - q.Add("redirect", r.URL.Path+"?"+r.URL.RawQuery) - r.URL.RawQuery = q.Encode() - r.URL.Path = "/login" - http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) + if redirectURL != "" { + q.Add("redirect", redirectURL) + } + u.RawQuery = q.Encode() + + http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) return } + httpapi.Write(rw, code, response) } diff --git a/coderd/users_test.go b/coderd/users_test.go index 3a9b10aedc73b..dbb6824463a38 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "net/url" "sort" "strings" "testing" @@ -265,12 +264,7 @@ func TestPostLogout(t *testing.T) { t.Run("Logout", func(t *testing.T) { t.Parallel() - accessURL, err := url.Parse("http://somedomain.com") - require.NoError(t, err) - - client := coderdtest.New(t, &coderdtest.Options{ - AccessURL: accessURL, - }) + client := coderdtest.New(t, nil) admin := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 153cf17d0edad..fa6b03ee9f89e 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -128,7 +128,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { t.Parallel() client, orgID, workspace, port := setupProxyTest(t) - t.Run("RedirectsWithoutAuth", func(t *testing.T) { + t.Run("LoginWithoutAuth", func(t *testing.T) { t.Parallel() client := codersdk.New(client.URL) client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { @@ -141,13 +141,14 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { resp, err := client.Request(ctx, http.MethodGet, "/@me/"+workspace.Name+"/apps/example", nil) require.NoError(t, err) defer resp.Body.Close() - location, err := resp.Location() + + loc, err := resp.Location() require.NoError(t, err) - require.Equal(t, "/login", location.Path) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + require.True(t, loc.Query().Has("message")) + require.True(t, loc.Query().Has("redirect")) }) - t.Run("NoAccessShould401", func(t *testing.T) { + t.Run("NoAccessShould404", func(t *testing.T) { t.Parallel() userClient := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) @@ -284,7 +285,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { }).String() } - t.Run("NoAuthShould401", func(t *testing.T) { + t.Run("LoginWithoutAuth", func(t *testing.T) { t.Parallel() unauthedClient := codersdk.New(client.URL) unauthedClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect @@ -296,7 +297,17 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { resp, err := unauthedClient.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppName), nil) require.NoError(t, err) defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + loc, err := resp.Location() + require.NoError(t, err) + require.True(t, loc.Query().Has("message")) + require.False(t, loc.Query().Has("redirect")) + + expectedURL := *client.URL + expectedURL.Path = "/login" + loc.RawQuery = "" + require.Equal(t, &expectedURL, loc) }) t.Run("NoAccessShould401", func(t *testing.T) { From 5321befc69cbaa825b94f311cbe97dd2ff36d7ce Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 13 Sep 2022 16:09:21 +0000 Subject: [PATCH 23/24] fixup! kyle comments --- coderd/workspaceapps.go | 9 +++++++++ scripts/coder-dev.sh | 6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 44807d5fa73db..c82ed680a3258 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -56,6 +56,15 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht host := httpapi.RequestHost(r) if host == "" { + if r.URL.Path == "/derp" { + // The /derp endpoint is used by wireguard clients to tunnel + // through coderd. For some reason these requests don't set + // a Host header properly sometimes (no idea how), which + // causes this path to get hit. + next.ServeHTTP(rw, r) + return + } + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ Message: "Could not determine request Host.", }) diff --git a/scripts/coder-dev.sh b/scripts/coder-dev.sh index dfba6261bab8c..5c3de4e310d13 100755 --- a/scripts/coder-dev.sh +++ b/scripts/coder-dev.sh @@ -15,9 +15,9 @@ RELATIVE_BINARY_PATH="build/coder_${GOOS}_${GOARCH}" # To preserve the CWD when running the binary, we need to use pushd and popd to # get absolute paths to everything. pushd "$PROJECT_ROOT" - mkdir -p ./.coderv2 - CODER_DEV_BIN="$(realpath "$RELATIVE_BINARY_PATH")" - CODER_DEV_DIR="$(realpath ./.coderv2)" +mkdir -p ./.coderv2 +CODER_DEV_BIN="$(realpath "$RELATIVE_BINARY_PATH")" +CODER_DEV_DIR="$(realpath ./.coderv2)" popd if [[ ! -x "${CODER_DEV_BIN}" ]]; then From ad53b429c481777427dbac4f93aac1baff2d1b54 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 13 Sep 2022 16:50:37 +0000 Subject: [PATCH 24/24] fixup! kyle comments --- coderd/workspaceapps_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index fa6b03ee9f89e..2f601b629be9e 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -77,13 +77,16 @@ func setupProxyTest(t *testing.T) (*codersdk.Client, uuid.UUID, codersdk.Workspa Auth: &proto.Agent_Token{ Token: authToken, }, - Apps: []*proto.App{{ - Name: proxyTestAppName, - Url: fmt.Sprintf("http://127.0.0.1:%d?%s", tcpAddr.Port, proxyTestAppQuery), - }, { - Name: proxyTestFakeAppName, - Url: "http://127.0.0.2", - }}, + Apps: []*proto.App{ + { + Name: proxyTestAppName, + Url: fmt.Sprintf("http://127.0.0.1:%d?%s", tcpAddr.Port, proxyTestAppQuery), + }, { + Name: proxyTestFakeAppName, + // Hopefully this IP and port doesn't exist. + Url: "http://127.1.0.1:65535", + }, + }, }}, }}, },