From 8cdbc1a8ca7bda3a70a9f90602e8bc0420783268 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 4 Dec 2023 14:52:11 -0400 Subject: [PATCH] Refactor authCfg.UsersForHost to not return an error --- internal/config/auth_config_test.go | 12 +++++------- internal/config/config.go | 9 ++++----- pkg/cmd/auth/logout/logout.go | 7 ++----- pkg/cmd/auth/shared/login_flow.go | 4 ++-- pkg/cmd/auth/shared/login_flow_test.go | 4 ++-- pkg/cmd/auth/status/status.go | 2 +- pkg/cmd/auth/switch/switch.go | 7 ++----- 7 files changed, 18 insertions(+), 27 deletions(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 944bcea65..bd934f506 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -379,8 +379,7 @@ func TestLogoutOfActiveUserSwitchesUserIfPossible(t *testing.T) { require.NoError(t, err) require.Equal(t, "test-token-1", token) - usersForHost, err := authCfg.UsersForHost("github.com") - require.NoError(t, err) + usersForHost := authCfg.UsersForHost("github.com") require.NotContains(t, "active-user", usersForHost) } @@ -581,10 +580,10 @@ func TestUsersForHostNoHost(t *testing.T) { authCfg := newTestAuthConfig(t) // When we get the users for a host that doesn't exist - _, err := authCfg.UsersForHost("github.com") + users := authCfg.UsersForHost("github.com") - // Then it returns an error - require.EqualError(t, err, "unknown host: github.com") + // Then it returns nil + require.Nil(t, users) } func TestUsersForHostWithUsers(t *testing.T) { @@ -596,10 +595,9 @@ func TestUsersForHostWithUsers(t *testing.T) { require.NoError(t, err) // When we get the users for that host - users, err := authCfg.UsersForHost("github.com") + users := authCfg.UsersForHost("github.com") // Then it succeeds and returns the users - require.NoError(t, err) require.Equal(t, []string{"test-user-1", "test-user-2"}, users) } diff --git a/internal/config/config.go b/internal/config/config.go index 8dadf03e8..d9d4f6692 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -388,8 +388,7 @@ 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 { - // This error is ignorable because if there is no host then no logout is required - users, _ := c.UsersForHost(hostname) + users := c.UsersForHost(hostname) // If there is only one (or zero) users, then we remove the host // and unset the keyring tokens. @@ -453,13 +452,13 @@ func (c *AuthConfig) activateUser(hostname, user string) error { return ghConfig.Write(c.cfg) } -func (c *AuthConfig) UsersForHost(hostname string) ([]string, error) { +func (c *AuthConfig) UsersForHost(hostname string) []string { users, err := c.cfg.Keys([]string{hostsKey, hostname, usersKey}) if err != nil { - return nil, fmt.Errorf("unknown host: %s", hostname) + return nil } - return users, nil + return users } func (c *AuthConfig) TokenForUser(hostname, user string) (string, string, error) { diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index a47435dad..c1755c715 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -84,7 +84,7 @@ func logoutRun(opts *LogoutOptions) error { } if username != "" { - knownUsers, _ := cfg.Authentication().UsersForHost(hostname) + knownUsers := cfg.Authentication().UsersForHost(hostname) if !slices.Contains(knownUsers, username) { return fmt.Errorf("not logged in to %s account %s", hostname, username) } @@ -101,10 +101,7 @@ func logoutRun(opts *LogoutOptions) error { if hostname != "" && host != hostname { continue } - knownUsers, err := cfg.Authentication().UsersForHost(host) - if err != nil { - return err - } + knownUsers := cfg.Authentication().UsersForHost(host) for _, user := range knownUsers { if username != "" && user != username { continue diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 9e3f369af..7bb974547 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -24,7 +24,7 @@ const defaultSSHKeyTitle = "GitHub CLI" type iconfig interface { Login(string, string, string, string, bool) (bool, error) - UsersForHost(string) ([]string, error) + UsersForHost(string) []string } type LoginOptions struct { @@ -191,7 +191,7 @@ func Login(opts *LoginOptions) error { // In this case we ignore the error if the host doesn't exist // because that can occur when the user is logging into a host // for the first time. - usersForHost, _ := cfg.UsersForHost(hostname) + usersForHost := cfg.UsersForHost(hostname) userWasAlreadyLoggedIn := slices.Contains(usersForHost, username) if gitProtocol != "" { diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index b14d2228c..263ecc3d8 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -25,8 +25,8 @@ func (c tinyConfig) Login(host, username, token, gitProtocol string, encrypt boo return false, nil } -func (c tinyConfig) UsersForHost(hostname string) ([]string, error) { - return nil, nil +func (c tinyConfig) UsersForHost(hostname string) []string { + return nil } func TestLogin_ssh(t *testing.T) { diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 2caef6700..d2713aeac 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -205,7 +205,7 @@ func statusRun(opts *StatusOptions) error { }) statuses[hostname] = append(statuses[hostname], entry) - users, _ := authCfg.UsersForHost(hostname) + users := authCfg.UsersForHost(hostname) for _, username := range users { if username == activeUser { continue diff --git a/pkg/cmd/auth/switch/switch.go b/pkg/cmd/auth/switch/switch.go index c705291a6..bac323cf3 100644 --- a/pkg/cmd/auth/switch/switch.go +++ b/pkg/cmd/auth/switch/switch.go @@ -99,7 +99,7 @@ func switchRun(opts *SwitchOptions) error { } if username != "" { - knownUsers, _ := cfg.Authentication().UsersForHost(hostname) + knownUsers := cfg.Authentication().UsersForHost(hostname) if !slices.Contains(knownUsers, username) { return fmt.Errorf("not logged in to %s account %s", hostname, username) } @@ -116,10 +116,7 @@ func switchRun(opts *SwitchOptions) error { if err != nil { return err } - knownUsers, err := cfg.Authentication().UsersForHost(host) - if err != nil { - return err - } + knownUsers := cfg.Authentication().UsersForHost(host) for _, user := range knownUsers { if username != "" && user != username { continue