fix: error if protocol isn't specified in --access-url (#4835)

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(),
 	)

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
 }
 

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,

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,