Repository navigation
Reject existing profile names after "Create a new profile" - #7196
Merged
Merged
Conversation
After "Create a new profile", the profile name prompt suggests DEFAULT, and pressing Enter logged in to an existing DEFAULT profile and reused its host. For device code login that profile may be an account profile the picker hid, which then failed with an error about a profile the user did not pick. The name prompt now rejects the names of all existing profiles in this case. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
The name prompt suggested DEFAULT even when a DEFAULT profile already existed, and then rejected it on Enter. A taken suggestion is now left out, so the prompt asks for a name without a default and rejects an empty answer. Co-authored-by: Isaac <no-reply@databricks.com>
auth token offers the same "Create a new profile" picker entry, and its name prompt could still log in to an existing profile. It now rejects existing profile names too. The error no longer tells the user to select the profile from the list, because device code login hides some profiles from the picker. auth login now loads the profiles once and filters the picker list from them. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 73e6af8
Top 6 slowest tests (at least 2 minutes):
|
simonfaltum
reviewed
Oct 27, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Nice fix. A few things below.
The main one is that Validate checks the raw input while RunPrompt returns it with trailing \r\n stripped. So a pasted name with a trailing newline gets past the taken-name check, and a pasted blank line gets past the "required" check. I think trimming at the top of Validate fixes both (details inline).
One coverage gap that doesn't fit on a single line: there's no test for "Create a new profile" in the regular browser auth login flow, which is where the original bug was. The promptForProfile unit tests and the device-code/auth token termtests wouldn't catch it if the takenProfileNames = allProfiles.Names() wiring broke for that path. A termtest similar to TestResolveNoArgsTokenCreateNewProfileRejectsTakenName would cover it.
The prompt validated the raw buffer, but RunPrompt trims the trailing newline only after validation. A pasted "taken\n" passed the taken-name check, and a pasted blank line or spaces passed the required check. Co-authored-by: Isaac <no-reply@databricks.com>
SaveToProfile rejects it, but only after the user has gone through the OAuth flow. Co-authored-by: Isaac <no-reply@databricks.com>
LoadProfiles skips sections without a host, so a name like [foo] with only cluster_id was accepted as a new profile, and SaveToProfile merged the login into that section, keeping its old keys. Read the taken names from the config file sections instead. Co-authored-by: Isaac <no-reply@databricks.com>
The taken profile names now come from the config file sections, so login no longer needs all profiles to filter the picker list from. Co-authored-by: Isaac <no-reply@databricks.com>
The test only checked that an empty answer is rejected, so it would still pass if the hidden DEFAULT profile name were only dropped from the suggestion. Co-authored-by: Isaac <no-reply@databricks.com>
… picker test env.Get falls back to the OS environment, so with either one exported resolveNoArgsToken returned before the picker and the test timed out. Co-authored-by: Isaac <no-reply@databricks.com>
The profile name prompt rejects the names of all existing sections, so runInlineLogin always creates a new profile. Looking up the existing profile, preserving its scopes, resources and client ID, and clearing its keys could no longer do anything. Co-authored-by: Isaac <no-reply@databricks.com>
This is the flow of the original bug. The other tests cover the prompt itself, device code login and auth token, but not this wiring. Co-authored-by: Isaac <no-reply@databricks.com>
TestLoginCreateNewProfileRejectsTakenName copied the setup of startDeviceCodeLogin. Move that setup into startLogin, which startDeviceCodeLogin now calls. Co-authored-by: Isaac <no-reply@databricks.com>
simonfaltum
approved these changes
Oct 29, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Looks good to me, and all my earlier comments are addressed. A few non-blocking things inline. Most are bugs that already exist on main but get past the new check, so follow-ups are fine.
One more that has no changed line to anchor on: in runInlineLogin (token.go L461) the timeout ctx is created after NewPersistentAuth, and Challenge uses the ctx stored at construction, so defaultTimeout never applies. Closing the browser tab leaves auth token waiting until Ctrl+C. Moving those two lines above NewPersistentAuth, as login.go does, fixes it. Unrelated to this change, so a follow-up is fine.
| func SaveToProfile(ctx context.Context, cfg *config.Config, clearKeys ...string) error { | ||
| if cfg.Profile == databricksSettingsSection { | ||
| return fmt.Errorf("profile name %q is reserved for internal use", databricksSettingsSection) | ||
| if err := ValidateProfileName(cfg.Profile); err != nil { |
Member
There was a problem hiding this comment.
[nice to have] Further down (L494), section.Key(attr.Name) falls back to the parent section's key when the section doesn't have it, because go-ini treats team.prod as a child of [team]. So with [team] holding host and token, typing team.prod after "Create a new profile" passes the taken-name check. After OAuth, the save rewrites [team]'s host and auth_type, keeps its token, and leaves an empty [team.prod].
Using section.NewKey(attr.Name, attr.GetString(cfg)) only looks at the section's own keys, which fixes it (L312 in SaveResourcesToProfile has the same pattern). Fine as a follow-up since it predates this PR.
The taken profile names are read before the OAuth wait. If a section with the chosen name appears in the meantime, the save merges into it, so clear the same keys as the auth login saves do. Co-authored-by: Isaac <no-reply@databricks.com>
A section without a host is hidden from the picker but rejected at the name prompt, so the error now says to rerun the command with --profile. The prompt is shared by auth login and auth token, so the hint does not name a specific command. Co-authored-by: Isaac <no-reply@databricks.com>
go-ini writes section headers unescaped, so a pasted "foo\nbar" or a name like "[foo]" produced a config file that no longer parses. Co-authored-by: Isaac <no-reply@databricks.com>
This was referenced Oct 30, 2026
go-ini reads the section name up to the last ']' on the line, so names like "[foo]" or "a[b" round-trip. Rejecting them broke existing profiles with such names in auth switch, configure and auth login --profile. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: a7e6114
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
After "Create a new profile" in the
databricks auth loginanddatabricks auth tokenpickers, the profile name prompt rejects the name of any existing config section withprofile X already exists. Choose a different name. This includes the profiles the device code picker hides and sections without ahost. A takenDEFAULTis no longer suggested: the prompt then has no default and requires a name.All profile name prompts now also trim spaces and pasted newlines, and reject the reserved
__settings__name before the OAuth flow rather than after it.Saving a profile now rejects names that contain line breaks, for example a pasted
foo\nbar. go-ini writes section headers unescaped, so such a name produced a config file that no longer parses. This applies to every command that saves a profile or sets the default profile, such asauth login,auth token,auth switchandconfigure.The
auth tokeninline login can no longer target an existing profile, so its code for keeping that profile's scopes, client ID and resources is removed.Why
Follow-up to a review comment on #7179. With no host known yet, the prompt suggested
DEFAULT, and pressing Enter logged in to the existingDEFAULTprofile and reused its host. With--use-device-code, if that was an account profile the picker hid, login failed with--use-device-code is only supported for workspace hosts.A generated name like
DEFAULT-1is not offered instead: the host is not known yet, so the name would not say which workspace the profile is for.Tests
TestPromptForProfile*cover the prompt: free and taken defaults, and empty, taken, reserved and pasted names.TestLoginCreateNewProfileRejectsTakenName,TestLoginDeviceCodeCreateNewProfileRejectsHiddenProfileNameandTestResolveNoArgsTokenCreateNewProfileRejectsTakenNamecover the browser login, device code login andauth tokenflows. Like the other picker tests, they are skipped on Windows.TestGetSectionNamescovers sections without a host and empty sections.TestValidateProfileNameandTestSaveToProfile_SquareBracketsRoundTripcover line breaks, and check that names with square brackets are still accepted and parse back.cmd/authunit tests andTestAccept/cmd/authpass.This pull request and its description were written by Isaac.