From 8e89af96e8cfb5efb8558b8ab370e7b627ab8cb6 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 1 Dec 2023 11:31:22 +0100 Subject: [PATCH] Write tests for SwitchUser --- internal/config/auth_config_test.go | 122 ++++++++++++++++++++++++++++ internal/config/config.go | 8 +- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index d43a14e51..349713da1 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -418,6 +418,128 @@ func TestLogoutIgnoresErrorsFromConfigAndKeyring(t *testing.T) { require.NoError(t, err) } +func TestSwitchUserMakesSecureTokenActive(t *testing.T) { + // Given we have a user with a secure token + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", true) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", true) + require.NoError(t, err) + + // When we switch to that user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Their secure token is now active + token, err := authCfg.TokenFromKeyring("github.com") + require.NoError(t, err) + require.Equal(t, "test-token-1", token) +} + +func TestSwitchUserMakesInsecureTokenActive(t *testing.T) { + // Given we have a user with an insecure token + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", false) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", false) + require.NoError(t, err) + + // When we switch to that user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Their insecure token is now active + token, source := authCfg.Token("github.com") + require.Equal(t, "test-token-1", token) + require.Equal(t, oauthTokenKey, source) +} + +func TestSwitchUserUpdatesTheActiveUser(t *testing.T) { + // Given we have two users logged into a host + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", false) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", false) + require.NoError(t, err) + + // When we switch to the other user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Then the active user is updated + activeUser, err := authCfg.User("github.com") + require.NoError(t, err) + require.Equal(t, "test-user-1", activeUser) +} + +// TODO: This might be removed +func TestSwitchUserUpdatesTheHostLevelGitProtocol(t *testing.T) { + // Given we have two users logged into a host + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", false) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "https", false) + require.NoError(t, err) + + // When we switch to the other user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Then the host level git protocol is updated + requireKeyWithValue(t, authCfg.cfg, []string{hostsKey, "github.com", gitProtocolKey}, "ssh") +} + +func TestSwitchUserErrorsIfNoTokenMadeActive(t *testing.T) { + // Given we have a user but no token can be found (because we deleted them, simulating an error case) + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", true) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", true) + require.NoError(t, err) + + keyring.Delete(keyringServiceName("github.com"), "test-user-1") + + // When we switch to the user + err = authCfg.SwitchUser("github.com", "test-user-1") + + // Then it returns an error + require.EqualError(t, err, "no token found for 'test-user-1'") +} + +func TestSwitchClearsActiveSecureTokenWhenSwitchingToInsecureUser(t *testing.T) { + // Given we have an active secure token + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", false) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", true) + require.NoError(t, err) + + // When we switch to an insecure user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Then the active secure token is cleared + _, err = authCfg.TokenFromKeyring("github.com") + require.Error(t, err) +} + +func TestSwitchClearsActiveInsecureTokenWhenSwitchingToSecureUser(t *testing.T) { + // Given we have an active insecure token + keyring.MockInit() + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", true) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "ssh", false) + require.NoError(t, err) + + // When we switch to a secure user + require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) + + // Then the active insecure token is cleared + requireNoKey(t, authCfg.cfg, []string{hostsKey, "github.com", oauthTokenKey}) +} + func TestUsersForHostNoHost(t *testing.T) { // Given we have a config with no hosts authCfg := newTestAuthConfig(t) diff --git a/internal/config/config.go b/internal/config/config.go index c30f4e294..7d30623be 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -334,7 +334,6 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure return insecureStorageUsed, c.SwitchUser(hostname, username) } -// TODO: Write tests // TODO: How should git protocol be handled? Do we need to set it at the user level since it could have been changed? func (c *AuthConfig) SwitchUser(hostname, user string) error { // We first need to idempotently clear out any set tokens for the host @@ -345,15 +344,22 @@ func (c *AuthConfig) SwitchUser(hostname, user string) error { // following branches should be true. // If there is a token in the secure keyring for the user, move it to the active slot + var tokenSwitched bool if token, err := keyring.Get(keyringServiceName(hostname), user); err == nil { if err = keyring.Set(keyringServiceName(hostname), "", token); err != nil { return fmt.Errorf("failed to move active token in keyring: %v", err) } + tokenSwitched = true } // If there is a token in the insecure config for the user, move it to the active field if token, err := c.cfg.Get([]string{hostsKey, hostname, usersKey, user, oauthTokenKey}); err == nil { c.cfg.Set([]string{hostsKey, hostname, oauthTokenKey}, token) + tokenSwitched = true + } + + if !tokenSwitched { + return fmt.Errorf("no token found for '%s'", user) } // Then we'll ensure the git protocol is moved as well