From 2fc6dbd851a435dbeb219609f09a924381e0620e Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 28 Nov 2023 21:03:52 +0000 Subject: [PATCH] Add user flag to auth logout command --- pkg/cmd/auth/logout/logout.go | 53 +++++--- pkg/cmd/auth/logout/logout_test.go | 193 +++++++++++++++++++++-------- 2 files changed, 173 insertions(+), 73 deletions(-) diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index 1c137515a..a20c8a77b 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -17,6 +17,7 @@ type LogoutOptions struct { Config func() (config.Config, error) Prompter shared.Prompt Hostname string + Username string } func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Command { @@ -29,23 +30,25 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "logout", Args: cobra.ExactArgs(0), - Short: "Log out of a GitHub host", - Long: heredoc.Docf(`Remove authentication for a GitHub host. + Short: "Log out of a GitHub user account", + Long: heredoc.Docf(` + Remove authentication for a GitHub user account. - This command removes the authentication configuration for a host either specified - interactively or via %[1]s--hostname%[1]s. + This command removes the authentication configuration for a user account + either specified interactively or via the %[1]s--hostname%[1]s and %[1]s--user%[1]s flags. `, "`"), Example: heredoc.Doc(` + # Select what host and user account to log out of via a prompt $ gh auth logout - # => select what host to log out of via a prompt - $ gh auth logout --hostname enterprise.internal - # => log out of specified host + # Log out of specified user account on specified host + $ gh auth logout --hostname enterprise.internal --user monalisa `), RunE: func(cmd *cobra.Command, args []string) error { - if opts.Hostname == "" && !opts.IO.CanPrompt() { - return cmdutil.FlagErrorf("--hostname required when not running interactively") - } + // if (opts.Hostname == "" || opts.Username == "") && !opts.IO.CanPrompt() { + // return cmdutil.FlagErrorf("--hostname and --user required when not running interactively") + // } + if runF != nil { return runF(opts) } @@ -55,14 +58,14 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") + cmd.Flags().StringVarP(&opts.Username, "user", "u", "", "The user account to log out of") return cmd } func logoutRun(opts *LogoutOptions) error { hostname := opts.Hostname - // TODO: opts.Username likeley needed in the future - username := "" + username := opts.Username cfg, err := opts.Config() if err != nil { @@ -79,6 +82,13 @@ func logoutRun(opts *LogoutOptions) error { if !slices.Contains(knownHosts, hostname) { return fmt.Errorf("not logged in to %s", hostname) } + + if username != "" { + knownUsers, _ := cfg.Authentication().UsersForHost(hostname) + if !slices.Contains(knownUsers, username) { + return fmt.Errorf("not logged in as %s on %s", username, hostname) + } + } } type hostUser struct { @@ -96,18 +106,18 @@ func logoutRun(opts *LogoutOptions) error { return err } for _, user := range knownUsers { + if username != "" && user != username { + continue + } candidates = append(candidates, hostUser{host: host, user: user}) } } - // We can ignore the error here because a host must always have an active user - preLogoutActiveUser, _ := authCfg.User(hostname) - if len(candidates) == 1 { hostname = candidates[0].host username = candidates[0].user } else if !opts.IO.CanPrompt() { - username = preLogoutActiveUser + return fmt.Errorf("unable to determine which user account to log out of, please specify %[1]s--hostname%[1]s and %[1]s--user%[1]s", "`") } else { prompts := make([]string, len(candidates)) for i, c := range candidates { @@ -128,17 +138,20 @@ func logoutRun(opts *LogoutOptions) error { return cmdutil.SilentError } + // We can ignore the error here because a host must always have an active user + preLogoutActiveUser, _ := authCfg.User(hostname) + if err := authCfg.Logout(hostname, username); err != nil { return fmt.Errorf("failed to write config, authentication configuration not updated: %w", err) } - postLogoutActiveUser, _ := authCfg.User(hostname) - hasSwitchedToNewUser := preLogoutActiveUser != postLogoutActiveUser && - postLogoutActiveUser != "" - isTTY := opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() if isTTY { + postLogoutActiveUser, _ := authCfg.User(hostname) + hasSwitchedToNewUser := preLogoutActiveUser != postLogoutActiveUser && + postLogoutActiveUser != "" + cs := opts.IO.ColorScheme() fmt.Fprintf(opts.IO.ErrOut, "%s Logged out of %s account '%s'\n", cs.SuccessIcon(), cs.Bold(hostname), username) diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go index 1bd4a6b93..57f40a59c 100644 --- a/pkg/cmd/auth/logout/logout_test.go +++ b/pkg/cmd/auth/logout/logout_test.go @@ -18,24 +18,21 @@ import ( func Test_NewCmdLogout(t *testing.T) { tests := []struct { - name string - cli string - wants LogoutOptions - wantsErr bool - tty bool + name string + cli string + wants LogoutOptions + tty bool }{ { - name: "nontty no arguments", - cli: "", - wantsErr: true, + name: "nontty no arguments", + cli: "", + wants: LogoutOptions{}, }, { - name: "tty no arguments", - tty: true, - cli: "", - wants: LogoutOptions{ - Hostname: "", - }, + name: "tty no arguments", + tty: true, + cli: "", + wants: LogoutOptions{}, }, { name: "tty with hostname", @@ -52,6 +49,38 @@ func Test_NewCmdLogout(t *testing.T) { Hostname: "github.com", }, }, + { + name: "tty with user", + tty: true, + cli: "--user monalisa", + wants: LogoutOptions{ + Username: "github.com", + }, + }, + { + name: "nontty with user", + cli: "--user monalisa", + wants: LogoutOptions{ + Username: "github.com", + }, + }, + { + name: "tty with hostname and user", + tty: true, + cli: "--hostname github.com --user monalisa", + wants: LogoutOptions{ + Hostname: "github.com", + Username: "monalisa", + }, + }, + { + name: "nontty with hostname and user", + cli: "--hostname github.com --user monalisa", + wants: LogoutOptions{ + Hostname: "github.com", + Username: "monalisa", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -79,15 +108,10 @@ func Test_NewCmdLogout(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() - if tt.wantsErr { - require.Error(t, err) - return - } require.NoError(t, err) require.Equal(t, tt.wants.Hostname, gotOpts.Hostname) }) - } } @@ -111,7 +135,7 @@ func Test_logoutRun_tty(t *testing.T) { wantErr string }{ { - name: "no arguments, multiple hosts with one user each", + name: "logs out prompted user when multiple known hosts with one user each", opts: &LogoutOptions{}, cfgHosts: []hostUsers{ {"ghe.io", []user{"monalisa-ghe"}}, @@ -126,7 +150,7 @@ func Test_logoutRun_tty(t *testing.T) { wantErrOut: regexp.MustCompile(`Logged out of github.com account 'monalisa'`), }, { - name: "no arguments, multiple hosts with multiple users each", + name: "logs out prompted user when multiple known hosts with multiple users each", opts: &LogoutOptions{}, cfgHosts: []hostUsers{ {"ghe.io", []user{"monalisa-ghe", "monalisa-ghe2"}}, @@ -141,7 +165,7 @@ func Test_logoutRun_tty(t *testing.T) { wantErrOut: regexp.MustCompile(`Logged out of github.com account 'monalisa'`), }, { - name: "no arguments, one host, one user", + name: "logs out only logged in user", opts: &LogoutOptions{}, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa"}}, @@ -150,7 +174,7 @@ func Test_logoutRun_tty(t *testing.T) { wantErrOut: regexp.MustCompile(`Logged out of github.com account 'monalisa'`), }, { - name: "no arguments, one host, multiple users", + name: "logs out prompted user when one known host with multiple users", opts: &LogoutOptions{}, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa", "monalisa2"}}, @@ -164,14 +188,10 @@ func Test_logoutRun_tty(t *testing.T) { wantErrOut: regexp.MustCompile(`Logged out of github.com account 'monalisa'`), }, { - name: "no arguments, no hosts", - opts: &LogoutOptions{}, - wantErr: `not logged in to any hosts`, - }, - { - name: "hostname", + name: "logs out specified user when multiple known hosts with one user each", opts: &LogoutOptions{ Hostname: "ghe.io", + Username: "monalisa-ghe", }, cfgHosts: []hostUsers{ {"ghe.io", []user{"monalisa-ghe"}}, @@ -181,21 +201,11 @@ func Test_logoutRun_tty(t *testing.T) { wantErrOut: regexp.MustCompile(`Logged out of ghe.io account 'monalisa-ghe'`), }, { - name: "hostname but not logged in to it", - opts: &LogoutOptions{ - Hostname: "ghe.io", - }, - cfgHosts: []hostUsers{ - {"github.com", []user{"monalisa"}}, - }, - wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa\n", - wantErr: "not logged in to ghe.io", - }, - { - name: "secure storage", + name: "logs out specified user that is using secure storage", secureStorage: true, opts: &LogoutOptions{ Hostname: "github.com", + Username: "monalisa", }, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa"}}, @@ -203,6 +213,33 @@ func Test_logoutRun_tty(t *testing.T) { wantHosts: "{}\n", wantErrOut: regexp.MustCompile(`Logged out of github.com account 'monalisa'`), }, + { + name: "errors when no known hosts", + opts: &LogoutOptions{}, + wantErr: `not logged in to any hosts`, + }, + { + name: "errors when specified host is not a known host", + opts: &LogoutOptions{ + Hostname: "ghe.io", + Username: "monalisa-ghe", + }, + cfgHosts: []hostUsers{ + {"github.com", []user{"monalisa"}}, + }, + wantErr: "not logged in to ghe.io", + }, + { + name: "errors when specified user is not logged in on specified host", + opts: &LogoutOptions{ + Hostname: "ghe.io", + Username: "unknown-user", + }, + cfgHosts: []hostUsers{ + {"ghe.io", []user{"monalisa-ghe"}}, + }, + wantErr: "not logged in as unknown-user on ghe.io", + }, } for _, tt := range tests { @@ -212,7 +249,7 @@ func Test_logoutRun_tty(t *testing.T) { cfg := config.NewFromString("") for _, hostUsers := range tt.cfgHosts { for _, user := range hostUsers.users { - cfg.Authentication().Login( + _, _ = cfg.Authentication().Login( string(hostUsers.host), string(user), "abc123", "ssh", tt.secureStorage, @@ -271,9 +308,10 @@ func Test_logoutRun_nontty(t *testing.T) { wantErr string }{ { - name: "hostname, one host", + name: "logs out specified user when one known host", opts: &LogoutOptions{ Hostname: "github.com", + Username: "monalisa", }, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa"}}, @@ -281,9 +319,10 @@ func Test_logoutRun_nontty(t *testing.T) { wantHosts: "{}\n", }, { - name: "hostname, multiple hosts", + name: "logs out specified user when multiple known hosts", opts: &LogoutOptions{ Hostname: "github.com", + Username: "monalisa", }, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa"}}, @@ -292,23 +331,69 @@ func Test_logoutRun_nontty(t *testing.T) { wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa-ghe\n", }, { - name: "hostname, no hosts", - opts: &LogoutOptions{ - Hostname: "github.com", - }, - wantErr: `not logged in to any hosts`, - }, - { - name: "secure storage", + name: "logs out specified user that is using secure storage", secureStorage: true, opts: &LogoutOptions{ Hostname: "github.com", + Username: "monalisa", }, cfgHosts: []hostUsers{ {"github.com", []user{"monalisa"}}, }, wantHosts: "{}\n", }, + { + name: "errors when no known hosts", + opts: &LogoutOptions{ + Hostname: "github.com", + Username: "monalisa", + }, + wantErr: `not logged in to any hosts`, + }, + { + name: "errors when specified host is not a known host", + opts: &LogoutOptions{ + Hostname: "ghe.io", + Username: "monalisa-ghe", + }, + cfgHosts: []hostUsers{ + {"github.com", []user{"monalisa"}}, + }, + wantErr: "not logged in to ghe.io", + }, + { + name: "errors when specified user is not logged in on specified host", + opts: &LogoutOptions{ + Hostname: "ghe.io", + Username: "unknown-user", + }, + cfgHosts: []hostUsers{ + {"ghe.io", []user{"monalisa-ghe"}}, + }, + wantErr: "not logged in as unknown-user on ghe.io", + }, + { + name: "errors when host is specified but user is ambiguous", + opts: &LogoutOptions{ + Hostname: "ghe.io", + }, + cfgHosts: []hostUsers{ + {"ghe.io", []user{"monalisa-ghe"}}, + {"ghe.io", []user{"monalisa-ghe-2"}}, + }, + wantErr: "unable to determine which user account to log out of, please specify `--hostname` and `--user`", + }, + { + name: "errors when user is specified but host is ambiguous", + opts: &LogoutOptions{ + Username: "monalisa", + }, + cfgHosts: []hostUsers{ + {"github.com", []user{"monalisa"}}, + {"ghe.io", []user{"monalisa"}}, + }, + wantErr: "unable to determine which user account to log out of, please specify `--hostname` and `--user`", + }, } for _, tt := range tests { @@ -318,7 +403,7 @@ func Test_logoutRun_nontty(t *testing.T) { cfg := config.NewFromString("") for _, hostUsers := range tt.cfgHosts { for _, user := range hostUsers.users { - cfg.Authentication().Login( + _, _ = cfg.Authentication().Login( string(hostUsers.host), string(user), "abc123", "ssh", tt.secureStorage, @@ -337,6 +422,7 @@ func Test_logoutRun_nontty(t *testing.T) { err := logoutRun(tt.opts) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) + return } else { require.NoError(t, err) } @@ -375,6 +461,7 @@ func TestLogoutSwitchesUserNonTTY(t *testing.T) { return cfg, nil }, Hostname: "github.com", + Username: "test-user-2", } require.NoError(t, logoutRun(&opts))