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

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,16 @@ 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")
+			}
+			return nil
+		},
 	})
+	return strings.TrimSpace(result), err
 }
 
 func promptForAccountID(ctx context.Context) (string, error) {

cmd/auth/auth_test.go

@@ -7,6 +7,7 @@ import (
 
 	"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 +142,16 @@ 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)
+}

cmd/auth/login.go

@@ -258,11 +258,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:
 				// Fall through to the profile name prompt below.
@@ -275,13 +270,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)
 		}
@@ -344,15 +334,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 DeviceCodeChallenge.
-		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 {
@@ -542,6 +525,32 @@ 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 ?o= and ?a= 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(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.
@@ -573,17 +582,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

@@ -22,10 +22,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

@@ -219,8 +219,10 @@ func startDeviceCodeLogin(t *testing.T, config string, args ...string) *termtest
 	t.Setenv(deviceCodeEnvVar, "true")
 
 	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)
 		cmd.SetArgs(append([]string{"--use-device-code"}, args...))
 		cmd.SetOut(io.Discard)
@@ -233,8 +235,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) {
@@ -249,8 +253,20 @@ 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 TestLoginDeviceCodeHostEmptyAfterNormalization(t *testing.T) {
+	for _, host := range []string{"/", "?o=123"} {
+		t.Run(host, func(t *testing.T) {
+			tm := startDeviceCodeLogin(t, "", "--profile", "newprof", "--host", host)
+			_, err := tm.Result()
+			assert.ErrorIs(t, err, errInvalidHost)
+		})
+	}
 }
 
 func TestMatchDeviceCodeProfiles(t *testing.T) {

cmd/auth/login_test.go

@@ -203,6 +203,21 @@ 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 is not set. Should prompt.
 	authArguments.Host = ""
 	err = setHostAndAccountId(ctx, nil, &authArguments, []string{})
@@ -788,11 +803,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)
@@ -1449,6 +1460,23 @@ func TestLoginRejectsPositionalArgWithHostFlag(t *testing.T) {
 	assert.ErrorContains(t, err, `argument "myprofile" 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 TestLoginRejectsPositionalArgWithProfileFlag(t *testing.T) {
 	ctx := cmdio.MockDiscard(t.Context())
 	authArgs := &auth.AuthArguments{}

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

libs/cmdio/cmdiotest/prompt_validate_baseline_test.go

@@ -45,3 +45,47 @@ func TestPromptBaseline_Validate(t *testing.T) {
 	require.NoError(t, err, "raw output: %q", tm.Raw())
 	assert.Equal(t, "https://example.com", v, "snapshot:\n%s", tm.Snapshot())
 }
+
+// TestPromptBaseline_ValidateEnterBeforeEdit pins that Enter on an untouched
+// prompt flips the glyph to "✗" along with the error line.
+func TestPromptBaseline_ValidateEnterBeforeEdit(t *testing.T) {
+	t.Parallel()
+	tm := termtest.NewPrompt(t, cmdio.PromptOptions{
+		Label: "Workspace host",
+		Validate: func(s string) error {
+			if s == "" {
+				return errors.New("host is required")
+			}
+			return nil
+		},
+	})
+	tm.WaitFor("Workspace host")
+	tm.Type(termtest.KeyEnter)
+	tm.Golden("01-enter")
+
+	tm.Type(termtest.KeyCtrlC)
+	_, err := tm.Result()
+	assert.Error(t, err)
+}
+
+// TestPromptBaseline_ValidateValidBeforeEdit pins that an untouched prompt
+// whose input already passes Validate shows "✔".
+func TestPromptBaseline_ValidateValidBeforeEdit(t *testing.T) {
+	t.Parallel()
+	tm := termtest.NewPrompt(t, cmdio.PromptOptions{
+		Label: "Workspace host",
+		Validate: func(s string) error {
+			if strings.Contains(s, " ") {
+				return errors.New("must not contain spaces")
+			}
+			return nil
+		},
+	})
+	tm.WaitFor("Workspace host")
+	tm.Golden("01-initial")
+
+	tm.Type(termtest.KeyEnter)
+	v, err := tm.Result()
+	require.NoError(t, err, "raw output: %q", tm.Raw())
+	assert.Empty(t, v)
+}

libs/cmdio/cmdiotest/testdata/TestPromptBaseline_Validate/01-empty.golden

@@ -1 +1 @@
-✗ Workspace host: █
+  Workspace host: █

libs/cmdio/cmdiotest/testdata/TestPromptBaseline_ValidateEnterBeforeEdit/01-enter.golden

@@ -0,0 +1,2 @@
+✗ Workspace host: █
+>> host is required

libs/cmdio/cmdiotest/testdata/TestPromptBaseline_ValidateValidBeforeEdit/01-initial.golden

@@ -0,0 +1 @@
+✔ Workspace host: █

libs/cmdio/prompt.go

@@ -33,7 +33,9 @@ type PromptOptions struct {
 	// Validate, when set, is called on every keystroke. While it returns a
 	// non-nil error the leading glyph flips from "✔" to "✗" and Enter is
 	// inert; pressing Enter while invalid surfaces the returned error
-	// below the prompt until the next edit.
+	// below the prompt until the next edit. Until the first edit or Enter,
+	// an invalid input shows a blank glyph instead, so an empty prompt does
+	// not open as an error.
 	Validate func(input string) error
 }
 
@@ -73,6 +75,9 @@ type promptModel struct {
 	deleted   bool
 	submitted bool
 
+	// edited is set by the first change to runes.
+	edited bool
+
 	// submitErr is the error from the last failed Enter attempt. Rendered
 	// below the prompt and cleared on the next edit.
 	submitErr error
@@ -92,12 +97,16 @@ func (m *promptModel) value() string {
 }
 
 // glyph returns the leading status indicator: bold-green ✔ when valid,
-// bold-red ✗ when validate rejects the buffer.
+// bold-red ✗ when validate rejects the buffer after an edit or Enter, and a
+// blank before that.
 func (m *promptModel) glyph() string {
-	if m.validate != nil && m.validate(m.value()) != nil {
-		return ansiBold + ansiRed + glyphInvalid + ansiReset
+	if m.validate == nil || m.validate(m.value()) == nil {
+		return ansiBold + ansiGreen + glyphValid + ansiReset
+	}
+	if !m.edited && m.submitErr == nil {
+		return " "
 	}
-	return ansiBold + ansiGreen + glyphValid + ansiReset
+	return ansiBold + ansiRed + glyphInvalid + ansiReset
 }
 
 func (m *promptModel) Init() tea.Cmd { return nil }
@@ -154,6 +163,7 @@ func (m *promptModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		}
 		m.runes = append(m.runes[:m.cursor-1], m.runes[m.cursor:]...)
 		m.cursor--
+		m.edited = true
 		m.submitErr = nil
 
 	case tea.KeyRunes, tea.KeySpace:
@@ -172,6 +182,7 @@ func (m *promptModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		m.runes = append(m.runes[:m.cursor], typed...)
 		m.runes = append(m.runes, tail...)
 		m.cursor += len(typed)
+		m.edited = true
 		m.submitErr = nil
 
 	default: