Reject existing profile names after "Create a new profile"

.nextchanges/cli/login-new-profile-existing-name.md

@@ -0,0 +1 @@
+* `databricks auth login` and `databricks auth token` now reject the name of an existing profile after "Create a new profile" instead of logging in to that profile. ([#7196](https://github.com/databricks/cli/pull/7196))

cmd/auth/login.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -26,14 +27,40 @@ import (
 	"golang.org/x/oauth2"
 )
 
-func promptForProfile(ctx context.Context, defaultValue string) (string, error) {
+// promptForProfile prompts for a profile name, rejecting the names in
+// takenNames. A taken defaultValue is not suggested.
+func promptForProfile(ctx context.Context, defaultValue string, takenNames []string) (string, error) {
 	if !cmdio.IsPromptSupported(ctx) {
 		return "", nil
 	}
 
+	label := "Databricks profile name"
+	if slices.Contains(takenNames, defaultValue) {
+		defaultValue = ""
+	} else {
+		label += " [" + defaultValue + "]"
+	}
 	result, err := cmdio.RunPrompt(ctx, cmdio.PromptOptions{
-		Label: "Databricks profile name [" + defaultValue + "]",
+		Label: label,
+		Validate: func(input string) error {
+			input = strings.TrimSpace(input)
+			if input == "" {
+				input = defaultValue
+			}
+			if input == "" {
+				return errors.New("profile name is required")
+			}
+			// Fail here rather than after the OAuth flow, in SaveToProfile.
+			if err := databrickscfg.ValidateProfileName(input); err != nil {
+				return err
+			}
+			if slices.Contains(takenNames, input) {
+				return fmt.Errorf("profile %s already exists. Choose a different name, or rerun with --profile %s to use it", input, input)
+			}
+			return nil
+		},
 	})
+	result = strings.TrimSpace(result)
 	if result == "" {
 		return defaultValue, err
 	}
@@ -227,7 +254,8 @@ a new profile is created.
 		// two action entries so the user can choose between web-based discovery
 		// (Create a new profile) and a manual host URL. Device code login skips
 		// discovery, so its "Create a new profile" asks for the host after the
-		// profile name, unless a profile with that name already has a host.
+		// profile name.
+		var takenProfileNames []string
 		if profileName == "" && authArguments.Host == "" && len(args) == 0 && cmdio.IsPromptSupported(ctx) {
 			matchProfiles := profile.MatchAllProfiles
 			if useDeviceCode {
@@ -265,7 +293,14 @@ a new profile is created.
 				}
 				authArguments.Host = host
 			case profilePickerCreateNew:
-				// Fall through to the profile name prompt below.
+				// Reject the names of existing profiles, including those the
+				// picker hid and sections without a host, so that the profile
+				// name prompt below cannot log in to an existing profile and
+				// reuse its host or keys.
+				takenProfileNames, err = databrickscfg.GetSectionNames(ctx, env.Get(ctx, "DATABRICKS_CONFIG_FILE"))
+				if err != nil {
+					return err
+				}
 			}
 		}
 
@@ -293,7 +328,7 @@ a new profile is created.
 			if profileName == "" {
 				profileName = "DEFAULT"
 			}
-			profileName, err = promptForProfile(ctx, profileName)
+			profileName, err = promptForProfile(ctx, profileName, takenProfileNames)
 			if err != nil {
 				return err
 			}

cmd/auth/login_device_code_test.go

@@ -208,27 +208,34 @@ func TestIsDeviceCodeSupportedForHost(t *testing.T) {
 	}
 }
 
-// startDeviceCodeLogin runs "auth login --use-device-code" with args in a
-// terminal emulator, using config as the contents of .databrickscfg.
-func startDeviceCodeLogin(t *testing.T, config string, args ...string) *termtest.Term[struct{}] {
+// startLogin runs "auth login" with args in a terminal emulator, using config
+// as the contents of .databrickscfg.
+func startLogin(t *testing.T, config string, args ...string) *termtest.Term[struct{}] {
 	home := t.TempDir()
 	t.Setenv("HOME", home)
 	t.Setenv("USERPROFILE", home)
 	t.Setenv("DATABRICKS_CONFIG_FILE", writeTempConfig(t, config))
 	t.Setenv("DATABRICKS_AUTH_STORAGE", "plaintext")
-	t.Setenv(deviceCodeEnvVar, "true")
 
 	return termtest.Run(t, func(ctx context.Context) (struct{}, error) {
 		cmd := newLoginCommand(&auth.AuthArguments{})
 		cmd.Flags().String("profile", "", "")
 		cmd.SetContext(ctx)
-		cmd.SetArgs(append([]string{"--use-device-code"}, args...))
+		// Cobra reads os.Args when the args are nil.
+		cmd.SetArgs(append([]string{}, args...))
 		cmd.SetOut(io.Discard)
 		cmd.SetErr(io.Discard)
 		return struct{}{}, cmd.Execute()
 	})
 }
 
+// startDeviceCodeLogin runs "auth login --use-device-code" with args in a
+// terminal emulator, using config as the contents of .databrickscfg.
+func startDeviceCodeLogin(t *testing.T, config string, args ...string) *termtest.Term[struct{}] {
+	t.Setenv(deviceCodeEnvVar, "true")
+	return startLogin(t, config, append([]string{"--use-device-code"}, args...)...)
+}
+
 func TestLoginDeviceCodeNewProfilePromptsForHost(t *testing.T) {
 	tm := startDeviceCodeLogin(t, "", "--profile", "newprof")
 	tm.WaitFor("Databricks host")
@@ -253,6 +260,29 @@ func TestLoginDeviceCodePickerEmptyHost(t *testing.T) {
 	assert.ErrorIs(t, err, errDeviceCodeNeedsHost)
 }
 
+func TestLoginDeviceCodeCreateNewProfileRejectsHiddenProfileName(t *testing.T) {
+	// See TestLoginDeviceCodePickerEmptyHost.
+	if runtime.GOOS == "windows" {
+		t.Skip("bubbletea cannot cancel reads on non-console input on Windows")
+	}
+	tm := startDeviceCodeLogin(t, `[DEFAULT]
+host = https://accounts.cloud.databricks.test
+account_id = acct-123
+`)
+	tm.WaitFor(profilePickerCreateNewLabel)
+	tm.Type(termtest.KeyEnter)
+	tm.WaitFor("Databricks profile name")
+	tm.Type(termtest.KeyEnter)
+	tm.WaitFor(">> profile name is required")
+	tm.Type("DEFAULT" + termtest.KeyEnter)
+	tm.WaitFor(">> profile DEFAULT already exists")
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("DEFAULT")) + "newprof" + termtest.KeyEnter)
+	tm.WaitFor("Databricks host")
+	tm.Type(termtest.KeyCtrlC)
+	_, err := tm.Result()
+	assert.EqualError(t, err, "^C")
+}
+
 func TestMatchDeviceCodeProfiles(t *testing.T) {
 	tests := []struct {
 		name    string

cmd/auth/login_test.go

@@ -10,6 +10,8 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -18,6 +20,7 @@ import (
 	"github.com/databricks/cli/libs/auth/storage"
 	"github.com/databricks/cli/libs/auth/u2m"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/cmdio/cmdiotest/termtest"
 	"github.com/databricks/cli/libs/databrickscfg/profile"
 	"github.com/databricks/cli/libs/env"
 	"github.com/databricks/cli/libs/log"
@@ -1459,3 +1462,81 @@ func TestLoginRejectsPositionalArgWithProfileFlag(t *testing.T) {
 	err := cmd.Execute()
 	assert.ErrorContains(t, err, `argument "https://example.com" cannot be combined with --host or --profile`)
 }
+
+func TestPromptForProfileRejectsTakenName(t *testing.T) {
+	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
+		return promptForProfile(ctx, "DEFAULT", []string{"DEFAULT", "taken"})
+	})
+	screen := tm.WaitFor("Databricks profile name")
+	assert.NotContains(t, screen, "[DEFAULT]")
+	tm.Type(termtest.KeyEnter)
+	tm.WaitFor(">> profile name is required")
+	tm.Type("taken" + termtest.KeyEnter)
+	tm.WaitFor(">> profile taken already exists. Choose a different name, or rerun with --profile taken to use it")
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("taken")) + "__settings__" + termtest.KeyEnter)
+	tm.WaitFor(`>> profile name "__settings__" is reserved for internal use`)
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("__settings__")) + "take" + termtest.KeyEnter)
+	name, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "take", name)
+}
+
+func TestPromptForProfileSuggestsFreeDefault(t *testing.T) {
+	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
+		return promptForProfile(ctx, "DEFAULT", []string{"taken"})
+	})
+	tm.WaitFor("Databricks profile name [DEFAULT]")
+	tm.Type(termtest.KeyEnter)
+	name, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "DEFAULT", name)
+}
+
+func TestPromptForProfileTrimsPastedName(t *testing.T) {
+	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
+		return promptForProfile(ctx, "DEFAULT", []string{"DEFAULT", "taken"})
+	})
+	tm.WaitFor("Databricks profile name")
+	// A bracketed paste arrives as a single input event, so the trailing
+	// newline lands in the buffer instead of submitting the prompt.
+	tm.Type("\x1b[200~taken\n\x1b[201~" + termtest.KeyEnter)
+	tm.WaitFor(">> profile taken already exists")
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("taken\n")) + "   " + termtest.KeyEnter)
+	tm.WaitFor(">> profile name is required")
+	tm.Type(" new " + termtest.KeyEnter)
+	name, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "new", name)
+}
+
+func TestPromptForProfileRejectsMultilinePaste(t *testing.T) {
+	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
+		return promptForProfile(ctx, "DEFAULT", nil)
+	})
+	tm.WaitFor("Databricks profile name [DEFAULT]")
+	tm.Type("\x1b[200~foo\nbar\x1b[201~" + termtest.KeyEnter)
+	tm.WaitFor(`>> profile name "foo\nbar" must not contain line breaks`)
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("foo\nbar")) + "[foo]" + termtest.KeyEnter)
+	name, err := tm.Result()
+	require.NoError(t, err)
+	assert.Equal(t, "[foo]", name)
+}
+
+func TestLoginCreateNewProfileRejectsTakenName(t *testing.T) {
+	// See TestLoginDeviceCodePickerEmptyHost.
+	if runtime.GOOS == "windows" {
+		t.Skip("bubbletea cannot cancel reads on non-console input on Windows")
+	}
+	tm := startLogin(t, `[DEFAULT]
+host = https://default.cloud.databricks.test
+`)
+	tm.WaitFor(profilePickerCreateNewLabel)
+	tm.Type(termtest.KeyDown + termtest.KeyEnter)
+	screen := tm.WaitFor("Databricks profile name")
+	assert.NotContains(t, screen, "[DEFAULT]")
+	tm.Type("DEFAULT" + termtest.KeyEnter)
+	tm.WaitFor(">> profile DEFAULT already exists")
+	tm.Type(termtest.KeyCtrlC)
+	_, err := tm.Result()
+	assert.EqualError(t, err, "^C")
+}

cmd/auth/token.go

@@ -422,38 +422,28 @@ func resolveNoArgsToken(ctx context.Context, profiler profile.Profiler, authArgs
 
 // runInlineLogin runs a minimal interactive login flow: prompts for a profile
 // name and host, performs the OAuth challenge, saves the profile to
-// .databrickscfg, and returns the new profile name and profile.
+// .databrickscfg, and returns the new profile name and profile. It rejects
+// the names of existing sections so that it cannot log in to an existing
+// profile.
 func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenStore storage.Store, mode storage.StorageMode) (string, *profile.Profile, error) {
-	profileName, err := promptForProfile(ctx, "DEFAULT")
+	takenNames, err := databrickscfg.GetSectionNames(ctx, env.Get(ctx, "DATABRICKS_CONFIG_FILE"))
 	if err != nil {
 		return "", nil, err
 	}
-
-	existingProfile, err := loadProfileByName(ctx, profileName, profiler)
+	profileName, err := promptForProfile(ctx, "DEFAULT", takenNames)
 	if err != nil {
 		return "", nil, err
 	}
 
 	loginArgs := &auth.AuthArguments{}
 
-	err = setHostAndAccountId(ctx, existingProfile, loginArgs, nil)
+	err = setHostAndAccountId(ctx, nil, loginArgs, nil)
 	if err != nil {
 		return "", nil, err
 	}
 
 	loginArgs.Profile = profileName
 
-	// Preserve scopes from the existing profile so the inline login
-	// uses the same scopes the user previously configured.
-	var scopesList []string
-	if existingProfile != nil && existingProfile.Scopes != "" {
-		scopesList = splitScopes(existingProfile.Scopes)
-	}
-
-	// Preserve RFC 8707 resource indicators from the existing profile so the
-	// inline login requests the same resources the user previously configured.
-	resourcesList := u2mResourcesFromProfile(existingProfile)
-
 	oauthArgument, err := loginArgs.ToOAuthArgument()
 	if err != nil {
 		return "", nil, err
@@ -462,15 +452,6 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenStore s
 		u2m.WithOAuthArgument(oauthArgument),
 		u2m.WithBrowser(func(url string) error { return browser.Open(ctx, url) }),
 	}
-	if clientID := u2mClientIDFromProfile(existingProfile); clientID != "" {
-		persistentAuthOpts = append(persistentAuthOpts, u2m.WithClientID(clientID))
-	}
-	if len(scopesList) > 0 {
-		persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList))
-	}
-	if len(resourcesList) > 0 {
-		persistentAuthOpts = append(persistentAuthOpts, u2m.WithResources(resourcesList))
-	}
 	persistentAuth, err := u2m.NewPersistentAuth(ctx, persistentAuthOpts...)
 	if err != nil {
 		return "", nil, err
@@ -485,6 +466,8 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenStore s
 		return "", nil, err
 	}
 
+	// The section may have been created during the OAuth wait, after the
+	// taken names were read.
 	clearKeys := oauthLoginClearKeys()
 	clearKeys = append(clearKeys, databrickscfg.ExperimentalIsUnifiedHostKey)
 
@@ -495,18 +478,10 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenStore s
 		AccountID:   loginArgs.AccountID,
 		WorkspaceID: loginArgs.WorkspaceID,
 		ConfigFile:  env.Get(ctx, "DATABRICKS_CONFIG_FILE"),
-		Scopes:      scopesList,
-		ClientID:    u2mClientIDFromProfile(existingProfile),
 	}, clearKeys...)
 	if err != nil {
 		return "", nil, err
 	}
-	// TODO: resources is not supported in the SDK yet. This function is
-	// a short-cut to add the property to the profile until it is supported
-	// in the SDK.
-	if err := databrickscfg.SaveResourcesToProfile(ctx, profileName, env.Get(ctx, "DATABRICKS_CONFIG_FILE"), resourcesList); err != nil {
-		return "", nil, err
-	}
 	if err := storeLoginToken(ctx, tokenStore, mode, oauthArgument, token); err != nil {
 		return "", nil, err
 	}

cmd/auth/token_test.go

@@ -7,13 +7,16 @@ import (
 	"errors"
 	"net/http"
 	"net/url"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/databricks/cli/libs/auth"
 	"github.com/databricks/cli/libs/auth/storage"
 	"github.com/databricks/cli/libs/auth/u2m"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/cmdio/cmdiotest/termtest"
 	"github.com/databricks/cli/libs/databrickscfg/profile"
 	"github.com/databricks/cli/libs/env"
 	"github.com/databricks/databricks-sdk-go/httpclient/fixtures"
@@ -965,3 +968,33 @@ func TestWriteTokenErrorOutput(t *testing.T) {
 	assert.Equal(t, unauthenticatedErrorCode, got.ErrorCode)
 	assert.Equal(t, "refresh token is invalid", got.Message)
 }
+
+func TestResolveNoArgsTokenCreateNewProfileRejectsTakenName(t *testing.T) {
+	// See TestLoginDeviceCodePickerEmptyHost.
+	if runtime.GOOS == "windows" {
+		t.Skip("bubbletea cannot cancel reads on non-console input on Windows")
+	}
+	// Either one would resolve the profile before the picker is shown.
+	t.Setenv("DATABRICKS_HOST", "")
+	t.Setenv("DATABRICKS_CONFIG_PROFILE", "")
+	t.Setenv("DATABRICKS_CONFIG_FILE", writeTempConfig(t, `[prod]
+host = https://prod.cloud.databricks.test
+
+[hostless]
+cluster_id = abc
+`))
+	tm := termtest.Run(t, func(ctx context.Context) (string, error) {
+		name, _, err := resolveNoArgsToken(ctx, profile.DefaultProfiler, &auth.AuthArguments{}, nil, storage.StorageModePlaintext)
+		return name, err
+	})
+	tm.WaitFor(profilePickerCreateNewLabel)
+	tm.Type(termtest.KeyDown + termtest.KeyEnter)
+	tm.WaitFor("Databricks profile name [DEFAULT]")
+	tm.Type("prod" + termtest.KeyEnter)
+	tm.WaitFor(">> profile prod already exists")
+	tm.Type(strings.Repeat(termtest.KeyBackspace, len("prod")) + "hostless" + termtest.KeyEnter)
+	tm.WaitFor(">> profile hostless already exists")
+	tm.Type(termtest.KeyCtrlC)
+	_, err := tm.Result()
+	assert.EqualError(t, err, "^C")
+}