-
Notifications
You must be signed in to change notification settings - Fork 146
Add interactive profile picker to auth logout #4616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d12cc7c
f515acd
efe7f6f
bf46cfd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,276 @@ | ||
| package auth | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "runtime" | ||
| "slices" | ||
| "strings" | ||
|
|
||
| "github.com/databricks/cli/libs/cmdio" | ||
| "github.com/databricks/cli/libs/databrickscfg" | ||
| "github.com/databricks/cli/libs/databrickscfg/profile" | ||
| "github.com/databricks/cli/libs/log" | ||
| "github.com/databricks/databricks-sdk-go/credentials/u2m/cache" | ||
| "github.com/manifoldco/promptui" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| const logoutWarningTemplate = `{{ "Warning" | yellow }}: This will permanently log out of profile {{ .ProfileName | bold }}. | ||
|
|
||
| The following changes will be made: | ||
| - Remove profile {{ .ProfileName | bold }} from {{ .ConfigPath }} | ||
| - Delete any cached OAuth tokens for this profile | ||
|
|
||
| You will need to run {{ "databricks auth login" | bold }} to re-authenticate. | ||
| ` | ||
|
|
||
| func newLogoutCommand() *cobra.Command { | ||
| defaultConfigPath := "~/.databrickscfg" | ||
| if runtime.GOOS == "windows" { | ||
| defaultConfigPath = "%USERPROFILE%\\.databrickscfg" | ||
| } | ||
|
|
||
| cmd := &cobra.Command{ | ||
| Use: "logout", | ||
| Short: "Log out of a Databricks profile", | ||
| Long: fmt.Sprintf(`Log out of a Databricks profile. | ||
|
|
||
| This command removes the specified profile from %s and deletes | ||
| any associated cached OAuth tokens. | ||
|
|
||
| You will need to run "databricks auth login" to re-authenticate after | ||
| logging out. | ||
|
|
||
| This command requires a profile to be specified (using --profile). If you | ||
| omit --profile and run in an interactive terminal, you'll be shown an | ||
| interactive profile picker to select which profile to log out of. | ||
|
|
||
| While this command always removes the specified profile, the runtime behaviour | ||
| depends on whether you run it in an interactive terminal and which flags you | ||
| provide. | ||
|
|
||
| 1. If you specify --profile, the command will log out of that profile. | ||
| In an interactive terminal, you'll be asked to confirm unless --force | ||
| is specified. | ||
|
|
||
| 2. If you omit --profile and run in an interactive terminal, you'll be shown | ||
| an interactive picker listing all profiles from your configuration file. | ||
| Profiles are sorted alphabetically by name. You can search by profile | ||
| name, host, or account ID. After selecting a profile, you'll be asked to | ||
| confirm unless --force is specified. | ||
|
|
||
| 3. If you omit --profile and run in a non-interactive environment (e.g. | ||
| CI/CD pipelines), the command will fail with an error asking you to | ||
| specify --profile. | ||
|
|
||
| 4. Use --force to skip the confirmation prompt. This is required when | ||
| running in non-interactive mode; otherwise the command will fail.`, | ||
| defaultConfigPath), | ||
| } | ||
|
|
||
| var force bool | ||
| var profileName string | ||
| cmd.Flags().BoolVar(&force, "force", false, "Skip confirmation prompt") | ||
| cmd.Flags().StringVar(&profileName, "profile", "", "The profile to log out of") | ||
|
|
||
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
| ctx := cmd.Context() | ||
|
|
||
| if profileName == "" { | ||
| if !cmdio.IsPromptSupported(ctx) { | ||
| return errors.New("the command is being run in a non-interactive environment, please specify a profile to log out of using --profile") | ||
| } | ||
| selected, err := promptForLogoutProfile(ctx, profile.DefaultProfiler) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| profileName = selected | ||
| } | ||
|
|
||
| tokenCache, err := cache.NewFileTokenCache() | ||
| if err != nil { | ||
| log.Warnf(ctx, "Failed to open token cache: %v", err) | ||
| } | ||
|
|
||
| return runLogout(ctx, logoutArgs{ | ||
| profileName: profileName, | ||
| force: force, | ||
| profiler: profile.DefaultProfiler, | ||
| tokenCache: tokenCache, | ||
| configFilePath: os.Getenv("DATABRICKS_CONFIG_FILE"), | ||
| }) | ||
| } | ||
|
|
||
| return cmd | ||
| } | ||
|
|
||
| type logoutArgs struct { | ||
| profileName string | ||
| force bool | ||
| profiler profile.Profiler | ||
| tokenCache cache.TokenCache | ||
| configFilePath string | ||
| } | ||
|
|
||
| func runLogout(ctx context.Context, args logoutArgs) error { | ||
| matchedProfile, err := getMatchingProfile(ctx, args.profileName, args.profiler) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !args.force { | ||
| if !cmdio.IsPromptSupported(ctx) { | ||
| return errors.New("please specify --force to skip confirmation in non-interactive mode") | ||
| } | ||
|
|
||
| configPath := args.configFilePath | ||
| if configPath == "" { | ||
| configPath = "~/.databrickscfg" | ||
| } | ||
| err := cmdio.RenderWithTemplate(ctx, map[string]string{ | ||
| "ProfileName": args.profileName, | ||
| "ConfigPath": configPath, | ||
|
Comment on lines
+129
to
+135
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "~/.databrickscfg" appears as a fallback here and also as defaultConfigPath at line 31. Consider reusing the constant (or better, resolving the actual path from the profiler/config). |
||
| }, "", logoutWarningTemplate) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| approved, err := cmdio.AskYesOrNo(ctx, "Are you sure?") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if !approved { | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| clearTokenCache(ctx, *matchedProfile, args.profiler, args.tokenCache) | ||
|
|
||
| err = databrickscfg.DeleteProfile(ctx, args.profileName, args.configFilePath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to remove profile: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
|
Comment on lines
+150
to
+157
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we run DeleteProfile first, then do best-effort token cleanup? Right now a config write failure returns an error but can still remove tokens, which leaves a partial logout state. |
||
| } | ||
|
Comment on lines
+157
to
+158
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we need a success message here? |
||
|
|
||
| // getMatchingProfile loads a profile by name and returns an error with | ||
| // available profile names if the profile is not found. | ||
| func getMatchingProfile(ctx context.Context, profileName string, profiler profile.Profiler) (*profile.Profile, error) { | ||
| if profiler == nil { | ||
| return nil, errors.New("profiler cannot be nil") | ||
| } | ||
|
Comment on lines
+162
to
+165
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is always called with profile.DefaultProfiler. A nil profiler is a programming error, not a user error. I'd drop this check (or if you really want it, make it a panic). The rest of the codebase doesn't guard against nil profilers. |
||
|
|
||
| profiles, err := profiler.LoadProfiles(ctx, profile.WithName(profileName)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if len(profiles) == 0 { | ||
| allProfiles, err := profiler.LoadProfiles(ctx, profile.MatchAllProfiles) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("profile %q not found", profileName) | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("profile %q not found. Available profiles: %s", profileName, allProfiles.Names()) | ||
| } | ||
|
|
||
| return &profiles[0], nil | ||
| } | ||
|
|
||
| type logoutProfileItem struct { | ||
| PaddedName string | ||
| profile.Profile | ||
| } | ||
|
|
||
| // promptForLogoutProfile shows an interactive profile picker for logout. | ||
| // Account profiles are displayed as "name (account: id)", workspace profiles | ||
| // as "name (host)". | ||
| func promptForLogoutProfile(ctx context.Context, profiler profile.Profiler) (string, error) { | ||
| allProfiles, err := profiler.LoadProfiles(ctx, profile.MatchAllProfiles) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| if len(allProfiles) == 0 { | ||
| return "", errors.New("no profiles configured. Run 'databricks auth login' to create a profile") | ||
| } | ||
|
|
||
| slices.SortFunc(allProfiles, func(a, b profile.Profile) int { | ||
| return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name)) | ||
| }) | ||
|
|
||
| maxNameLen := 0 | ||
| for _, p := range allProfiles { | ||
| maxNameLen = max(maxNameLen, len(p.Name)) | ||
| } | ||
|
|
||
| items := make([]logoutProfileItem, len(allProfiles)) | ||
| for i, p := range allProfiles { | ||
| items[i] = logoutProfileItem{ | ||
| PaddedName: fmt.Sprintf("%-*s", maxNameLen, p.Name), | ||
| Profile: p, | ||
| } | ||
| } | ||
|
|
||
| i, _, err := cmdio.RunSelect(ctx, &promptui.Select{ | ||
| Label: "Select a profile to log out of", | ||
| Items: items, | ||
| StartInSearchMode: len(items) > 5, | ||
| // Allow searching by name, host, and account ID. | ||
| Searcher: func(input string, index int) bool { | ||
| input = strings.ToLower(input) | ||
| name := strings.ToLower(items[index].Name) | ||
| host := strings.ToLower(items[index].Host) | ||
| accountID := strings.ToLower(items[index].AccountID) | ||
| return strings.Contains(name, input) || strings.Contains(host, input) || strings.Contains(accountID, input) | ||
| }, | ||
| Templates: &promptui.SelectTemplates{ | ||
| Label: "{{ . | faint }}", | ||
| Active: `▸ {{.PaddedName | bold}}{{if .AccountID}} (account: {{.AccountID}}){{else}} ({{.Host}}){{end}}`, | ||
| Inactive: ` {{.PaddedName}}{{if .AccountID}} (account: {{.AccountID | faint}}){{else}} ({{.Host | faint}}){{end}}`, | ||
| Selected: `{{ "Selected profile" | faint }}: {{ .Name | bold }}`, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return items[i].Name, nil | ||
| } | ||
|
|
||
| // clearTokenCache removes cached OAuth tokens for the given profile from the | ||
| // token cache. It removes: | ||
| // 1. The entry keyed by the profile name. | ||
| // 2. The entry keyed by the host URL, but only if no other remaining profile | ||
| // references the same host. | ||
| func clearTokenCache(ctx context.Context, p profile.Profile, profiler profile.Profiler, tokenCache cache.TokenCache) { | ||
| if tokenCache == nil { | ||
| return | ||
| } | ||
|
|
||
| profileName := p.Name | ||
| if err := tokenCache.Store(profileName, nil); err != nil { | ||
| log.Warnf(ctx, "Failed to delete profile-keyed token for profile %q: %v", profileName, err) | ||
| } | ||
|
|
||
| host := strings.TrimRight(p.Host, "/") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For account and unified profiles, the legacy host cache key is "/oidc/accounts/<account_id>", not just "". This cleanup currently checks/removes by host only, so stale tokens can remain after logout. Could we compute, and do something smarter here to clear the host cache key? |
||
| if host == "" { | ||
| return | ||
| } | ||
|
|
||
| otherProfilesUsingHost, err := profiler.LoadProfiles(ctx, func(candidate profile.Profile) bool { | ||
| return candidate.Name != profileName && profile.WithHost(host)(candidate) | ||
| }) | ||
| if err != nil { | ||
| log.Warnf(ctx, "Failed to load profiles using host %q: %v", host, err) | ||
| return | ||
| } | ||
|
|
||
| if len(otherProfilesUsingHost) == 0 { | ||
| if err := tokenCache.Store(host, nil); err != nil { | ||
| log.Warnf(ctx, "Failed to delete host-keyed token for host %q: %v", host, err) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The profile existence is validated here and again inside DeleteProfile (via findMatchingProfile). Since you already have the matched profile, consider having DeleteProfile accept the profile name without re-verifying, or consolidate the check in one place.