Skip to content

feat: allow prefixes at the beginning of subdomain app hostnames #10150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 33 additions & 5 deletions coderd/httpapi/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var (

// ApplicationURL is a parsed application URL hostname.
type ApplicationURL struct {
Prefix string
AppSlugOrPort string
AgentName string
WorkspaceName string
Expand All @@ -32,6 +33,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)
Expand All @@ -47,20 +49,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")],
Expand Down Expand Up @@ -125,8 +149,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) {
Expand Down
22 changes: 22 additions & 0 deletions coderd/httpapi/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ func TestApplicationURLString(t *testing.T) {
},
Expected: "8080--agent--workspace--user",
},
{
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 {
Expand Down Expand Up @@ -123,6 +134,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 {
Expand Down
4 changes: 4 additions & 0 deletions coderd/workspaceagents.go
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,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,
Expand Down
113 changes: 113 additions & 0 deletions coderd/workspaceapps/apptest/apptest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
20 changes: 19 additions & 1 deletion coderd/workspaceapps/apptest/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions coderd/workspaceapps/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions coderd/workspaceapps/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
}
Expand All @@ -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,
Expand Down Expand Up @@ -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),
)
}

Expand Down
2 changes: 2 additions & 0 deletions coderd/workspaceapps/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Loading