Reject an empty host at the login host prompt

.nextchanges/cli/login-reject-empty-host.md

@@ -0,0 +1 @@
+* `databricks auth login` and `databricks auth token` now reject an empty host instead of falling back to `DATABRICKS_HOST`. An empty host after "Enter a host" in the login profile picker no longer starts discovery login. ([#7195](https://github.com/databricks/cli/pull/7195))

acceptance/cmd/auth/login/invalid-host/out.test.toml

@@ -0,0 +1 @@
+Cloud = false

acceptance/cmd/auth/login/invalid-host/output.txt

@@ -0,0 +1,3 @@
+
+>>> musterr [CLI] auth login --host ?o=123 --profile test
+Error: invalid host "?o=123"

acceptance/cmd/auth/login/invalid-host/script

@@ -0,0 +1,5 @@
+sethome "./home"
+
+# A host that is empty without its query params is rejected before login
+# can fall back to discovery or DATABRICKS_HOST.
+trace musterr $CLI auth login --host '?o=123' --profile test

acceptance/cmd/auth/login/invalid-host/test.toml

@@ -0,0 +1,3 @@
+Ignore = [
+    "home"
+]

acceptance/cmd/auth/token/invalid-host/out.test.toml

@@ -0,0 +1 @@
+Cloud = false

acceptance/cmd/auth/token/invalid-host/output.txt

@@ -0,0 +1,3 @@
+
+>>> musterr [CLI] auth token --host ?o=123
+Error: invalid host "?o=123"

acceptance/cmd/auth/token/invalid-host/script

@@ -0,0 +1,9 @@
+sethome "./home"
+
+unset DATABRICKS_HOST
+unset DATABRICKS_TOKEN
+unset DATABRICKS_CONFIG_PROFILE
+
+# A host that is empty without its query params is rejected instead of
+# falling back to DATABRICKS_HOST.
+trace musterr $CLI auth token --host '?o=123'

acceptance/cmd/auth/token/invalid-host/test.toml

@@ -0,0 +1,3 @@
+Ignore = [
+    "home"
+]

cmd/auth/auth.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/databricks/cli/libs/auth"
 	"github.com/databricks/cli/libs/cmdio"
@@ -51,9 +52,18 @@ func promptForHost(ctx context.Context) (string, error) {
 	// long "label + value" line that wraps causes each keystroke to leave a
 	// stale render on screen instead of overwriting the previous one.
 	cmdio.LogString(ctx, "Example: https://<databricks-instance>.cloud.databricks.com")
-	return cmdio.RunPrompt(ctx, cmdio.PromptOptions{
+	result, err := cmdio.RunPrompt(ctx, cmdio.PromptOptions{
 		Label: "Databricks host",
+		Validate: func(input string) error {
+			if strings.TrimSpace(input) == "" {
+				return errors.New("host is required")
+			}
+			// Reject hosts that setHost would reject after the prompt,
+			// e.g. "/" or "?o=123", while the user can still edit them.
+			return normalizeLoginHost(&auth.AuthArguments{Host: input})
+		},
 	})
+	return strings.TrimSpace(result), err
 }
 
 func promptForAccountID(ctx context.Context) (string, error) {

cmd/auth/auth_test.go

@@ -3,10 +3,12 @@ package auth
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/cmdctx"
+	"github.com/databricks/cli/libs/cmdio/cmdiotest/termtest"
 	"github.com/databricks/cli/libs/databrickscfg/profile"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -141,3 +143,27 @@ func TestProfileHostCompatibleViaCobra(t *testing.T) {
 		assert.NotContains(t, err.Error(), "conflicts with --host")
 	}
 }
+
+func TestPromptForHostRejectsEmptyHost(t *testing.T) {
+	tm := termtest.Run(t, promptForHost)
+	tm.WaitFor("Databricks host")
+	tm.Type(termtest.KeyEnter)
+	tm.Golden("01-empty")
+	tm.Type("  " + termtest.KeyEnter)
+	tm.Golden("02-spaces")
+	tm.Type("https://workspace.test " + termtest.KeyEnter)
+	host, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "https://workspace.test", host)
+}
+
+func TestPromptForHostRejectsHostEmptyAfterNormalization(t *testing.T) {
+	tm := termtest.Run(t, promptForHost)
+	tm.WaitFor("Databricks host")
+	tm.Type("?o=123" + termtest.KeyEnter)
+	tm.Golden("01-query-params")
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("?o=123")) + "https://workspace.test/?o=123" + termtest.KeyEnter)
+	host, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "https://workspace.test/?o=123", host)
+}

cmd/auth/login.go

@@ -286,11 +286,6 @@ a new profile is created.
 				if err != nil {
 					return err
 				}
-				// Fail here rather than prompting for the host again after
-				// the profile name.
-				if useDeviceCode && host == "" {
-					return errDeviceCodeNeedsHost
-				}
 				authArguments.Host = host
 			case profilePickerCreateNew:
 				// Reject the names of existing profiles, including those the
@@ -310,13 +305,8 @@ a new profile is created.
 		// only populated by setHostAndAccountId further down, which runs after
 		// the profile-name prompt.
 		if skipWorkspace && profileName == "" && authArguments.Host != "" && authArguments.AccountID == "" {
-			params := auth.ExtractHostQueryParams(authArguments.Host)
-			authArguments.Host = params.Host
-			if authArguments.AccountID == "" {
-				authArguments.AccountID = params.AccountID
-			}
-			if authArguments.WorkspaceID == "" {
-				authArguments.WorkspaceID = params.WorkspaceID
+			if err := normalizeLoginHost(authArguments); err != nil {
+				return err
 			}
 			runHostDiscovery(ctx, authArguments)
 		}
@@ -379,15 +369,8 @@ a new profile is created.
 		// Check the host once it is resolved, before setAccountID prompts for
 		// an account ID for account and unified hosts. A workspace without
 		// device code login enabled is rejected later by deviceCodeLogin.
-		if useDeviceCode {
-			// The host prompt accepts an empty answer. Without this check,
-			// login would resolve the host from DATABRICKS_HOST instead.
-			if authArguments.Host == "" {
-				return errDeviceCodeNeedsHost
-			}
-			if !isDeviceCodeSupportedForHost(authArguments.Host, authArguments.DiscoveryURL) {
-				return errDeviceCodeWorkspaceOnly
-			}
+		if useDeviceCode && !isDeviceCodeSupportedForHost(authArguments.Host, authArguments.DiscoveryURL) {
+			return errDeviceCodeWorkspaceOnly
 		}
 		err = setAccountID(ctx, existingProfile, authArguments)
 		if err != nil {
@@ -577,6 +560,33 @@ func setHostAndAccountId(ctx context.Context, existingProfile *profile.Profile,
 	return setAccountID(ctx, existingProfile, authArguments)
 }
 
+var errInvalidHost = errors.New("invalid host")
+
+// normalizeLoginHost strips the trailing slash and the query params from the
+// host and moves the workspace and account ID query params into WorkspaceID and
+// AccountID unless they are set.
+// It fails if nothing is left of the host, e.g. for "/" or "?o=123", because
+// with an empty host login would use discovery or DATABRICKS_HOST instead.
+func normalizeLoginHost(authArguments *auth.AuthArguments) error {
+	rawHost := authArguments.Host
+	authArguments.Host = strings.TrimSuffix(strings.TrimSpace(authArguments.Host), "/")
+
+	// Extract query parameters from the host URL (?o=workspace_id, ?a=account_id).
+	// URL params from explicit --host override stale profile values.
+	params := auth.ExtractHostQueryParams(authArguments.Host)
+	authArguments.Host = params.Host
+	if authArguments.Host == "" {
+		return fmt.Errorf("%w %q", errInvalidHost, rawHost)
+	}
+	if authArguments.WorkspaceID == "" {
+		authArguments.WorkspaceID = params.WorkspaceID
+	}
+	if authArguments.AccountID == "" {
+		authArguments.AccountID = params.AccountID
+	}
+	return nil
+}
+
 // Sets the host in the persistentAuth object based on the provided arguments and flags.
 // Follows the following precedence:
 // 1. [HOST] (first positional argument) or --host flag. Error if both are specified.
@@ -608,17 +618,8 @@ func setHost(ctx context.Context, existingProfile *profile.Profile, authArgument
 		}
 	}
 
-	authArguments.Host = strings.TrimSuffix(authArguments.Host, "/")
-
-	// Extract query parameters from the host URL (?o=workspace_id, ?a=account_id).
-	// URL params from explicit --host override stale profile values.
-	params := auth.ExtractHostQueryParams(authArguments.Host)
-	authArguments.Host = params.Host
-	if authArguments.WorkspaceID == "" {
-		authArguments.WorkspaceID = params.WorkspaceID
-	}
-	if authArguments.AccountID == "" {
-		authArguments.AccountID = params.AccountID
+	if err := normalizeLoginHost(authArguments); err != nil {
+		return err
 	}
 
 	// Inherit workspace_id from the existing profile AFTER URL param extraction.

cmd/auth/login_device_code.go

@@ -27,10 +27,7 @@ var deviceCodeIncompatibleFlags = []string{
 	"skip-workspace",
 }
 
-var (
-	errDeviceCodeNeedsHost     = errors.New("--use-device-code requires a workspace host. Pass it as an argument or with --host")
-	errDeviceCodeWorkspaceOnly = errors.New("--use-device-code is only supported for workspace hosts")
-)
+var errDeviceCodeWorkspaceOnly = errors.New("--use-device-code is only supported for workspace hosts")
 
 // isDeviceCodeSupportedForHost reports whether host is a workspace host. A
 // unified host is only recognized by its account-scoped discoveryURL, so with

cmd/auth/login_device_code_test.go

@@ -221,8 +221,10 @@ func startLogin(t *testing.T, config string, args ...string) *termtest.Term[stru
 	t.Setenv("DATABRICKS_AUTH_STORAGE", "plaintext")
 
 	return termtest.Run(t, func(ctx context.Context) (struct{}, error) {
-		cmd := newLoginCommand(&auth.AuthArguments{})
+		authArguments := &auth.AuthArguments{}
+		cmd := newLoginCommand(authArguments)
 		cmd.Flags().String("profile", "", "")
+		cmd.Flags().StringVar(&authArguments.Host, "host", "", "")
 		cmd.SetContext(ctx)
 		// Cobra reads os.Args when the args are nil.
 		cmd.SetArgs(append([]string{}, args...))
@@ -243,8 +245,10 @@ func TestLoginDeviceCodeNewProfilePromptsForHost(t *testing.T) {
 	tm := startDeviceCodeLogin(t, "", "--profile", "newprof")
 	tm.WaitFor("Databricks host")
 	tm.Type(termtest.KeyEnter)
+	tm.WaitFor(">> host is required")
+	tm.Type(termtest.KeyCtrlC)
 	_, err := tm.Result()
-	assert.ErrorIs(t, err, errDeviceCodeNeedsHost)
+	assert.EqualError(t, err, "^C")
 }
 
 func TestLoginDeviceCodePickerEmptyHost(t *testing.T) {
@@ -259,8 +263,10 @@ func TestLoginDeviceCodePickerEmptyHost(t *testing.T) {
 	tm.Type(termtest.KeyDown + termtest.KeyEnter)
 	tm.WaitFor("Databricks host")
 	tm.Type(termtest.KeyEnter)
+	tm.WaitFor(">> host is required")
+	tm.Type(termtest.KeyCtrlC)
 	_, err := tm.Result()
-	assert.ErrorIs(t, err, errDeviceCodeNeedsHost)
+	assert.EqualError(t, err, "^C")
 }
 
 func TestLoginDeviceCodeCreateNewProfileRejectsHiddenProfileName(t *testing.T) {
@@ -286,6 +292,23 @@ account_id = acct-123
 	assert.EqualError(t, err, "^C")
 }
 
+func TestLoginDeviceCodeHostEmptyAfterNormalization(t *testing.T) {
+	tests := []struct {
+		name string
+		host string
+	}{
+		{name: "slash", host: "/"},
+		{name: "query params only", host: "?o=123"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tm := startDeviceCodeLogin(t, "", "--profile", "newprof", "--host", tt.host)
+			_, err := tm.Result()
+			assert.ErrorIs(t, err, errInvalidHost)
+		})
+	}
+}
+
 func TestMatchDeviceCodeProfiles(t *testing.T) {
 	tests := []struct {
 		name    string

cmd/auth/login_test.go

@@ -206,6 +206,33 @@ func TestSetHost(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "https://www.host2.test", authArguments.Host)
 
+	// Test host that is empty after normalization from flag
+	authArguments.Host = "?o=123"
+	err = setHostAndAccountId(ctx, profile1, &authArguments, []string{})
+	assert.ErrorIs(t, err, errInvalidHost)
+
+	// Test host that is empty after normalization from argument
+	authArguments.Host = ""
+	err = setHostAndAccountId(ctx, profile1, &authArguments, []string{"/"})
+	assert.ErrorIs(t, err, errInvalidHost)
+
+	// Test host that is empty after normalization from profile
+	authArguments.Host = ""
+	err = setHostAndAccountId(ctx, &profile.Profile{Host: "/"}, &authArguments, []string{})
+	assert.ErrorIs(t, err, errInvalidHost)
+
+	// Test host of only spaces from flag
+	authArguments.Host = "  "
+	err = setHostAndAccountId(ctx, profile1, &authArguments, []string{})
+	assert.EqualError(t, err, `invalid host "  "`)
+
+	// Test spaces around the host are trimmed before the query params are extracted
+	authArguments = auth.AuthArguments{Host: " https://www.host1.test/?o=123 "}
+	err = setHostAndAccountId(ctx, profile1, &authArguments, []string{})
+	assert.NoError(t, err)
+	assert.Equal(t, "https://www.host1.test", authArguments.Host)
+	assert.Equal(t, "123", authArguments.WorkspaceID)
+
 	// Test host is not set. Should prompt.
 	authArguments.Host = ""
 	err = setHostAndAccountId(ctx, nil, &authArguments, []string{})
@@ -791,11 +818,7 @@ func TestSkipWorkspaceProfileNameUsesDiscoveredAccountID(t *testing.T) {
 	args := &auth.AuthArguments{Host: server.URL}
 
 	// Mirrors the pre-naming block in newLoginCommand's RunE for --skip-workspace.
-	params := auth.ExtractHostQueryParams(args.Host)
-	args.Host = params.Host
-	if args.AccountID == "" {
-		args.AccountID = params.AccountID
-	}
+	require.NoError(t, normalizeLoginHost(args))
 	runHostDiscovery(ctx, args)
 
 	assert.Equal(t, "abc-123", args.AccountID)
@@ -1463,6 +1486,23 @@ func TestLoginRejectsPositionalArgWithProfileFlag(t *testing.T) {
 	assert.ErrorContains(t, err, `argument "https://example.com" cannot be combined with --host or --profile`)
 }
 
+func TestLoginSkipWorkspaceHostEmptyAfterNormalization(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
+	t.Setenv("DATABRICKS_CONFIG_FILE", writeTempConfig(t, ""))
+	t.Setenv("DATABRICKS_AUTH_STORAGE", "plaintext")
+
+	ctx := cmdio.MockDiscard(t.Context())
+	authArgs := &auth.AuthArguments{Host: "?a=123"}
+	cmd := newLoginCommand(authArgs)
+	cmd.Flags().String("profile", "", "")
+	cmd.SetContext(ctx)
+	cmd.SetArgs([]string{"--skip-workspace"})
+	err := cmd.Execute()
+	assert.EqualError(t, err, `invalid host "?a=123"`)
+}
+
 func TestPromptForProfileRejectsTakenName(t *testing.T) {
 	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
 		return promptForProfile(ctx, "DEFAULT", []string{"DEFAULT", "taken"})

cmd/auth/testdata/TestPromptForHostRejectsEmptyHost/01-empty.golden

@@ -0,0 +1,3 @@
+Example: https://<databricks-instance>.cloud.databricks.com
+✗ Databricks host: █
+>> host is required

cmd/auth/testdata/TestPromptForHostRejectsEmptyHost/02-spaces.golden

@@ -0,0 +1,3 @@
+Example: https://<databricks-instance>.cloud.databricks.com
+✗ Databricks host:   █
+>> host is required

cmd/auth/testdata/TestPromptForHostRejectsHostEmptyAfterNormalization/01-query-params.golden

@@ -0,0 +1,3 @@
+Example: https://<databricks-instance>.cloud.databricks.com
+✗ Databricks host: ?o=123█
+>> invalid host "?o=123"