feat: allow prefixes at the beginning of subdomain app hostnames

coder · deansheather · Oct 10, 2023 · Oct 9, 2023 · Oct 10, 2023 · Oct 10, 2023
commit 2a7f2e3726d6f6fa9260aed115bc6f93d358128d
diff --git a/coderd/httpapi/url.go b/coderd/httpapi/url.go
@@ -25,6 +25,7 @@ var (
 
 // ApplicationURL is a parsed application URL hostname.
 type ApplicationURL struct {
+	Prefix        string
 	AppSlugOrPort string
 	AgentName     string
 	WorkspaceName string
@@ -35,6 +36,7 @@ type ApplicationURL struct {
 // want to append a period and the base hostname.
 func (a ApplicationURL) String() string {
 	var appURL strings.Builder
+	_, _ = appURL.WriteString(a.Prefix)
 	_, _ = appURL.WriteString(a.AppSlugOrPort)
 	_, _ = appURL.WriteString("--")
 	_, _ = appURL.WriteString(a.AgentName)
@@ -44,7 +46,7 @@ func (a ApplicationURL) String() string {
 	_, _ = appURL.WriteString(a.Username)
 	hostname := appURL.String()
 
-	if len(hostname) < 64 { // max length for the subdomain level
+	if len(hostname) < 62 { // max length for the subdomain level
 		return hostname
 	}
 	return fmt.Sprintf("app-%08x", crc32.Checksum([]byte(hostname), crcTable))
@@ -55,20 +57,42 @@ func (a ApplicationURL) String() string {
 // error. If the hostname is not a subdomain of the given base hostname, returns
 // a non-nil error.
 //
-// The base hostname should not include a scheme, leading asterisk or dot.
-//
 // Subdomains should be in the form:
 //
-//	{PORT/APP_SLUG}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME}
-//	(eg. https://8080--main--dev--dean.hi.c8s.io)
+//		({PREFIX}---)?{PORT/APP_SLUG}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME}
+//		e.g.
+//	     https://8080--main--dev--dean.hi.c8s.io
+//	     https://app--main--dev--dean.hi.c8s.io
+//	     https://prefix---8080--main--dev--dean.hi.c8s.io
+//	     https://prefix---app--main--dev--dean.hi.c8s.io
+//
+// The optional prefix is permitted to allow customers to put additional URL at
+// the beginning of their application URL (i.e. if they want to simulate
+// different subdomains on the same app/port).
+//
+// Prefix requires three hyphens at the end to separate it from the rest of the
+// URL so we can add/remove segments in the future from the parsing logic.
+//
+// TODO(dean): make the agent name optional when using the app slug. This will
+// reduce the character count for app URLs.
 func ParseSubdomainAppURL(subdomain string) (ApplicationURL, error) {
+	var (
+		prefixSegments = strings.Split(subdomain, "---")
+		prefix         = ""
+	)
+	if len(prefixSegments) > 1 {
+		prefix = strings.Join(prefixSegments[:len(prefixSegments)-1], "---") + "---"
+		subdomain = prefixSegments[len(prefixSegments)-1]
+	}
+
 	matches := appURL.FindAllStringSubmatch(subdomain, -1)
 	if len(matches) == 0 {
 		return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain)
 	}
 	matchGroup := matches[0]
 
 	return ApplicationURL{
+		Prefix:        prefix,
 		AppSlugOrPort: matchGroup[appURL.SubexpIndex("AppSlug")],
 		AgentName:     matchGroup[appURL.SubexpIndex("AgentName")],
 		WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")],
@@ -133,8 +157,12 @@ func CompileHostnamePattern(pattern string) (*regexp.Regexp, error) {
 	}
 	for i, label := range strings.Split(pattern, ".") {
 		if i == 0 {
-			// We have to allow the asterisk to be a valid hostname label.
+			// We have to allow the asterisk to be a valid hostname label, so
+			// we strip the asterisk (which is only on the first one).
 			label = strings.TrimPrefix(label, "*")
+			// Put an "a" at the start to stand in for the asterisk in the regex
+			// test below. This makes `*.coder.com` become `a.coder.com` and
+			// `*--prod.coder.com` become `a--prod.coder.com`.
 			label = "a" + label
 		}
 		if !validHostnameLabelRegex.MatchString(label) {

diff --git a/coderd/httpapi/url_test.go b/coderd/httpapi/url_test.go
@@ -52,6 +52,17 @@ func TestApplicationURLString(t *testing.T) {
 			},
 			Expected: "app-90667f72",
 		},
+		{
+			Name: "Prefix",
+			URL: httpapi.ApplicationURL{
+				Prefix:        "yolo---",
+				AppSlugOrPort: "app",
+				AgentName:     "agent",
+				WorkspaceName: "workspace",
+				Username:      "user",
+			},
+			Expected: "yolo---app--agent--workspace--user",
+		},
 	}
 
 	for _, c := range testCases {
@@ -133,6 +144,17 @@ func TestParseSubdomainAppURL(t *testing.T) {
 				Username:      "user-name",
 			},
 		},
+		{
+			Name:      "Prefix",
+			Subdomain: "dean---was---here---app--agent--workspace--user",
+			Expected: httpapi.ApplicationURL{
+				Prefix:        "dean---was---here---",
+				AppSlugOrPort: "app",
+				AgentName:     "agent",
+				WorkspaceName: "workspace",
+				Username:      "user",
+			},
+		},
 	}
 
 	for _, c := range testCases {

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -1409,6 +1409,10 @@ func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent,
 				appSlug = dbApp.DisplayName
 			}
 			subdomainName = httpapi.ApplicationURL{
+				// We never generate URLs with a prefix. We only allow prefixes
+				// when parsing URLs from the hostname. Users that want this
+				// feature can write out their own URLs.
+				Prefix:        "",
 				AppSlugOrPort: appSlug,
 				AgentName:     agent.Name,
 				WorkspaceName: workspace.Name,

diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go
@@ -19,6 +19,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-jose/go-jose/v3"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -552,6 +553,118 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
 		})
 	})
 
+	t.Run("WorkspaceAppsProxySubdomainHostnamePrefix/OK", func(t *testing.T) {
+		t.Parallel()
+
+		appDetails := setupProxyTest(t, nil)
+
+		// Try to load the owner app with a prefix.
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		prefixedOwnerApp := appDetails.Apps.Owner
+		prefixedOwnerApp.Prefix = "some---prefix---"
+
+		u := appDetails.SubdomainAppURL(prefixedOwnerApp)
+		require.Contains(t, u.Host, prefixedOwnerApp.Prefix)
+
+		resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, u.String(), nil)
+		require.NoError(t, err)
+		_ = resp.Body.Close()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		require.Equal(t, resp.Header.Get("X-Got-Host"), u.Host)
+
+		// Parse the returned signed token to verify that it contains the
+		// prefix.
+		var appTokenCookie *http.Cookie
+		for _, c := range resp.Cookies() {
+			if c.Name == codersdk.SignedAppTokenCookie {
+				appTokenCookie = c
+				break
+			}
+		}
+		require.NotNil(t, appTokenCookie, "no signed app token cookie in response")
+
+		// Parse the JWT without verifying it (since we can't access the key
+		// from this test).
+		object, err := jose.ParseSigned(appTokenCookie.Value)
+		require.NoError(t, err)
+		require.Len(t, object.Signatures, 1)
+
+		// Parse the payload.
+		var tok workspaceapps.SignedToken
+		//nolint:gosec
+		err = json.Unmarshal(object.UnsafePayloadWithoutVerification(), &tok)
+		require.NoError(t, err)
+
+		// Verify the prefix is in the token.
+		require.Equal(t, prefixedOwnerApp.Prefix, tok.Request.Prefix)
+
+		// Ensure the signed app token cookie is valid by making a request with
+		// it with no session token.
+		appTokenClient := appDetails.AppClient(t)
+		appTokenClient.SetSessionToken("")
+		appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil)
+		require.NoError(t, err)
+		appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie})
+
+		resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, u.String(), nil)
+		require.NoError(t, err)
+		_ = resp.Body.Close()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		require.Equal(t, resp.Header.Get("X-Got-Host"), u.Host)
+	})
+
+	t.Run("WorkspaceAppsProxySubdomainHostnamePrefix/Different", func(t *testing.T) {
+		t.Parallel()
+
+		appDetails := setupProxyTest(t, nil)
+
+		// Try to load the owner app with a prefix.
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		prefixedOwnerApp := appDetails.Apps.Owner
+		t.Log(appDetails.SubdomainAppURL(prefixedOwnerApp))
+		prefixedOwnerApp.Prefix = "some---prefix---"
+		t.Log(appDetails.SubdomainAppURL(prefixedOwnerApp))
+
+		u := appDetails.SubdomainAppURL(prefixedOwnerApp)
+		require.Contains(t, u.Host, prefixedOwnerApp.Prefix)
+
+		resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, u.String(), nil)
+		require.NoError(t, err)
+		_ = resp.Body.Close()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+
+		// Find the cookie.
+		var appTokenCookie *http.Cookie
+		for _, c := range resp.Cookies() {
+			if c.Name == codersdk.SignedAppTokenCookie {
+				appTokenCookie = c
+				break
+			}
+		}
+		require.NotNil(t, appTokenCookie, "no signed app token cookie in response")
+
+		// Ensure the signed app token cookie is valid only for the given prefix
+		// by making a request with it with no session token.
+		appTokenClient := appDetails.AppClient(t)
+		appTokenClient.SetSessionToken("")
+		appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil)
+		require.NoError(t, err)
+		appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie})
+
+		prefixedOwnerApp.Prefix = "different---"
+		u = appDetails.SubdomainAppURL(prefixedOwnerApp)
+		require.Contains(t, u.Host, prefixedOwnerApp.Prefix)
+
+		resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, u.String(), nil)
+		require.NoError(t, err)
+		_ = resp.Body.Close()
+		require.NotEqual(t, http.StatusOK, resp.StatusCode)
+	})
+
 	// This test ensures that the subdomain handler does nothing if
 	// --app-hostname is not set by the admin.
 	t.Run("WorkspaceAppsProxySubdomainPassthrough", func(t *testing.T) {

diff --git a/coderd/workspaceapps/apptest/setup.go b/coderd/workspaceapps/apptest/setup.go
@@ -25,13 +25,14 @@ import (
 	"github.com/coder/coder/v2/coderd/workspaceapps"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/agentsdk"
+	"github.com/coder/coder/v2/cryptorand"
 	"github.com/coder/coder/v2/provisioner/echo"
 	"github.com/coder/coder/v2/provisionersdk/proto"
 	"github.com/coder/coder/v2/testutil"
 )
 
 const (
-	proxyTestAgentName            = "agent-name"
+	proxyTestAgentName            = "agnt-name"
 	proxyTestAppNameFake          = "taf"
 	proxyTestAppNameOwner         = "tao"
 	proxyTestAppNameAuthenticated = "taa"
@@ -88,7 +89,9 @@ type App struct {
 	AgentName     string
 	AppSlugOrPort string
 
-	Query string
+	// Prefix should have ---.
+	Prefix string
+	Query  string
 }
 
 // Details are the full test details returned from setupProxyTestWithFactory.
@@ -143,6 +146,7 @@ func (d *Details) PathAppURL(app App) *url.URL {
 // SubdomainAppURL returns the URL for the given subdomain app.
 func (d *Details) SubdomainAppURL(app App) *url.URL {
 	appHost := httpapi.ApplicationURL{
+		Prefix:        app.Prefix,
 		AppSlugOrPort: app.AppSlugOrPort,
 		AgentName:     app.AgentName,
 		WorkspaceName: app.WorkspaceName,
@@ -252,6 +256,7 @@ func appServer(t *testing.T, headers http.Header, isHTTPS bool) uint16 {
 				_, err := r.Cookie(codersdk.SessionTokenCookie)
 				assert.ErrorIs(t, err, http.ErrNoCookie)
 				w.Header().Set("X-Forwarded-For", r.Header.Get("X-Forwarded-For"))
+				w.Header().Set("X-Got-Host", r.Host)
 				for name, values := range headers {
 					for _, value := range values {
 						w.Header().Add(name, value)
@@ -290,6 +295,17 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
 		scheme = "https"
 	}
 
+	// Workspace name needs to be short to avoid hitting 62 char hostname
+	// segment limit.
+	workspaceName, err := cryptorand.String(6)
+	require.NoError(t, err)
+	workspaceName = "ws-" + workspaceName
+	workspaceMutators = append([]func(*codersdk.CreateWorkspaceRequest){
+		func(req *codersdk.CreateWorkspaceRequest) {
+			req.Name = workspaceName
+		},
+	}, workspaceMutators...)
+
 	appURL := fmt.Sprintf("%s://127.0.0.1:%d?%s", scheme, port, proxyTestAppQuery)
 	protoApps := []*proto.App{
 		{
@@ -354,6 +370,7 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
 		require.True(t, app.Subdomain)
 
 		appURL := httpapi.ApplicationURL{
+			Prefix: "",
 			// findProtoApp is needed as the order of apps returned from PG database
 			// is not guaranteed.
 			AppSlugOrPort: findProtoApp(t, protoApps, app.Slug).Slug,
@@ -382,6 +399,7 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
 		require.NoError(t, err)
 
 		appHost := httpapi.ApplicationURL{
+			Prefix:        "",
 			AppSlugOrPort: "{{port}}",
 			AgentName:     proxyTestAgentName,
 			WorkspaceName: workspace.Name,

diff --git a/coderd/workspaceapps/db_test.go b/coderd/workspaceapps/db_test.go
@@ -752,6 +752,7 @@ func Test_ResolveRequest(t *testing.T) {
 		require.NoError(t, err)
 
 		appHost := httpapi.ApplicationURL{
+			Prefix:        "",
 			AppSlugOrPort: req.AppSlugOrPort,
 			AgentName:     req.AgentNameOrID,
 			WorkspaceName: req.WorkspaceNameOrID,

diff --git a/coderd/workspaceapps/errors.go b/coderd/workspaceapps/errors.go
@@ -22,6 +22,7 @@ func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseW
 			slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID),
 			slog.F("agent_name_or_id", appReq.AgentNameOrID),
 			slog.F("app_slug_or_port", appReq.AppSlugOrPort),
+			slog.F("hostname_prefix", appReq.Prefix),
 			slog.F("warnings", warnings),
 		)
 	}
@@ -48,6 +49,7 @@ func WriteWorkspaceApp500(log slog.Logger, accessURL *url.URL, rw http.ResponseW
 			slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID),
 			slog.F("agent_name_or_id", appReq.AgentNameOrID),
 			slog.F("app_name_or_port", appReq.AppSlugOrPort),
+			slog.F("hostname_prefix", appReq.Prefix),
 		)
 	}
 	log.Warn(ctx,
@@ -76,6 +78,7 @@ func WriteWorkspaceAppOffline(log slog.Logger, accessURL *url.URL, rw http.Respo
 			slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID),
 			slog.F("agent_name_or_id", appReq.AgentNameOrID),
 			slog.F("app_slug_or_port", appReq.AppSlugOrPort),
+			slog.F("hostname_prefix", appReq.Prefix),
 		)
 	}
 

diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go
@@ -301,6 +301,7 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
 		AppRequest: Request{
 			AccessMethod:      AccessMethodPath,
 			BasePath:          basePath,
+			Prefix:            "", // Prefix doesn't exist for path apps
 			UsernameOrID:      chi.URLParam(r, "user"),
 			WorkspaceAndAgent: chi.URLParam(r, "workspace_and_agent"),
 			// We don't support port proxying on paths. The ResolveRequest method
@@ -405,6 +406,7 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler)
 					AppRequest: Request{
 						AccessMethod:      AccessMethodSubdomain,
 						BasePath:          "/",
+						Prefix:            app.Prefix,
 						UsernameOrID:      app.Username,
 						WorkspaceNameOrID: app.WorkspaceName,
 						AgentNameOrID:     app.AgentName,