diff --git a/cli/server.go b/cli/server.go index 925a1a619a840..abf7d0aaa0e4f 100644 --- a/cli/server.go +++ b/cli/server.go @@ -688,7 +688,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/coderd.go b/coderd/coderd.go index 14a18e7986aae..8d90c934fb846 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -160,6 +160,26 @@ func New(options *Options) *API { httpmw.Recover(api.Logger), httpmw.Logger(api.Logger), httpmw.Prometheus(options.PrometheusRegistry), + // handleSubdomainApplications checks if the first subdomain is a valid + // app URL. If it is, it will serve that application. + 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, true), + httpmw.ExtractUserParam(api.Database), + httpmw.ExtractWorkspaceAndAgentParam(api.Database), + ), // Build-Version is helpful for debugging. func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 490dce5a125a6..646835d8fe532 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -182,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) 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/httpapi/username.go b/coderd/httpapi/username.go index f6e5fcc28dca3..f41e0d96d3205 100644 --- a/coderd/httpapi/username.go +++ b/coderd/httpapi/username.go @@ -8,8 +8,8 @@ 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-]*") ) // UsernameValid returns whether the input string is a valid username. @@ -20,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/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/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 020212e4185a1..7d68526c030b1 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -162,7 +162,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) + api.setAuthCookie(rw, cookie) redirect := state.Redirect if redirect == "" { @@ -296,7 +296,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) + api.setAuthCookie(rw, cookie) redirect := state.Redirect if redirect == "" { diff --git a/coderd/users.go b/coderd/users.go index 6c8046d94665d..b2f97b6c19744 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -864,7 +864,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { return } - http.SetCookie(rw, cookie) + api.setAuthCookie(rw, cookie) httpapi.Write(rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ SessionToken: cookie.Value, @@ -941,8 +941,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { Name: codersdk.SessionTokenKey, Path: "/", } - - http.SetCookie(rw, cookie) + api.setAuthCookie(rw, cookie) // Delete the session token from database. apiKey := httpmw.APIKey(r) @@ -1122,6 +1121,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 b47c805bfe883..dbb6824463a38 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -284,9 +284,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) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index de05d27789aba..c82ed680a3258 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -1,9 +1,8 @@ package coderd import ( - "database/sql" - "errors" "fmt" + "net" "net/http" "net/http/httputil" "net/url" @@ -31,60 +30,153 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } - app, err := api.Database.GetWorkspaceAppByAgentIDAndName(r.Context(), database.GetWorkspaceAppByAgentIDAndNameParams{ - AgentID: agent.ID, - Name: chi.URLParam(r, "workspaceapp"), - }) - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(rw, http.StatusNotFound, codersdk.Response{ - Message: "Application not found.", - }) - 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 } - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace application.", - Detail: err.Error(), + + appName, port := httpapi.AppNameOrPort(chi.URLParam(r, "workspaceapp")) + api.proxyWorkspaceApplication(proxyApplication{ + Workspace: workspace, + Agent: agent, + AppName: appName, + Port: port, + Path: chiPath, + DashboardOnError: true, + }, rw, r) +} + +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() + + 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.", + }) + return + } + + app, err := httpapi.ParseSubdomainAppURL(host) + if err != nil { + // 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 + // though? + next.ServeHTTP(rw, r) + return + } + + 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) + + // 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, + Port: app.Port, + Path: r.URL.Path, + DashboardOnError: false, + }, rw, r) + })).ServeHTTP(rw, r.WithContext(ctx)) }) + } +} + +// proxyApplication are the required fields to proxy a workspace application. +type proxyApplication struct { + Workspace database.Workspace + Agent database.WorkspaceAgent + + // 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 + + // 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) { + if !api.Authorize(r, rbac.ActionCreate, proxyApp.Workspace.ExecutionRBAC()) { + httpapi.ResourceNotFound(rw) return } - if !app.Url.Valid { - httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Application %s does not have a url.", app.Name), + + // 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, }) - 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 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 @@ -94,9 +186,30 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) return } - r.URL.Path = path - conn, release, err := api.workspaceAgentCache.Acquire(r, agent.ID) + r.URL.Path = proxyApp.Path + appURL.RawQuery = "" + + proxy := httputil.NewSingleHostReverseProxy(appURL) + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + 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) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to dial workspace agent.", @@ -119,3 +232,23 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) proxy.ServeHTTP(rw, r) } + +// 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. +// +// 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. + appCookie.Domain = "." + api.AccessURL.Hostname() + return &appCookie +} diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 7e67f057bb354..2f601b629be9e 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -6,6 +6,7 @@ import ( "io" "net" "net/http" + "net/url" "testing" "time" @@ -16,14 +17,26 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" "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" "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) @@ -33,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() { @@ -40,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, @@ -57,17 +72,21 @@ 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: "fake", - 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", + }, + }, }}, }}, }, @@ -91,11 +110,28 @@ 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) +} - t.Run("RedirectsWithoutAuth", func(t *testing.T) { +func TestWorkspaceAppsProxyPath(t *testing.T) { + t.Parallel() + client, orgID, workspace, port := setupProxyTest(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 { @@ -108,10 +144,27 @@ 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("NoAccessShould404", 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) { @@ -138,7 +191,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) { @@ -147,12 +200,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, "", string(body)) + 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, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) }) @@ -165,6 +234,174 @@ 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 := httpapi.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("LoginWithoutAuth", 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.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) { + 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) + }) +} diff --git a/scripts/coder-dev.sh b/scripts/coder-dev.sh index 5f9b2d02fba3e..5c3de4e310d13 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 +# 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 fb302f894b6db..7d4d901c4bd96 100644 --- a/site/src/components/AppLink/AppLink.tsx +++ b/site/src/components/AppLink/AppLink.tsx @@ -28,7 +28,10 @@ export const AppLink: FC> = ({ appCommand, }) => { const styles = useStyles() - let 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. + let href = `/@${userName}/${workspaceName}.${agentName}/apps/${encodeURIComponent(appName)}/` if (appCommand) { href = `/@${userName}/${workspaceName}.${agentName}/terminal?command=${encodeURIComponent( appCommand,