diff --git a/cli/resetpassword_test.go b/cli/resetpassword_test.go index 1beade14e9be9..508a9304d8ef3 100644 --- a/cli/resetpassword_test.go +++ b/cli/resetpassword_test.go @@ -41,7 +41,7 @@ func TestResetPassword(t *testing.T) { serverCmd, cfg := clitest.New(t, "server", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--postgres-url", connectionURL, "--cache-dir", t.TempDir(), ) diff --git a/cli/server.go b/cli/server.go index 60455f158d9be..f359f6fc7f297 100644 --- a/cli/server.go +++ b/cli/server.go @@ -225,7 +225,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co cfg.AccessURL.Value = tunnel.URL if cfg.WildcardAccessURL.Value == "" { - u, err := parseURL(ctx, tunnel.URL) + u, err := parseURL(tunnel.URL) if err != nil { return xerrors.Errorf("parse tunnel url: %w", err) } @@ -235,7 +235,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co } } - accessURLParsed, err := parseURL(ctx, cfg.AccessURL.Value) + accessURLParsed, err := parseURL(cfg.AccessURL.Value) if err != nil { return xerrors.Errorf("parse URL: %w", err) } @@ -469,7 +469,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co } // Parse the raw telemetry URL! - telemetryURL, err := parseURL(ctx, cfg.Telemetry.URL.Value) + telemetryURL, err := parseURL(cfg.Telemetry.URL.Value) if err != nil { return xerrors.Errorf("parse telemetry url: %w", err) } @@ -779,20 +779,14 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co return root } -// parseURL parses a string into a URL. It works around some technically correct -// but undesired behavior of url.Parse by prepending a scheme if one does not -// exist so that the URL does not get parsed improperly. -func parseURL(ctx context.Context, u string) (*url.URL, error) { +// parseURL parses a string into a URL. +func parseURL(u string) (*url.URL, error) { var ( hasScheme = strings.HasPrefix(u, "http:") || strings.HasPrefix(u, "https:") ) if !hasScheme { - // Append a scheme if it doesn't have one. Otherwise the hostname - // will likely get parsed as the scheme and cause methods like Hostname() - // to return an empty string, largely obviating the purpose of this - // function. - u = "https://" + u + return nil, xerrors.Errorf("URL %q must have a scheme of either http or https", u) } parsed, err := url.Parse(u) @@ -800,13 +794,6 @@ func parseURL(ctx context.Context, u string) (*url.URL, error) { return nil, err } - // If the specified url is a loopback device and no scheme has been - // specified, prefer http over https. It's unlikely anyone intends to use - // https on a loopback and if they do they can specify a scheme. - if local, _ := isLocalURL(ctx, parsed); local && !hasScheme { - parsed.Scheme = "http" - } - return parsed, nil } diff --git a/cli/server_test.go b/cli/server_test.go index d91e2e8889352..596730237a731 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -56,7 +56,7 @@ func TestServer(t *testing.T) { root, cfg := clitest.New(t, "server", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--postgres-url", connectionURL, "--cache-dir", t.TempDir(), ) @@ -91,7 +91,7 @@ func TestServer(t *testing.T) { root, cfg := clitest.New(t, "server", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--cache-dir", t.TempDir(), ) pty := ptytest.New(t) @@ -120,10 +120,9 @@ func TestServer(t *testing.T) { pty.ExpectMatch("psql") }) - // Validate that an http scheme is prepended to a loopback - // access URL and that a warning is printed that it may not be externally + // Validate that a warning is printed that it may not be externally // reachable. - t.Run("NoSchemeLocalAccessURL", func(t *testing.T) { + t.Run("LocalAccessURL", func(t *testing.T) { t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() @@ -132,7 +131,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "localhost:3000/", + "--access-url", "http://localhost:3000/", "--cache-dir", t.TempDir(), ) pty := ptytest.New(t) @@ -155,7 +154,7 @@ func TestServer(t *testing.T) { // Validate that an https scheme is prepended to a remote access URL // and that a warning is printed for a host that cannot be resolved. - t.Run("NoSchemeRemoteAccessURL", func(t *testing.T) { + t.Run("RemoteAccessURL", func(t *testing.T) { t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() @@ -164,8 +163,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", - "--access-url", "foobarbaz.mydomain", + "--access-url", "https://foobarbaz.mydomain", "--cache-dir", t.TempDir(), ) pty := ptytest.New(t) @@ -195,7 +193,6 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", "--access-url", "https://google.com", "--cache-dir", t.TempDir(), ) @@ -216,6 +213,22 @@ func TestServer(t *testing.T) { require.NoError(t, <-errC) }) + t.Run("NoSchemeAccessURL", func(t *testing.T) { + t.Parallel() + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + root, _ := clitest.New(t, + "server", + "--in-memory", + "--address", ":0", + "--access-url", "google.com", + "--cache-dir", t.TempDir(), + ) + err := root.ExecuteContext(ctx) + require.Error(t, err) + }) + t.Run("TLSBadVersion", func(t *testing.T) { t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) @@ -225,7 +238,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--tls-enable", "--tls-min-version", "tls9", "--cache-dir", t.TempDir(), @@ -242,7 +255,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--tls-enable", "--tls-client-auth", "something", "--cache-dir", t.TempDir(), @@ -299,7 +312,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--cache-dir", t.TempDir(), } args = append(args, c.args...) @@ -320,7 +333,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--tls-enable", "--tls-cert-file", certPath, "--tls-key-file", keyPath, @@ -360,7 +373,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--tls-enable", "--tls-cert-file", cert1Path, "--tls-key-file", key1Path, @@ -444,7 +457,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--provisioner-daemons", "1", "--cache-dir", t.TempDir(), ) @@ -471,7 +484,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--trace=true", "--cache-dir", t.TempDir(), ) @@ -509,7 +522,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--telemetry", "--telemetry-url", server.URL, "--cache-dir", t.TempDir(), @@ -540,7 +553,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--provisioner-daemons", "1", "--prometheus-enable", "--prometheus-address", ":"+strconv.Itoa(randomPort), @@ -593,7 +606,7 @@ func TestServer(t *testing.T) { "server", "--in-memory", "--address", ":0", - "--access-url", "example.com", + "--access-url", "http://example.com", "--oauth2-github-client-id", "fake", "--oauth2-github-client-secret", "fake", "--oauth2-github-enterprise-base-url", fakeRedirect, diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index e39c4126401a0..30d46b3835811 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -24,7 +24,7 @@ const config: PlaywrightTestConfig = { command: `go run -tags embed ${path.join( __dirname, "../../enterprise/cmd/coder/main.go", - )} server --in-memory --access-url 127.0.0.1:${basePort}`, + )} server --in-memory --access-url http://127.0.0.1:${basePort}`, port: basePort, timeout: 120 * 10000, reuseExistingServer: false,