From 68e30beac45269adefa506794ae4057b4fc50783 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 1 Nov 2023 12:21:31 +0100 Subject: [PATCH] Logout removes token from keyring using username --- internal/config/auth_config_test.go | 24 +++++++++++++++++------- internal/config/config.go | 3 ++- pkg/cmd/auth/logout/logout.go | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 85e5c8d23..3cbf71311 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -340,17 +340,23 @@ func TestLogoutRemovesHostAndKeyringToken(t *testing.T) { // Given we are logged into a host keyring.MockInit() authCfg := newTestAuthConfig(t) - _, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", true) + host := "github.com" + user := "test-user" + token := "test-token" + + _, err := authCfg.Login(host, user, token, "ssh", true) require.NoError(t, err) // When we logout - err = authCfg.Logout("github.com") + err = authCfg.Logout(host, user) // Then we return success, and the host and token are removed from the config and keyring require.NoError(t, err) - requireNoKey(t, authCfg.cfg, []string{hostsKey, "github.com"}) - _, err = keyring.Get(keyringServiceName("github.com"), "") + requireNoKey(t, authCfg.cfg, []string{hostsKey, host}) + _, err = keyring.Get(keyringServiceName(host), "") + require.ErrorContains(t, err, "secret not found in keyring") + _, err = keyring.Get(keyringServiceName(host), user) require.ErrorContains(t, err, "secret not found in keyring") } @@ -368,7 +374,7 @@ func TestLogoutIgnoresErrorsFromConfigAndKeyring(t *testing.T) { authCfg := newTestAuthConfig(t) // When we logout - err := authCfg.Logout("github.com") + err := authCfg.Logout("github.com", "test-user") // Then it returns success anyway, suppressing the errors require.NoError(t, err) @@ -475,14 +481,18 @@ func TestTokenWorksRightAfterMigration(t *testing.T) { func TestLogoutRigthAfterMigrationRemovesHost(t *testing.T) { // Given we have logged in before migration authCfg := newTestAuthConfig(t) - _, err := preMigrationLogin(authCfg, "github.com", "test-user", "test-token", "ssh", false) + host := "github.com" + user := "test-user" + token := "test-token" + + _, err := preMigrationLogin(authCfg, host, user, token, "ssh", false) require.NoError(t, err) // When we migrate and logout var m migration.MultiAccount require.NoError(t, Migrate(authCfg.cfg, m)) - require.NoError(t, authCfg.Logout("github.com")) + require.NoError(t, authCfg.Logout(host, user)) // Then the host is removed from the config requireNoKey(t, authCfg.cfg, []string{hostsKey, "github.com"}) diff --git a/internal/config/config.go b/internal/config/config.go index 123a20a91..45437885d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -286,9 +286,10 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure // 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 string) error { +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) } diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index b2afc9a6c..e3be3e678 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -125,7 +125,7 @@ func logoutRun(opts *LogoutOptions) error { usernameStr = fmt.Sprintf(" account '%s'", username) } - if err := authCfg.Logout(hostname); err != nil { + if err := authCfg.Logout(hostname, username); err != nil { return fmt.Errorf("failed to write config, authentication configuration not updated: %w", err) }