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

This commit is contained in:
Dean Sheather
2023-10-10 20:02:39 +00:00
committed by GitHub
parent f48bc33e00
commit e7d9b8d858
15 changed files with 306 additions and 6 deletions
+113
View File
@@ -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) {
+19 -1
View File
@@ -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"
@@ -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,
+1
View File
@@ -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,
+3
View File
@@ -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),
)
}
+2
View File
@@ -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,
+11
View File
@@ -64,6 +64,7 @@ func (r IssueTokenRequest) AppBaseURL() (*url.URL, error) {
}
appHost := httpapi.ApplicationURL{
Prefix: r.AppRequest.Prefix,
AppSlugOrPort: r.AppRequest.AppSlugOrPort,
AgentName: r.AppRequest.AgentNameOrID,
WorkspaceName: r.AppRequest.WorkspaceNameOrID,
@@ -83,6 +84,9 @@ type Request struct {
// for this particular app. For subdomain apps, this should be "/". This is
// used for setting the cookie path.
BasePath string `json:"base_path"`
// Prefix is the prefix of the subdomain app URL. Prefix should have a
// trailing "---" if set.
Prefix string `json:"app_prefix"`
// For the following fields, if the AccessMethod is AccessMethodTerminal,
// then only AgentNameOrID may be set and it must be a UUID. The other
@@ -170,6 +174,13 @@ func (r Request) Validate() error {
return xerrors.New("app slug or port is required")
}
if r.Prefix != "" && r.AccessMethod != AccessMethodSubdomain {
return xerrors.New("prefix is only valid for subdomain apps")
}
if r.Prefix != "" && !strings.HasSuffix(r.Prefix, "---") {
return xerrors.New("prefix must have a trailing '---'")
}
return nil
}
+38
View File
@@ -153,6 +153,44 @@ func Test_RequestValidate(t *testing.T) {
},
errContains: "app slug or port is required",
},
{
name: "Prefix/OK",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "blah---",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
},
{
name: "Prefix/Invalid",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "blah", // no trailing ---
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
errContains: "prefix must have a trailing '---'",
},
{
name: "Prefix/NotAllowedPath",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath,
Prefix: "blah---",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
errContains: "prefix is only valid for subdomain apps",
},
{
name: "Terminal/OtherFields/UsernameOrID",
req: workspaceapps.Request{
+1
View File
@@ -50,6 +50,7 @@ func (t SignedToken) MatchesRequest(req Request) bool {
return t.AccessMethod == req.AccessMethod &&
tokenBasePath == reqBasePath &&
t.Prefix == req.Prefix &&
t.UsernameOrID == req.UsernameOrID &&
t.WorkspaceNameOrID == req.WorkspaceNameOrID &&
t.AgentNameOrID == req.AgentNameOrID &&
+48
View File
@@ -174,6 +174,54 @@ func Test_TokenMatchesRequest(t *testing.T) {
},
want: false,
},
{
name: "SamePrefix",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "dean-was--here---",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
token: workspaceapps.SignedToken{
Request: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "dean--was--here---",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "quux",
},
},
want: false,
},
{
name: "DifferentPrefix",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "yolo--",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
token: workspaceapps.SignedToken{
Request: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
Prefix: "swag--",
BasePath: "/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "quux",
},
},
want: false,
},
}
for _, c := range cases {