Implemented auth logout for multiaccount

internal/config/auth_config_test.go

@@ -367,6 +367,57 @@ func TestLogoutRemovesHostAndKeyringToken(t *testing.T) {
 	require.ErrorContains(t, err, "secret not found in keyring")
 }
 
+func TestLogoutOfActiveUserSwitchesUserIfPossible(t *testing.T) {
+	// Given we have two accounts logged into a host
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "inactive-user", "test-token-1", "ssh", true)
+	require.NoError(t, err)
+
+	_, err = authCfg.Login("github.com", "active-user", "test-token-2", "https", true)
+	require.NoError(t, err)
+
+	// When we logout of the active user
+	err = authCfg.Logout("github.com", "active-user")
+
+	// Then we return success and the inactive user is now active
+	require.NoError(t, err)
+	activeUser, err := authCfg.User("github.com")
+	require.NoError(t, err)
+	require.Equal(t, "inactive-user", activeUser)
+
+	token, err := authCfg.TokenFromKeyring("github.com")
+	require.NoError(t, err)
+	require.Equal(t, "test-token-1", token)
+
+	usersForHost, err := authCfg.UsersForHost("github.com")
+	require.NoError(t, err)
+	require.NotContains(t, "active-user", usersForHost)
+}
+
+func TestLogoutOfInactiveUserDoesNotSwitchUser(t *testing.T) {
+	// Given we have two accounts logged into a host
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "inactive-user-1", "test-token-1.1", "ssh", true)
+	require.NoError(t, err)
+
+	_, err = authCfg.Login("github.com", "inactive-user-2", "test-token-1.2", "ssh", true)
+	require.NoError(t, err)
+
+	_, err = authCfg.Login("github.com", "active-user", "test-token-2", "https", true)
+	require.NoError(t, err)
+
+	// When we logout of an inactive user
+	err = authCfg.Logout("github.com", "inactive-user-1")
+
+	// Then we return success and the active user is still active
+	require.NoError(t, err)
+	activeUser, err := authCfg.User("github.com")
+	require.NoError(t, err)
+	require.Equal(t, "active-user", activeUser)
+}
+
 // Note that I'm not sure this test enforces particularly desirable behaviour
 // since it leads users to believe a token has been removed when really
 // that might have failed for some reason.
@@ -517,7 +568,7 @@ func TestTokenWorksRightAfterMigration(t *testing.T) {
 	require.Equal(t, oauthTokenKey, source)
 }
 
-func TestLogoutRigthAfterMigrationRemovesHost(t *testing.T) {
+func TestLogoutRightAfterMigrationRemovesHost(t *testing.T) {
 	// Given we have logged in before migration
 	authCfg := newTestAuthConfig(t)
 	host := "github.com"

internal/config/config.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 
 	"github.com/cli/cli/v2/internal/keyring"
 	ghAuth "github.com/cli/go-gh/v2/pkg/auth"
@@ -333,6 +334,7 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 	return insecureStorageUsed, c.switchUser(hostname, username)
 }
 
+// TODO: Verify that git protocol switching works as expected
 func (c *AuthConfig) switchUser(hostname, user string) error {
 	// We first need to idempotently clear out any set tokens for the host
 	_ = keyring.Delete(keyringServiceName(hostname), "")
@@ -367,10 +369,36 @@ func (c *AuthConfig) switchUser(hostname, user string) error {
 // Logout will remove user, git protocol, and auth token for the given hostname.
 // It will remove the auth token from the encrypted storage if it exists there.
 func (c *AuthConfig) Logout(hostname, username string) error {
-	_ = c.cfg.Remove([]string{hostsKey, hostname})
-	_ = keyring.Delete(keyringServiceName(hostname), "")
-	_ = keyring.Delete(keyringServiceName(hostname), username)
-	return ghConfig.Write(c.cfg)
+	// This error is ignorable because if there is no host then no logout is required
+	users, _ := c.UsersForHost(hostname)
+
+	// If there is only one (or zero) users, then we remove the host
+	// and unset the keyring tokens.
+	if len(users) < 2 {
+		_ = c.cfg.Remove([]string{hostsKey, hostname})
+		_ = keyring.Delete(keyringServiceName(hostname), "")
+		_ = keyring.Delete(keyringServiceName(hostname), username)
+		return ghConfig.Write(c.cfg)
+	}
+
+	// Otherwise, we remove the user from this host
+	_ = c.cfg.Remove([]string{hostsKey, hostname, usersKey, username})
+
+	// This error is ignorable because we already know there is an active user for the host
+	activeUser, _ := c.User(hostname)
+
+	// If the user we're removing isn't active, then we just write the config
+	if activeUser != username {
+		return ghConfig.Write(c.cfg)
+	}
+
+	// Otherwise we get the first user in the slice that isn't the user we're removing
+	switchUserIdx := slices.IndexFunc(users, func(n string) bool {
+		return n != username
+	})
+
+	// And switch to them
+	return c.switchUser(hostname, users[switchUserIdx])
 }
 
 func (c *AuthConfig) UsersForHost(hostname string) ([]string, error) {

internal/prompter/test.go

@@ -138,13 +138,13 @@ func IndexFor(options []string, answer string) (int, error) {
 			return ix, nil
 		}
 	}
-	return -1, NoSuchAnswerErr(answer)
+	return -1, NoSuchAnswerErr(answer, options)
 }
 
 func NoSuchPromptErr(prompt string) error {
 	return fmt.Errorf("no such prompt '%s'", prompt)
 }
 
-func NoSuchAnswerErr(answer string) error {
-	return fmt.Errorf("no such answer '%s'", answer)
+func NoSuchAnswerErr(answer string, options []string) error {
+	return fmt.Errorf("no such answer '%s' in [%s]", answer, strings.Join(options, ", "))
 }

pkg/cmd/auth/logout/logout.go

@@ -2,10 +2,9 @@ package logout
 
 import (
 	"fmt"
-	"net/http"
+	"slices"
 
 	"github.com/MakeNowJust/heredoc"
-	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/config"
 	"github.com/cli/cli/v2/pkg/cmd/auth/shared"
 	"github.com/cli/cli/v2/pkg/cmdutil"
@@ -14,19 +13,17 @@ import (
 )
 
 type LogoutOptions struct {
-	HttpClient func() (*http.Client, error)
-	IO         *iostreams.IOStreams
-	Config     func() (config.Config, error)
-	Prompter   shared.Prompt
-	Hostname   string
+	IO       *iostreams.IOStreams
+	Config   func() (config.Config, error)
+	Prompter shared.Prompt
+	Hostname string
 }
 
 func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Command {
 	opts := &LogoutOptions{
-		HttpClient: f.HttpClient,
-		IO:         f.IOStreams,
-		Config:     f.Config,
-		Prompter:   f.Prompter,
+		IO:       f.IOStreams,
+		Config:   f.Config,
+		Prompter: f.Prompter,
 	}
 
 	cmd := &cobra.Command{
@@ -64,41 +61,65 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co
 
 func logoutRun(opts *LogoutOptions) error {
 	hostname := opts.Hostname
+	// TODO: opts.Username likeley needed in the future
+	username := ""
 
 	cfg, err := opts.Config()
 	if err != nil {
 		return err
 	}
 	authCfg := cfg.Authentication()
 
-	candidates := authCfg.Hosts()
-	if len(candidates) == 0 {
+	knownHosts := authCfg.Hosts()
+	if len(knownHosts) == 0 {
 		return fmt.Errorf("not logged in to any hosts")
 	}
 
-	if hostname == "" {
-		if len(candidates) == 1 {
-			hostname = candidates[0]
-		} else {
-			selected, err := opts.Prompter.Select(
-				"What account do you want to log out of?", "", candidates)
-			if err != nil {
-				return fmt.Errorf("could not prompt: %w", err)
-			}
-			hostname = candidates[selected]
+	if hostname != "" {
+		if !slices.Contains(knownHosts, hostname) {
+			return fmt.Errorf("not logged in to %s", hostname)
 		}
-	} else {
-		var found bool
-		for _, c := range candidates {
-			if c == hostname {
-				found = true
-				break
-			}
+	}
+
+	type hostUser struct {
+		host string
+		user string
+	}
+	var candidates []hostUser
+
+	for _, host := range knownHosts {
+		if hostname != "" && host != hostname {
+			continue
 		}
+		knownUsers, err := cfg.Authentication().UsersForHost(host)
+		if err != nil {
+			return err
+		}
+		for _, user := range knownUsers {
+			candidates = append(candidates, hostUser{host: host, user: user})
+		}
+	}
 
-		if !found {
-			return fmt.Errorf("not logged into %s", hostname)
+	// 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
+	} else {
+		prompts := make([]string, len(candidates))
+		for i, c := range candidates {
+			prompts[i] = fmt.Sprintf("%s (%s)", c.user, c.host)
+		}
+		selected, err := opts.Prompter.Select(
+			"What account do you want to log out of?", "", prompts)
+		if err != nil {
+			return fmt.Errorf("could not prompt: %w", err)
 		}
+		hostname = candidates[selected].host
+		username = candidates[selected].user
 	}
 
 	if src, writeable := shared.AuthTokenWriteable(authCfg, hostname); !writeable {
@@ -107,34 +128,25 @@ func logoutRun(opts *LogoutOptions) error {
 		return cmdutil.SilentError
 	}
 
-	httpClient, err := opts.HttpClient()
-	if err != nil {
-		return err
-	}
-	apiClient := api.NewClientFromHTTP(httpClient)
-
-	username, err := api.CurrentLoginName(apiClient, hostname)
-	if err != nil {
-		// suppressing; the user is trying to delete this token and it might be bad.
-		// we'll see if the username is in the config and fall back to that.
-		username, _ = authCfg.User(hostname)
-	}
-
-	usernameStr := ""
-	if username != "" {
-		usernameStr = fmt.Sprintf(" account '%s'", username)
-	}
-
 	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 {
 		cs := opts.IO.ColorScheme()
-		fmt.Fprintf(opts.IO.ErrOut, "%s Logged out of %s%s\n",
-			cs.SuccessIcon(), cs.Bold(hostname), usernameStr)
+		fmt.Fprintf(opts.IO.ErrOut, "%s Logged out of %s account '%s'\n",
+			cs.SuccessIcon(), cs.Bold(hostname), username)
+
+		if hasSwitchedToNewUser {
+			fmt.Fprintf(opts.IO.ErrOut, "%s Switched account to '%s'\n",
+				cs.SuccessIcon(), cs.Bold(postLogoutActiveUser))
+		}
 	}
 
 	return nil