Renamed from PR review

coder · Emyrk · Apr 5, 2023 · Apr 4, 2023 · Apr 4, 2023 · Apr 4, 2023
commit 70309ce6bbffbfffc47e385f4a78a8f8c0e77468
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -86,9 +86,9 @@ type Options struct {
 	// It will use the same scheme and port number as the access URL.
 	// E.g. "*.apps.coder.com" or "*-apps.coder.com".
 	AppHostname string
-	// AppHostnameRegex contains the regex version of options.AppHostname as
+	// AppHostnameRegex contains the regex version of options.Hostname as
 	// generated by httpapi.CompileHostnamePattern(). It MUST be set if
-	// options.AppHostname is set.
+	// options.Hostname is set.
 	AppHostnameRegex *regexp.Regexp
 	Logger           slog.Logger
 	Database         database.Store
@@ -125,7 +125,7 @@ type Options struct {
 	TemplateScheduleStore schedule.TemplateScheduleStore
 	// AppSigningKey denotes the symmetric key to use for signing temporary app
 	// tokens.
-	AppSigningKey      workspaceapps.AppSigningKey
+	AppSigningKey      workspaceapps.SigningKey
 	HealthcheckFunc    func(ctx context.Context) (*healthcheck.Report, error)
 	HealthcheckTimeout time.Duration
 	HealthcheckRefresh time.Duration
@@ -183,7 +183,7 @@ func New(options *Options) *API {
 		options.Logger, options.DeploymentValues.Experiments.Value(),
 	)
 	if options.AppHostname != "" && options.AppHostnameRegex == nil || options.AppHostname == "" && options.AppHostnameRegex != nil {
-		panic("coderd: both AppHostname and AppHostnameRegex must be set or unset")
+		panic("coderd: both Hostname and HostnameRegex must be set or unset")
 	}
 	if options.AgentConnectionUpdateFrequency == 0 {
 		options.AgentConnectionUpdateFrequency = 3 * time.Second
@@ -328,19 +328,19 @@ func New(options *Options) *API {
 	api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0)
 	api.TailnetCoordinator.Store(&options.TailnetCoordinator)
 
-	workspaceAppServer := workspaceapps.WorkspaceAppServer{
+	workspaceAppServer := workspaceapps.Server{
 		Logger: options.Logger.Named("workspaceapps"),
 
-		PrimaryAccessURL: api.AccessURL,
+		DashboardURL:     api.AccessURL,
 		AccessURL:        api.AccessURL,
-		AppHostname:      api.AppHostname,
-		AppHostnameRegex: api.AppHostnameRegex,
+		Hostname:         api.AppHostname,
+		HostnameRegex:    api.AppHostnameRegex,
 		DeploymentValues: options.DeploymentValues,
 		RealIPConfig:     options.RealIPConfig,
 
-		TokenProvider:      api.WorkspaceAppsProvider,
-		WorkspaceConnCache: api.workspaceAgentCache,
-		AppSigningKey:      options.AppSigningKey,
+		SignedTokenProvider: api.WorkspaceAppsProvider,
+		WorkspaceConnCache:  api.workspaceAgentCache,
+		AppSigningKey:       options.AppSigningKey,
 	}
 
 	apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{

diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -81,7 +81,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
 	// security purposes.
 	u.Scheme = api.AccessURL.Scheme
 
-	// Ensure that the redirect URI is a subdomain of api.AppHostname and is a
+	// Ensure that the redirect URI is a subdomain of api.Hostname and is a
 	// valid app subdomain.
 	subdomain, ok := httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, u.Host)
 	if !ok {

diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go
@@ -32,12 +32,12 @@ type DBTokenProvider struct {
 	DeploymentValues              *codersdk.DeploymentValues
 	OAuth2Configs                 *httpmw.OAuth2Configs
 	WorkspaceAgentInactiveTimeout time.Duration
-	TokenSigningKey               AppSigningKey
+	TokenSigningKey               SigningKey
 }
 
 var _ SignedTokenProvider = &DBTokenProvider{}
 
-func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey AppSigningKey) SignedTokenProvider {
+func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey SigningKey) SignedTokenProvider {
 	if len(tokenSigningKey) != 64 {
 		panic("token signing key must be 64 bytes")
 	}

diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go
@@ -19,14 +19,14 @@ const (
 	RedirectURIQueryParam = "redirect_uri"
 )
 
-// ResolveRequest calls TokenProvider to use an existing signed app token in the
+// ResolveRequest calls SignedTokenProvider to use an existing signed app token in the
 // request or issue a new one. If it returns a newly minted token, it sets the
 // cookie for you.
-func ResolveRequest(log slog.Logger, accessURL *url.URL, p SignedTokenProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, bool) {
+func ResolveRequest(log slog.Logger, dashboardURL *url.URL, p SignedTokenProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, bool) {
 	appReq = appReq.Normalize()
 	err := appReq.Validate()
 	if err != nil {
-		WriteWorkspaceApp500(log, accessURL, rw, r, &appReq, err, "invalid app request")
+		WriteWorkspaceApp500(log, dashboardURL, rw, r, &appReq, err, "invalid app request")
 		return nil, false
 	}
 

diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go
@@ -59,28 +59,34 @@ var nonCanonicalHeaders = map[string]string{
 	"Sec-Websocket-Version":    "Sec-WebSocket-Version",
 }
 
-// WorkspaceAppServer serves workspace apps endpoints, including:
+// Server serves workspace apps endpoints, including:
 // - Path-based apps
 // - Subdomain app middleware
 // - Workspace reconnecting-pty (aka. web terminal)
-type WorkspaceAppServer struct {
+type Server struct {
 	Logger slog.Logger
 
-	// PrimaryAccessURL should be a url to the coderd dashboard. This can be the
-	// same as the AccessURL if the WorkspaceAppServer is embedded.
-	PrimaryAccessURL *url.URL
-	AccessURL        *url.URL
-	AppHostname      string
-	AppHostnameRegex *regexp.Regexp
+	// DashboardURL should be a url to the coderd dashboard. This can be the
+	// same as the AccessURL if the Server is embedded.
+	DashboardURL *url.URL
+	AccessURL    *url.URL
+	// Hostname should be the wildcard hostname to use for workspace
+	// applications INCLUDING the asterisk, (optional) suffix and leading dot.
+	// It will use the same scheme and port number as the access URL.
+	// E.g. "*.apps.coder.com" or "*-apps.coder.com".
+	Hostname string
+	// HostnameRegex contains the regex version of Hostname as generated by
+	// httpapi.CompileHostnamePattern(). It MUST be set if Hostname is set.
+	HostnameRegex    *regexp.Regexp
 	DeploymentValues *codersdk.DeploymentValues
 	RealIPConfig     *httpmw.RealIPConfig
 
-	TokenProvider      SignedTokenProvider
-	WorkspaceConnCache *wsconncache.Cache
-	AppSigningKey      AppSigningKey
+	SignedTokenProvider SignedTokenProvider
+	WorkspaceConnCache  *wsconncache.Cache
+	AppSigningKey       SigningKey
 }
 
-func (s *WorkspaceAppServer) Attach(r chi.Router, pathAppRateLimiter func(http.Handler) http.Handler) {
+func (s *Server) Attach(r chi.Router, pathAppRateLimiter func(http.Handler) http.Handler) {
 	servePathApps := func(r chi.Router) {
 		r.Use(pathAppRateLimiter)
 		r.HandleFunc("/*", s.workspaceAppsProxyPath)
@@ -97,14 +103,14 @@ func (s *WorkspaceAppServer) Attach(r chi.Router, pathAppRateLimiter func(http.H
 
 // workspaceAppsProxyPath proxies requests to a workspace application
 // through a relative URL path.
-func (s *WorkspaceAppServer) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) {
+func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) {
 	if s.DeploymentValues.DisablePathApps.Value() {
 		site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
 			Status:       http.StatusUnauthorized,
 			Title:        "Unauthorized",
 			Description:  "Path-based applications are disabled on this Coder deployment by the administrator.",
 			RetryEnabled: false,
-			DashboardURL: s.PrimaryAccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return
 	}
@@ -117,7 +123,7 @@ func (s *WorkspaceAppServer) workspaceAppsProxyPath(rw http.ResponseWriter, r *h
 			Title:        "Application Not Found",
 			Description:  "Applications must be accessed with the full username, not @me.",
 			RetryEnabled: false,
-			DashboardURL: s.PrimaryAccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return
 	}
@@ -132,7 +138,7 @@ func (s *WorkspaceAppServer) workspaceAppsProxyPath(rw http.ResponseWriter, r *h
 
 	// ResolveRequest will only return a new signed token if the actor has the RBAC
 	// permissions to connect to a workspace.
-	token, ok := ResolveRequest(s.Logger, s.AccessURL, s.TokenProvider, rw, r, Request{
+	token, ok := ResolveRequest(s.Logger, s.DashboardURL, s.SignedTokenProvider, rw, r, Request{
 		AccessMethod:      AccessMethodPath,
 		BasePath:          basePath,
 		UsernameOrID:      chi.URLParam(r, "user"),
@@ -152,20 +158,20 @@ func (s *WorkspaceAppServer) workspaceAppsProxyPath(rw http.ResponseWriter, r *h
 // DevURLs in Coder V1).
 //
 // There are a lot of paths here:
-//  1. If api.AppHostname is not set then we pass on.
+//  1. If api.Hostname is not set then we pass on.
 //  2. If we can't read the request hostname then we return a 400.
 //  3. If the request hostname matches api.AccessURL then we pass on.
 //  5. We split the subdomain into the subdomain and the "rest". If there are no
 //     periods in the hostname then we pass on.
 //  5. We parse the subdomain into a httpapi.ApplicationURL struct. If we
 //     encounter an error:
-//     a. If the "rest" does not match api.AppHostname then we pass on;
+//     a. If the "rest" does not match api.Hostname then we pass on;
 //     b. Otherwise, we return a 400.
-//  6. Finally, we verify that the "rest" matches api.AppHostname, else we
+//  6. Finally, we verify that the "rest" matches api.Hostname, else we
 //     return a 404.
 //
 // Rationales for each of the above steps:
-//  1. We pass on if api.AppHostname is not set to avoid returning any errors if
+//  1. We pass on if api.Hostname is not set to avoid returning any errors if
 //     `--app-hostname` is not configured.
 //  2. Every request should have a valid Host header anyways.
 //  3. We pass on if the request hostname matches api.AccessURL so we can
@@ -175,22 +181,22 @@ func (s *WorkspaceAppServer) workspaceAppsProxyPath(rw http.ResponseWriter, r *h
 //     must be a subdomain of a hostname, which implies there must be at least
 //     one period.
 //  5. a. If the request subdomain is not a valid application URL, and the
-//     "rest" does not match api.AppHostname, then it is very unlikely that
+//     "rest" does not match api.Hostname, then it is very unlikely that
 //     the request was intended for this handler. We pass on.
 //     b. If the request subdomain is not a valid application URL, but the
-//     "rest" matches api.AppHostname, then we return a 400 because the
+//     "rest" matches api.Hostname, then we return a 400 because the
 //     request is probably a typo or something.
-//  6. We finally verify that the "rest" matches api.AppHostname for security
+//  6. We finally verify that the "rest" matches api.Hostname for security
 //     purposes regarding re-authentication and application proxy session
 //     tokens.
-func (s *WorkspaceAppServer) SubdomainAppMW(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler {
+func (s *Server) SubdomainAppMW(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()
 
 			// Step 1: Pass on if subdomain-based application proxying is not
 			// configured.
-			if s.AppHostname == "" || s.AppHostnameRegex == nil {
+			if s.Hostname == "" || s.HostnameRegex == nil {
 				next.ServeHTTP(rw, r)
 				return
 			}
@@ -214,7 +220,7 @@ func (s *WorkspaceAppServer) SubdomainAppMW(middlewares ...func(http.Handler) ht
 			}
 
 			// Steps 3-6: Parse application from subdomain.
-			app, ok := s.parseWorkspaceApplicationHostname(rw, r, next, host)
+			app, ok := s.parseHostname(rw, r, next, host)
 			if !ok {
 				return
 			}
@@ -233,7 +239,7 @@ func (s *WorkspaceAppServer) SubdomainAppMW(middlewares ...func(http.Handler) ht
 						// Retry is disabled because the user needs to remove
 						// the query parameter before they try again.
 						RetryEnabled: false,
-						DashboardURL: s.AccessURL.String(),
+						DashboardURL: s.DashboardURL.String(),
 					})
 					return
 				}
@@ -256,7 +262,7 @@ func (s *WorkspaceAppServer) SubdomainAppMW(middlewares ...func(http.Handler) ht
 				return
 			}
 
-			token, ok := ResolveRequest(s.Logger, s.AccessURL, s.TokenProvider, rw, r, Request{
+			token, ok := ResolveRequest(s.Logger, s.DashboardURL, s.SignedTokenProvider, rw, r, Request{
 				AccessMethod:      AccessMethodSubdomain,
 				BasePath:          "/",
 				UsernameOrID:      app.Username,
@@ -278,11 +284,16 @@ func (s *WorkspaceAppServer) SubdomainAppMW(middlewares ...func(http.Handler) ht
 	}
 }
 
-func (s *WorkspaceAppServer) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *http.Request, next http.Handler, host string) (httpapi.ApplicationURL, bool) {
+// parseHostname will return if a given request is attempting to access a
+// workspace app via a subdomain. If it is, the hostname of the request is parsed
+// into an httpapi.ApplicationURL and true is returned. If the request is not
+// accessing a workspace app, then the next handler is called and false is
+// returned.
+func (s *Server) parseHostname(rw http.ResponseWriter, r *http.Request, next http.Handler, host string) (httpapi.ApplicationURL, bool) {
 	// Check if the hostname matches either of the access URLs. If it does, the
 	// user was definitely trying to connect to the dashboard/API or a
 	// path-based app.
-	if httpapi.HostnamesMatch(s.PrimaryAccessURL.Hostname(), host) || httpapi.HostnamesMatch(s.AccessURL.Hostname(), host) {
+	if httpapi.HostnamesMatch(s.DashboardURL.Hostname(), host) || httpapi.HostnamesMatch(s.AccessURL.Hostname(), host) {
 		next.ServeHTTP(rw, r)
 		return httpapi.ApplicationURL{}, false
 	}
@@ -296,7 +307,7 @@ func (s *WorkspaceAppServer) parseWorkspaceApplicationHostname(rw http.ResponseW
 
 	// Split the subdomain so we can parse the application details and verify it
 	// matches the configured app hostname later.
-	subdomain, ok := httpapi.ExecuteHostnamePattern(s.AppHostnameRegex, host)
+	subdomain, ok := httpapi.ExecuteHostnamePattern(s.HostnameRegex, host)
 	if !ok {
 		// Doesn't match the regex, so it's not a valid application URL.
 		next.ServeHTTP(rw, r)
@@ -318,7 +329,7 @@ func (s *WorkspaceAppServer) parseWorkspaceApplicationHostname(rw http.ResponseW
 			Title:        "Invalid Application URL",
 			Description:  fmt.Sprintf("Could not parse subdomain application URL %q: %s", subdomain, err.Error()),
 			RetryEnabled: false,
-			DashboardURL: s.AccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return httpapi.ApplicationURL{}, false
 	}
@@ -329,23 +340,23 @@ func (s *WorkspaceAppServer) parseWorkspaceApplicationHostname(rw http.ResponseW
 // setWorkspaceAppCookie sets a cookie on the workspace app domain. If the app
 // hostname cannot be parsed properly, a static error page is rendered and false
 // is returned.
-func (s *WorkspaceAppServer) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool {
-	hostSplit := strings.SplitN(s.AppHostname, ".", 2)
+func (s *Server) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool {
+	hostSplit := strings.SplitN(s.Hostname, ".", 2)
 	if len(hostSplit) != 2 {
 		// This should be impossible as we verify the app hostname on
 		// startup, but we'll check anyways.
-		s.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", s.AppHostname))
+		s.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", s.Hostname))
 		site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
 			Status:       http.StatusInternalServerError,
 			Title:        "Internal Server Error",
 			Description:  "The app is configured with an invalid app wildcard hostname. Please contact an administrator.",
 			RetryEnabled: false,
-			DashboardURL: s.AccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return false
 	}
 
-	// Set the app cookie for all subdomains of s.AppHostname. We don't set an
+	// Set the app cookie for all subdomains of s.Hostname. We don't set an
 	// expiration because the key in the database already has an expiration, and
 	// expired tokens don't affect the user experience (they get auto-redirected
 	// to re-smuggle the API key).
@@ -364,7 +375,7 @@ func (s *WorkspaceAppServer) setWorkspaceAppCookie(rw http.ResponseWriter, r *ht
 	return true
 }
 
-func (s *WorkspaceAppServer) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appToken SignedToken, path string) {
+func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appToken SignedToken, path string) {
 	ctx := r.Context()
 
 	// Filter IP headers from untrusted origins.
@@ -384,7 +395,7 @@ func (s *WorkspaceAppServer) proxyWorkspaceApp(rw http.ResponseWriter, r *http.R
 			Title:        "Bad Request",
 			Description:  fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()),
 			RetryEnabled: true,
-			DashboardURL: s.PrimaryAccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return
 	}
@@ -439,7 +450,7 @@ func (s *WorkspaceAppServer) proxyWorkspaceApp(rw http.ResponseWriter, r *http.R
 			Title:        "Bad Gateway",
 			Description:  "Failed to proxy request to application: " + err.Error(),
 			RetryEnabled: true,
-			DashboardURL: s.PrimaryAccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 	}
 
@@ -450,7 +461,7 @@ func (s *WorkspaceAppServer) proxyWorkspaceApp(rw http.ResponseWriter, r *http.R
 			Title:        "Bad Gateway",
 			Description:  "Could not connect to workspace agent: " + err.Error(),
 			RetryEnabled: true,
-			DashboardURL: s.PrimaryAccessURL.String(),
+			DashboardURL: s.DashboardURL.String(),
 		})
 		return
 	}
@@ -490,7 +501,7 @@ func (s *WorkspaceAppServer) proxyWorkspaceApp(rw http.ResponseWriter, r *http.R
 // @Param workspaceagent path string true "Workspace agent ID" format(uuid)
 // @Success 101
 // @Router /workspaceagents/{workspaceagent}/pty [get]
-func (s *WorkspaceAppServer) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
+func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
 	// TODO: Fix this later
@@ -499,7 +510,7 @@ func (s *WorkspaceAppServer) workspaceAgentPTY(rw http.ResponseWriter, r *http.R
 	// s.WebsocketWaitMutex.Unlock()
 	// defer s.WebsocketWaitGroup.Done()
 
-	appToken, ok := ResolveRequest(s.Logger, s.AccessURL, s.TokenProvider, rw, r, Request{
+	appToken, ok := ResolveRequest(s.Logger, s.AccessURL, s.SignedTokenProvider, rw, r, Request{
 		AccessMethod:  AccessMethodTerminal,
 		BasePath:      r.URL.Path,
 		AgentNameOrID: chi.URLParam(r, "workspaceagent"),