diff --git a/cli/deployment/flags.go b/cli/deployment/flags.go index 3a03bea762b1c..df18e950270f2 100644 --- a/cli/deployment/flags.go +++ b/cli/deployment/flags.go @@ -32,7 +32,7 @@ func Flags() *codersdk.DeploymentFlags { Name: "Wildcard Address URL", Flag: "wildcard-access-url", EnvVar: "CODER_WILDCARD_ACCESS_URL", - Description: `Specifies the wildcard hostname to use for workspace applications in the form "*.example.com".`, + Description: `Specifies the wildcard hostname to use for workspace applications in the form "*.example.com" or "*-suffix.example.com". Ports or schemes should not be included. The scheme will be copied from the access URL.`, }, Address: &codersdk.StringFlag{ Name: "Bind Address", diff --git a/cli/server.go b/cli/server.go index e3cad09ca27ff..9d828abbea606 100644 --- a/cli/server.go +++ b/cli/server.go @@ -17,6 +17,7 @@ import ( "os/signal" "os/user" "path/filepath" + "regexp" "strconv" "strings" "sync" @@ -53,6 +54,7 @@ import ( "github.com/coder/coder/coderd/database/migrations" "github.com/coder/coder/coderd/devtunnel" "github.com/coder/coder/coderd/gitsshkey" + "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/prometheusmetrics" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/tracing" @@ -297,13 +299,19 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *code return xerrors.Errorf("create derp map: %w", err) } - appHostname := strings.TrimPrefix(dflags.WildcardAccessURL.Value, "http://") - appHostname = strings.TrimPrefix(appHostname, "https://") - appHostname = strings.TrimPrefix(appHostname, "*.") + appHostname := strings.TrimSpace(dflags.WildcardAccessURL.Value) + var appHostnameRegex *regexp.Regexp + if appHostname != "" { + appHostnameRegex, err = httpapi.CompileHostnamePattern(appHostname) + if err != nil { + return xerrors.Errorf("parse wildcard access URL %q: %w", appHostname, err) + } + } options := &coderd.Options{ AccessURL: accessURLParsed, AppHostname: appHostname, + AppHostnameRegex: appHostnameRegex, Logger: logger.Named("coderd"), Database: databasefake.New(), DERPMap: derpMap, diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index b12c8bc170a29..cd43b774d5dea 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -23,7 +23,15 @@ func TestWorkspaceActivityBump(t *testing.T) { setupActivityTest := func(t *testing.T) (client *codersdk.Client, workspace codersdk.Workspace, assertBumped func(want bool)) { var ttlMillis int64 = 60 * 1000 - client, _, workspace, _ = setupProxyTest(t, func(cwr *codersdk.CreateWorkspaceRequest) { + client = coderdtest.New(t, &coderdtest.Options{ + AppHostname: proxyTestSubdomainRaw, + IncludeProvisionerDaemon: true, + AgentStatsRefreshInterval: time.Millisecond * 100, + MetricsCacheRefreshInterval: time.Millisecond * 100, + }) + user := coderdtest.CreateFirstUser(t, client) + + workspace = createWorkspaceWithApps(t, client, user.OrganizationID, 1234, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.TTLMillis = &ttlMillis }) diff --git a/coderd/coderd.go b/coderd/coderd.go index 8b5080346c35b..992ae6c7f5ca5 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -6,6 +6,7 @@ import ( "net/http" "net/url" "path/filepath" + "regexp" "sync" "sync/atomic" "time" @@ -46,11 +47,16 @@ import ( type Options struct { AccessURL *url.URL // AppHostname should be the wildcard hostname to use for workspace - // applications without the asterisk or leading dot. E.g. "apps.coder.com". + // applications INCLUDING the asterisk, (optional) suffix and leading dot. + // E.g. "*.apps.coder.com" or "*-apps.coder.com". AppHostname string - Logger slog.Logger - Database database.Store - Pubsub database.Pubsub + // AppHostnameRegex contains the regex version of options.AppHostname as + // generated by httpapi.CompileHostnamePattern(). It MUST be set if + // options.AppHostname is set. + AppHostnameRegex *regexp.Regexp + Logger slog.Logger + Database database.Store + Pubsub database.Pubsub // CacheDir is used for caching files served by the API. CacheDir string @@ -90,6 +96,9 @@ func New(options *Options) *API { if options == nil { options = &Options{} } + if options.AppHostname != "" && options.AppHostnameRegex == nil || options.AppHostname == "" && options.AppHostnameRegex != nil { + panic("coderd: both AppHostname and AppHostnameRegex must be set or unset") + } if options.AgentConnectionUpdateFrequency == 0 { options.AgentConnectionUpdateFrequency = 3 * time.Second } diff --git a/coderd/coderdtest/authorize_test.go b/coderd/coderdtest/authorize_test.go index 4d8daa54c8e27..d4db546454d7d 100644 --- a/coderd/coderdtest/authorize_test.go +++ b/coderd/coderdtest/authorize_test.go @@ -11,7 +11,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { t.Parallel() client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ // Required for any subdomain-based proxy tests to pass. - AppHostname: "test.coder.com", + AppHostname: "*.test.coder.com", Authorizer: &coderdtest.RecordingAuthorizer{}, IncludeProvisionerDaemon: true, }) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 7ec28668cea91..f8695deb04df5 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -20,6 +20,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "regexp" "strconv" "strings" "testing" @@ -49,6 +50,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbtestutil" "github.com/coder/coder/coderd/gitsshkey" + "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/util/ptr" @@ -172,6 +174,13 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance options.SSHKeygenAlgorithm = gitsshkey.AlgorithmEd25519 } + var appHostnameRegex *regexp.Regexp + if options.AppHostname != "" { + var err error + appHostnameRegex, err = httpapi.CompileHostnamePattern(options.AppHostname) + require.NoError(t, err) + } + return srv, cancelFunc, &coderd.Options{ AgentConnectionUpdateFrequency: 150 * time.Millisecond, // Force a long disconnection timeout to ensure @@ -179,6 +188,7 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance AgentInactiveDisconnectTimeout: testutil.WaitShort, AccessURL: serverURL, AppHostname: options.AppHostname, + AppHostnameRegex: appHostnameRegex, Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), CacheDir: t.TempDir(), Database: db, diff --git a/coderd/httpapi/url.go b/coderd/httpapi/url.go index ac7b81fd4269b..2de7038c32ecf 100644 --- a/coderd/httpapi/url.go +++ b/coderd/httpapi/url.go @@ -17,20 +17,9 @@ var ( // {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" -// - "foo" becomes "foo", "" -func SplitSubdomain(hostname string) (subdomain string, rest string) { - toks := strings.SplitN(hostname, ".", 2) - if len(toks) < 2 { - return toks[0], "" - } - return toks[0], toks[1] -} + validHostnameLabelRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) +) // ApplicationURL is a parsed application URL hostname. type ApplicationURL struct { @@ -111,3 +100,81 @@ func HostnamesMatch(a, b string) bool { return strings.EqualFold(aHost, bHost) } + +// CompileHostnamePattern compiles a hostname pattern into a regular expression. +// A hostname pattern is a string that may contain a single wildcard character +// at the beginning. The wildcard character matches any number of hostname-safe +// characters excluding periods. The pattern is case-insensitive. +// +// The supplied pattern: +// - must not start or end with a period +// - must contain exactly one asterisk at the beginning +// - must not contain any other wildcard characters +// - must not contain any other characters that are not hostname-safe (including +// whitespace) +// - must contain at least two hostname labels/segments (i.e. "foo" or "*" are +// not valid patterns, but "foo.bar" and "*.bar" are). +// +// The returned regular expression will match an entire hostname with optional +// trailing periods and whitespace. The first submatch will be the wildcard +// match. +func CompileHostnamePattern(pattern string) (*regexp.Regexp, error) { + pattern = strings.ToLower(pattern) + if strings.Contains(pattern, "http:") || strings.Contains(pattern, "https:") { + return nil, xerrors.Errorf("hostname pattern must not contain a scheme: %q", pattern) + } + if strings.Contains(pattern, ":") { + return nil, xerrors.Errorf("hostname pattern must not contain a port: %q", pattern) + } + if strings.HasPrefix(pattern, ".") || strings.HasSuffix(pattern, ".") { + return nil, xerrors.Errorf("hostname pattern must not start or end with a period: %q", pattern) + } + if strings.Count(pattern, ".") < 1 { + return nil, xerrors.Errorf("hostname pattern must contain at least two labels/segments: %q", pattern) + } + if strings.Count(pattern, "*") != 1 { + return nil, xerrors.Errorf("hostname pattern must contain exactly one asterisk: %q", pattern) + } + if !strings.HasPrefix(pattern, "*") { + return nil, xerrors.Errorf("hostname pattern must only contain an asterisk at the beginning: %q", pattern) + } + for i, label := range strings.Split(pattern, ".") { + if i == 0 { + // We have to allow the asterisk to be a valid hostname label. + label = strings.TrimPrefix(label, "*") + label = "a" + label + } + if !validHostnameLabelRegex.MatchString(label) { + return nil, xerrors.Errorf("hostname pattern contains invalid label %q: %q", label, pattern) + } + } + + // Replace periods with escaped periods. + regexPattern := strings.ReplaceAll(pattern, ".", "\\.") + + // Capture wildcard match. + regexPattern = strings.Replace(regexPattern, "*", "([^.]+)", 1) + + // Allow trailing period. + regexPattern = regexPattern + "\\.?" + + // Allow optional port number. + regexPattern += "(:\\d+)?" + + // Allow leading and trailing whitespace. + regexPattern = `^\s*` + regexPattern + `\s*$` + + return regexp.Compile(regexPattern) +} + +// ExecuteHostnamePattern executes a pattern generated by CompileHostnamePattern +// and returns the wildcard match. If the pattern does not match the hostname, +// returns false. +func ExecuteHostnamePattern(pattern *regexp.Regexp, hostname string) (string, bool) { + matches := pattern.FindStringSubmatch(hostname) + if len(matches) < 2 { + return "", false + } + + return matches[1], true +} diff --git a/coderd/httpapi/url_test.go b/coderd/httpapi/url_test.go index 91d232ece2301..2843c5efdd15f 100644 --- a/coderd/httpapi/url_test.go +++ b/coderd/httpapi/url_test.go @@ -1,6 +1,7 @@ package httpapi_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -8,64 +9,6 @@ import ( "github.com/coder/coder/coderd/httpapi" ) -func TestSplitSubdomain(t *testing.T) { - t.Parallel() - testCases := []struct { - Name string - Host string - ExpectedSubdomain string - ExpectedRest string - }{ - { - Name: "Empty", - Host: "", - ExpectedSubdomain: "", - ExpectedRest: "", - }, - { - Name: "NoSubdomain", - Host: "com", - ExpectedSubdomain: "com", - ExpectedRest: "", - }, - { - Name: "Domain", - Host: "coder.com", - ExpectedSubdomain: "coder", - ExpectedRest: "com", - }, - { - Name: "Subdomain", - Host: "subdomain.coder.com", - ExpectedSubdomain: "subdomain", - ExpectedRest: "coder.com", - }, - { - Name: "DoubleSubdomain", - Host: "subdomain1.subdomain2.coder.com", - ExpectedSubdomain: "subdomain1", - ExpectedRest: "subdomain2.coder.com", - }, - { - Name: "WithPort", - Host: "subdomain.coder.com:8080", - ExpectedSubdomain: "subdomain", - ExpectedRest: "coder.com:8080", - }, - } - - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() - - subdomain, rest := httpapi.SplitSubdomain(c.Host) - require.Equal(t, c.ExpectedSubdomain, subdomain) - require.Equal(t, c.ExpectedRest, rest) - }) - } -} - func TestApplicationURLString(t *testing.T) { t.Parallel() @@ -214,3 +157,239 @@ func TestParseSubdomainAppURL(t *testing.T) { }) } } + +func TestCompileHostnamePattern(t *testing.T) { + t.Parallel() + + type matchCase struct { + input string + // empty string denotes no match + match string + } + + type testCase struct { + name string + pattern string + errorContains string + // expectedRegex only needs to contain the inner part of the regex, not + // the prefix and suffix checks. + expectedRegex string + matchCases []matchCase + } + + testCases := []testCase{ + { + name: "Invalid_ContainsHTTP", + pattern: "http://*.hi.com", + errorContains: "must not contain a scheme", + }, + { + name: "Invalid_ContainsHTTPS", + pattern: "https://*.hi.com", + errorContains: "must not contain a scheme", + }, + { + name: "Invalid_ContainsPort", + pattern: "*.hi.com:8080", + errorContains: "must not contain a port", + }, + { + name: "Invalid_StartPeriod", + pattern: ".hi.com", + errorContains: "must not start or end with a period", + }, + { + name: "Invalid_EndPeriod", + pattern: "hi.com.", + errorContains: "must not start or end with a period", + }, + { + name: "Invalid_Empty", + pattern: "", + errorContains: "must contain at least two labels", + }, + { + name: "Invalid_SingleLabel", + pattern: "hi", + errorContains: "must contain at least two labels", + }, + { + name: "Invalid_NoWildcard", + pattern: "hi.com", + errorContains: "must contain exactly one asterisk", + }, + { + name: "Invalid_MultipleWildcards", + pattern: "**.hi.com", + errorContains: "must contain exactly one asterisk", + }, + { + name: "Invalid_WildcardNotFirst", + pattern: "hi.*.com", + errorContains: "must only contain an asterisk at the beginning", + }, + { + name: "Invalid_BadLabel1", + pattern: "*.h_i.com", + errorContains: "contains invalid label", + }, + { + name: "Invalid_BadLabel2", + pattern: "*.hi-.com", + errorContains: "contains invalid label", + }, + { + name: "Invalid_BadLabel3", + pattern: "*.-hi.com", + errorContains: "contains invalid label", + }, + + { + name: "Valid_Simple", + pattern: "*.hi", + expectedRegex: `([^.]+)\.hi`, + matchCases: []matchCase{ + { + input: "hi", + match: "", + }, + { + input: "hi.com", + match: "", + }, + { + input: "hi.hi.hi", + match: "", + }, + { + input: "abcd.hi", + match: "abcd", + }, + { + input: "abcd.hi.", + match: "abcd", + }, + { + input: " abcd.hi. ", + match: "abcd", + }, + { + input: "abcd.hi:8080", + match: "abcd", + }, + { + input: "ab__invalid__cd-.hi", + // Invalid subdomains still match the pattern because they + // managed to make it to the webserver anyways. + match: "ab__invalid__cd-", + }, + }, + }, + { + name: "Valid_MultiLevel", + pattern: "*.hi.com", + expectedRegex: `([^.]+)\.hi\.com`, + matchCases: []matchCase{ + { + input: "hi.com", + match: "", + }, + { + input: "abcd.hi.com", + match: "abcd", + }, + { + input: "ab__invalid__cd-.hi.com", + match: "ab__invalid__cd-", + }, + }, + }, + { + name: "Valid_WildcardSuffix1", + pattern: `*a.hi.com`, + expectedRegex: `([^.]+)a\.hi\.com`, + matchCases: []matchCase{ + { + input: "hi.com", + match: "", + }, + { + input: "abcd.hi.com", + match: "", + }, + { + input: "ab__invalid__cd-.hi.com", + match: "", + }, + { + input: "abcda.hi.com", + match: "abcd", + }, + { + input: "ab__invalid__cd-a.hi.com", + match: "ab__invalid__cd-", + }, + }, + }, + { + name: "Valid_WildcardSuffix2", + pattern: `*-test.hi.com`, + expectedRegex: `([^.]+)-test\.hi\.com`, + matchCases: []matchCase{ + { + input: "hi.com", + match: "", + }, + { + input: "abcd.hi.com", + match: "", + }, + { + input: "ab__invalid__cd-.hi.com", + match: "", + }, + { + input: "abcd-test.hi.com", + match: "abcd", + }, + { + input: "ab__invalid__cd-test.hi.com", + match: "ab__invalid__cd", + }, + }, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + regex, err := httpapi.CompileHostnamePattern(c.pattern) + if c.errorContains == "" { + require.NoError(t, err) + + expected := `^\s*` + c.expectedRegex + `\.?(:\d+)?\s*$` + require.Equal(t, expected, regex.String(), "generated regex does not match") + + for i, m := range c.matchCases { + m := m + t.Run(fmt.Sprintf("MatchCase%d", i), func(t *testing.T) { + t.Parallel() + + match, ok := httpapi.ExecuteHostnamePattern(regex, m.input) + if m.match == "" { + require.False(t, ok) + } else { + require.True(t, ok) + require.Equal(t, m.match, match) + } + }) + } + } else { + require.Error(t, err) + require.ErrorContains(t, err, c.errorContains) + } + }) + } +} diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 55f73d2c52ae1..5a7c192602fb5 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -139,7 +139,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht // Step 1: Pass on if subdomain-based application proxying is not // configured. - if api.AppHostname == "" { + if api.AppHostname == "" || api.AppHostnameRegex == nil { next.ServeHTTP(rw, r) return } @@ -219,28 +219,25 @@ func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *htt return httpapi.ApplicationURL{}, false } + // If there are no periods in the hostname, then it can't be a valid + // application URL. + if !strings.Contains(host, ".") { + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + // Split the subdomain so we can parse the application details and verify it // matches the configured app hostname later. - subdomain, rest := httpapi.SplitSubdomain(host) - if rest == "" { - // If there are no periods in the hostname, then it can't be a valid - // application URL. + subdomain, ok := httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, host) + if !ok { + // Doesn't match the regex, so it's not a valid application URL. next.ServeHTTP(rw, r) return httpapi.ApplicationURL{}, false } - matchingBaseHostname := httpapi.HostnamesMatch(api.AppHostname, rest) // Parse the application URL from the subdomain. app, err := httpapi.ParseSubdomainAppURL(subdomain) if err != nil { - // If it isn't a valid app URL and the base domain doesn't match the - // configured app hostname, this request was probably destined for the - // dashboard/API router. - if !matchingBaseHostname { - next.ServeHTTP(rw, r) - return httpapi.ApplicationURL{}, false - } - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ Status: http.StatusBadRequest, Title: "Invalid application URL", @@ -251,20 +248,6 @@ func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *htt return httpapi.ApplicationURL{}, false } - // At this point we've verified that the subdomain looks like a valid - // application URL, so the base hostname should match the configured app - // hostname. - if !matchingBaseHostname { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusNotFound, - Title: "Not Found", - Description: "The server does not accept application requests on this hostname.", - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return httpapi.ApplicationURL{}, false - } - return app, true } @@ -506,8 +489,8 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request // Ensure that the redirect URI is a subdomain of api.AppHostname and is a // valid app subdomain. - subdomain, rest := httpapi.SplitSubdomain(u.Hostname()) - if !httpapi.HostnamesMatch(api.AppHostname, rest) { + subdomain, ok := httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, u.Host) + if !ok { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "The redirect_uri query parameter must be a valid app subdomain.", }) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index ed2f536ef2c68..bbf80746d7987 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -35,15 +35,16 @@ const ( proxyTestAppNameAuthenticated = "test-app-authenticated" proxyTestAppNamePublic = "test-app-public" proxyTestAppQuery = "query=true" - proxyTestAppBody = "hello world" + proxyTestAppBody = "hello world from apps test" - proxyTestSubdomain = "test.coder.com" + proxyTestSubdomainRaw = "*.test.coder.com" + proxyTestSubdomain = "test.coder.com" ) func TestGetAppHost(t *testing.T) { t.Parallel() - cases := []string{"", "test.coder.com"} + cases := []string{"", proxyTestSubdomainRaw} for _, c := range cases { c := c name := c @@ -75,7 +76,7 @@ func TestGetAppHost(t *testing.T) { // setupProxyTest creates a workspace with an agent and some apps. It returns a // codersdk client, the first user, the workspace, and the port number the test // listener is running on. -func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) (*codersdk.Client, codersdk.CreateFirstUserResponse, codersdk.Workspace, uint16) { +func setupProxyTest(t *testing.T, customAppHost ...string) (*codersdk.Client, codersdk.CreateFirstUserResponse, codersdk.Workspace, uint16) { // #nosec ln, err := net.Listen("tcp", ":0") require.NoError(t, err) @@ -96,17 +97,42 @@ func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWork tcpAddr, ok := ln.Addr().(*net.TCPAddr) require.True(t, ok) + appHost := proxyTestSubdomainRaw + if len(customAppHost) > 0 { + appHost = customAppHost[0] + } + client := coderdtest.New(t, &coderdtest.Options{ - AppHostname: proxyTestSubdomain, + AppHostname: appHost, IncludeProvisionerDaemon: true, AgentStatsRefreshInterval: time.Millisecond * 100, MetricsCacheRefreshInterval: time.Millisecond * 100, }) user := coderdtest.CreateFirstUser(t, client) + + workspace := createWorkspaceWithApps(t, client, user.OrganizationID, uint16(tcpAddr.Port)) + + // 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, workspace, uint16(tcpAddr.Port) +} + +func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.UUID, port uint16, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { authToken := uuid.NewString() - appURL := fmt.Sprintf("http://127.0.0.1:%d?%s", tcpAddr.Port, proxyTestAppQuery) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + appURL := fmt.Sprintf("http://127.0.0.1:%d?%s", port, proxyTestAppQuery) + version := coderdtest.CreateTemplateVersion(t, client, orgID, &echo.Responses{ Parse: echo.ParseComplete, ProvisionDryRun: echo.ProvisionComplete, Provision: []*proto.Provision_Response{{ @@ -150,9 +176,9 @@ func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWork }, }}, }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + template := coderdtest.CreateTemplate(t, client, orgID, version.ID) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, workspaceMutators...) + workspace := coderdtest.CreateWorkspace(t, client, orgID, template.ID, workspaceMutators...) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) agentClient := codersdk.New(client.URL) @@ -168,20 +194,7 @@ func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWork }) coderdtest.AwaitWorkspaceAgents(t, client, workspace.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, workspace, uint16(tcpAddr.Port) + return workspace } func TestWorkspaceAppsProxyPath(t *testing.T) { @@ -528,28 +541,9 @@ func TestWorkspaceAppsProxySubdomainBlocked(t *testing.T) { return client } - t.Run("NotMatchingHostname", func(t *testing.T) { - t.Parallel() - client := setup(t, "test."+proxyTestSubdomain) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - uri := fmt.Sprintf("http://app--agent--workspace--username.%s/api/v2/users/me", proxyTestSubdomain) - resp, err := client.Request(ctx, http.MethodGet, uri, nil) - require.NoError(t, err) - defer resp.Body.Close() - - // Should have an error response. - require.Equal(t, http.StatusNotFound, resp.StatusCode) - body, err := io.ReadAll(resp.Body) - require.NoError(t, err) - require.Contains(t, string(body), "does not accept application requests on this hostname") - }) - t.Run("InvalidSubdomain", func(t *testing.T) { t.Parallel() - client := setup(t, proxyTestSubdomain) + client := setup(t, proxyTestSubdomainRaw) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() @@ -569,11 +563,11 @@ func TestWorkspaceAppsProxySubdomainBlocked(t *testing.T) { func TestWorkspaceAppsProxySubdomain(t *testing.T) { t.Parallel() - client, firstUser, workspace, port := setupProxyTest(t) + client, firstUser, _, 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 { + proxyURL := func(t *testing.T, client *codersdk.Client, appNameOrPort interface{}, pathAndQuery ...string) string { t.Helper() var ( @@ -587,16 +581,30 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { require.True(t, ok) } - me, err := client.User(context.Background(), codersdk.Me) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + me, err := client.User(ctx, codersdk.Me) require.NoError(t, err, "get current user details") - hostname := httpapi.ApplicationURL{ + workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Owner: codersdk.Me, + }) + require.NoError(t, err, "get workspaces") + require.Len(t, workspaces, 1, "expected 1 workspace") + + appHost, err := client.GetAppHost(ctx) + require.NoError(t, err, "get app host") + + subdomain := httpapi.ApplicationURL{ AppName: appName, Port: port, AgentName: proxyTestAgentName, - WorkspaceName: workspace.Name, + WorkspaceName: workspaces[0].Name, Username: me.Username, - }.String() + "." + proxyTestSubdomain + }.String() + + hostname := strings.Replace(appHost.Host, "*", subdomain, 1) actualPath := "/" query := "" @@ -625,7 +633,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := userClient.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppNameOwner), nil) + resp, err := userClient.Request(ctx, http.MethodGet, proxyURL(t, client, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusNotFound, resp.StatusCode) @@ -637,7 +645,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - slashlessURL := proxyURL(t, proxyTestAppNameOwner, "") + slashlessURL := proxyURL(t, client, proxyTestAppNameOwner, "") resp, err := client.Request(ctx, http.MethodGet, slashlessURL, nil) require.NoError(t, err) defer resp.Body.Close() @@ -654,7 +662,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - querylessURL := proxyURL(t, proxyTestAppNameOwner, "/", "") + querylessURL := proxyURL(t, client, proxyTestAppNameOwner, "/", "") resp, err := client.Request(ctx, http.MethodGet, querylessURL, nil) require.NoError(t, err) defer resp.Body.Close() @@ -671,7 +679,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppNameOwner, "/", proxyTestAppQuery), nil) + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery), nil) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -686,7 +694,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, port, "/", proxyTestAppQuery), nil) + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, client, port, "/", proxyTestAppQuery), nil) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -701,7 +709,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppNameFake, "/", ""), nil) + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, client, proxyTestAppNameFake, "/", ""), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusBadGateway, resp.StatusCode) @@ -714,7 +722,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { defer cancel() port := uint16(codersdk.MinimumListeningPort - 1) - resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, port, "/", proxyTestAppQuery), nil) + resp, err := client.Request(ctx, http.MethodGet, proxyURL(t, client, port, "/", proxyTestAppQuery), nil) require.NoError(t, err) defer resp.Body.Close() @@ -725,6 +733,72 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { require.NoError(t, err) require.Contains(t, resBody.Message, "Coder reserves ports less than") }) + + t.Run("SuffixWildcardOK", func(t *testing.T) { + t.Parallel() + + client, _, _, _ := setupProxyTest(t, "*-suffix.test.coder.com") + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + u := proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery) + t.Logf("url: %s", u) + + resp, err := client.Request(ctx, http.MethodGet, u, 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("SuffixWildcardNotMatch", func(t *testing.T) { + t.Parallel() + + client, _, _, _ := setupProxyTest(t, "*-suffix.test.coder.com") + + t.Run("NoSuffix", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + u := proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery) + // Replace the -suffix with nothing. + u = strings.Replace(u, "-suffix", "", 1) + + resp, err := client.Request(ctx, http.MethodGet, u, nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + // It's probably rendering the dashboard, so only ensure that the body + // doesn't match. + require.NotContains(t, string(body), proxyTestAppBody) + }) + + t.Run("DifferentSuffix", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + u := proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery) + // Replace the -suffix with something else. + u = strings.Replace(u, "-suffix", "-not-suffix", 1) + + resp, err := client.Request(ctx, http.MethodGet, u, nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + // It's probably rendering the dashboard, so only ensure that the body + // doesn't match. + require.NotContains(t, string(body), proxyTestAppBody) + }) + }) } func TestAppSharing(t *testing.T) { diff --git a/enterprise/coderd/coderdenttest/coderdenttest_test.go b/enterprise/coderd/coderdenttest/coderdenttest_test.go index c21a412617a85..d526f6927bc00 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest_test.go +++ b/enterprise/coderd/coderdenttest/coderdenttest_test.go @@ -25,7 +25,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { client, _, api := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ // Required for any subdomain-based proxy tests to pass. - AppHostname: "test.coder.com", + AppHostname: "*.test.coder.com", Authorizer: &coderdtest.RecordingAuthorizer{}, IncludeProvisionerDaemon: true, }, diff --git a/helm/Makefile b/helm/Makefile index 2d88f4ff09b43..a3f689b1637af 100644 --- a/helm/Makefile +++ b/helm/Makefile @@ -14,5 +14,5 @@ lint: lint/helm .PHONY: lint lint/helm: - helm lint --strict . + helm lint --strict --set coder.image.tag=v0.0.1 . .PHONY: lint/helm diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl index 95ba09a88692c..b0b04baacbe20 100644 --- a/helm/templates/_helpers.tpl +++ b/helm/templates/_helpers.tpl @@ -138,6 +138,24 @@ Coder TLS environment variables. {{- end }} {{- end }} +{{/* +Coder ingress wildcard hostname with the wildcard suffix stripped. +*/}} +{{- define "coder.ingressWildcardHost" -}} +{{/* This regex replace is required as the original input including the suffix + * is not a legal ingress host. We need to remove the suffix and keep the + * wildcard '*'. + * + * - '\\*' Starts with '*' + * - '[^.]*' Suffix is 0 or more characters, '-suffix' + * - '(' Start domain capture group + * - '\\.' The domain should be separated with a '.' from the subdomain + * - '.*' Rest of the domain. + * - ')' $1 is the ''.example.com' + */}} +{{- regexReplaceAll "\\*[^.]*(\\..*)" .Values.coder.ingress.wildcardHost "*${1}" -}} +{{- end }} + {{/* Fail on fully deprecated values or deprecated value combinations. This is included at the top of coder.yaml. diff --git a/helm/templates/ingress.yaml b/helm/templates/ingress.yaml index 594bda6747873..4644ae836c20a 100644 --- a/helm/templates/ingress.yaml +++ b/helm/templates/ingress.yaml @@ -26,8 +26,9 @@ spec: name: coder port: name: {{ include "coder.portName" . | quote }} + {{- if .Values.coder.ingress.wildcardHost }} - - host: {{ .Values.coder.ingress.wildcardHost | quote }} + - host: {{ include "coder.ingressWildcardHost" . | quote }} http: paths: - path: / @@ -46,7 +47,7 @@ spec: secretName: {{ .Values.coder.ingress.tls.secretName | quote}} {{- if .Values.coder.ingress.tls.wildcardSecretName }} - hosts: - - {{ .Values.coder.ingress.wildcardHost | quote }} + - {{ include "coder.ingressWildcardHost" . | quote }} secretName: {{ .Values.coder.ingress.tls.wildcardSecretName | quote}} {{- end }} {{- end }} diff --git a/helm/values.yaml b/helm/values.yaml index cfba214ee6028..30a21a8985d23 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -101,8 +101,10 @@ coder: # coder.ingress.host -- The hostname to match on. host: "" # coder.ingress.wildcardHost -- The wildcard hostname to match on. Should be - # in the form "*.example.com". Optional if not using applications over - # subdomains. + # in the form "*.example.com" or "*-suffix.example.com". If you are using a + # suffix after the wildcard, the suffix will be stripped from the created + # ingress to ensure that it is a legal ingress host. Optional if not using + # applications over subdomains. wildcardHost: "" # coder.ingress.annotations -- The ingress annotations. annotations: {} diff --git a/site/src/components/AppLink/AppLink.tsx b/site/src/components/AppLink/AppLink.tsx index 61a413d61b17b..10ffaf23e9a5a 100644 --- a/site/src/components/AppLink/AppLink.tsx +++ b/site/src/components/AppLink/AppLink.tsx @@ -51,7 +51,7 @@ export const AppLink: FC> = ({ } if (appsHost && appSubdomain) { const subdomain = `${appName}--${agentName}--${workspaceName}--${username}` - href = `${window.location.protocol}//${subdomain}.${appsHost}/` + href = `${window.location.protocol}//${appsHost}/`.replace("*", subdomain) } let canClick = true diff --git a/site/src/components/PortForwardButton/PortForwardButton.tsx b/site/src/components/PortForwardButton/PortForwardButton.tsx index eef1d65b27d0d..4662c2859726b 100644 --- a/site/src/components/PortForwardButton/PortForwardButton.tsx +++ b/site/src/components/PortForwardButton/PortForwardButton.tsx @@ -27,12 +27,33 @@ export interface PortForwardButtonProps { agentId: string } +const portForwardURL = ( + host: string, + port: number, + agentName: string, + workspaceName: string, + username: string, +): string => { + const { location } = window + + const subdomain = `${ + isNaN(port) ? 3000 : port + }--${agentName}--${workspaceName}--${username}` + return `${location.protocol}//${host}`.replace("*", subdomain) +} + const EnabledView: React.FC = (props) => { const { host, workspaceName, agentName, agentId, username } = props const styles = useStyles() const [port, setPort] = useState("3000") - const { location } = window - const urlExample = `${location.protocol}//${port}--${agentName}--${workspaceName}--${username}.${host}` + const urlExample = portForwardURL( + host, + parseInt(port), + agentName, + workspaceName, + username, + ) + const [state] = useMachine(portForwardMachine, { context: { agentId: agentId }, }) @@ -82,7 +103,13 @@ const EnabledView: React.FC = (props) => { {ports && ports.map((p, i) => { - const url = `${location.protocol}//${p.port}--${agentName}--${workspaceName}--${username}.${host}` + const url = portForwardURL( + host, + p.port, + agentName, + workspaceName, + username, + ) let label = `${p.port}` if (p.process_name) { label = `${p.process_name} - ${p.port}` diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index fd6903cad88a7..d881449cbcb64 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -217,7 +217,7 @@ export const handlers = [ // Applications host rest.get("/api/v2/applications/host", (req, res, ctx) => { - return res(ctx.status(200), ctx.json({ host: "dev.coder.com" })) + return res(ctx.status(200), ctx.json({ host: "*.dev.coder.com" })) }), // Groups