From 848cedd2c801a4f83ae10008f053af2cbcdf0d81 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Wed, 18 Jun 2025 09:56:44 +0900 Subject: [PATCH] Push up --- internal/config/auth_config_test.go | 106 ++++++++++++++-------------- internal/config/config.go | 20 +++--- 2 files changed, 64 insertions(+), 62 deletions(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 4e220cbbf..f685b9527 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -42,59 +42,6 @@ func TestTokenFromKeyringForUser(t *testing.T) { require.Equal(t, "test-token", token) } -func TestTokenFromKeyringPrioritizesActiveUserToken(t *testing.T) { - // Given a keyring that contains a token for a host - authCfg := newTestAuthConfig(t) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "", "test-token")) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user1", "test-token")) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user2", "test-token2")) - - // When we get the token from the auth config - token, err := authCfg.TokenFromKeyring("github.com") - - // Then it returns successfully with the correct token - require.NoError(t, err) - require.Equal(t, "test-token", token) - - // When we set the active user to test-user1 - authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user1") - - // And get the token from the auth config - token, err = authCfg.TokenFromKeyring("github.com") - - // Then it returns successfully with the correct token - require.NoError(t, err) - require.Equal(t, "test-token", token) - - // When we set the active user to test-user2 - authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user2") - - // And get the token from the auth config - token, err = authCfg.TokenFromKeyring("github.com") - - // Then it returns successfully with the correct token - require.NoError(t, err) - require.Equal(t, "test-token2", token) -} - -func TestTokenFromKeyringActiveUserNotInKeyringFallsBackToBlank(t *testing.T) { - // Given a keyring that contains a token for a host - authCfg := newTestAuthConfig(t) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "", "test-token")) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user1", "test-token1")) - require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user2", "test-token2")) - - // When we set the active user to test-user3 - authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user3") - - // And get the token from the auth config - token, err := authCfg.TokenFromKeyring("github.com") - - // Then it returns successfully with the fallback token - require.NoError(t, err) - require.Equal(t, "test-token", token) -} - func TestTokenFromKeyringForUserErrorsIfUsernameIsBlank(t *testing.T) { authCfg := newTestAuthConfig(t) @@ -822,6 +769,59 @@ func TestTokenWorksRightAfterMigration(t *testing.T) { require.Equal(t, oauthTokenKey, source) } +func TestTokenPrioritizesActiveUserToken(t *testing.T) { + // Given a keyring that contains a token for a host + authCfg := newTestAuthConfig(t) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "", "test-token")) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user1", "test-token")) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user2", "test-token2")) + + // When we get the token from the auth config + token, source := authCfg.ActiveToken("github.com") + + // Then it returns successfully with the correct token + require.Equal(t, "keyring", source) + require.Equal(t, "test-token", token) + + // When we set the active user to test-user1 + authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user1") + + // And get the token from the auth config + token, source = authCfg.ActiveToken("github.com") + + // Then it returns successfully with the correct token + require.Equal(t, "keyring", source) + require.Equal(t, "test-token", token) + + // When we set the active user to test-user2 + authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user2") + + // And get the token from the auth config + token, source = authCfg.ActiveToken("github.com") + + // Then it returns successfully with the correct token + require.Equal(t, source, "keyring") + require.Equal(t, "test-token2", token) +} + +func TestTokenWithActiveUserNotInKeyringFallsBackToBlank(t *testing.T) { + // Given a keyring that contains a token for a host + authCfg := newTestAuthConfig(t) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "", "test-token")) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user1", "test-token1")) + require.NoError(t, keyring.Set(keyringServiceName("github.com"), "test-user2", "test-token2")) + + // When we set the active user to test-user3 + authCfg.cfg.Set([]string{hostsKey, "github.com", userKey}, "test-user3") + + // And get the token from the auth config + token, source := authCfg.ActiveToken("github.com") + + // Then it returns successfully with the fallback token + require.Equal(t, "keyring", source) + require.Equal(t, "test-token", token) +} + func TestLogoutRightAfterMigrationRemovesHost(t *testing.T) { // Given we have logged in before migration authCfg := newTestAuthConfig(t) diff --git a/internal/config/config.go b/internal/config/config.go index 3e652079a..47d9403b0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -234,8 +234,18 @@ func (c *AuthConfig) ActiveToken(hostname string) (string, string) { } token, source := ghauth.TokenFromEnvOrConfig(hostname) if token == "" { + var user string var err error - token, err = c.TokenFromKeyring(hostname) + if user, err = c.ActiveUser(hostname); err == nil { + token, err = c.TokenFromKeyringForUser(hostname, user) + } + if err != nil { + // We should generally be able to find a token for the active user, + // but in some cases such as if the keyring was set up in a very old + // version of the CLI, it may only have a unkeyed token, so fallback + // to it. + token, err = c.TokenFromKeyring(hostname) + } if err == nil { source = "keyring" } @@ -281,14 +291,6 @@ func (c *AuthConfig) SetActiveToken(token, source string) { // TokenFromKeyring will retrieve the auth token for the given hostname, // only searching in encrypted storage. func (c *AuthConfig) TokenFromKeyring(hostname string) (string, error) { - if user, err := c.ActiveUser(hostname); err == nil && user != "" { - // Prioritize the user-specific token if it exists, which may be - // different from the blank active token, for example if a user uses - // GH_CONFIG_DIR to point to a different config directory. - if tok, err := c.TokenFromKeyringForUser(hostname, user); err == nil && tok != "" { - return tok, nil - } - } return keyring.Get(keyringServiceName(hostname), "") }