diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 61245c650..ca5f7e584 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -769,6 +769,62 @@ func TestTokenWorksRightAfterMigration(t *testing.T) { require.Equal(t, oauthTokenKey, source) } +func TestTokenPrioritizesActiveUserToken(t *testing.T) { + // Given a keyring where the active slot contains the token from a previous user + 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 no active user is set + authCfg.cfg.Remove([]string{hostsKey, "github.com", userKey}) + + // And get the token from the auth config + token, source := authCfg.ActiveToken("github.com") + + // Then it returns the token from the keyring active slot + 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 the token from the active user entry in the keyring + 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 the token from the active user entry in the keyring + require.Equal(t, "keyring", source) + 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 003a0ca17..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" } diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 321f7b7c0..bf4d51c4c 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -1358,7 +1358,12 @@ func Test_apiRun_cache(t *testing.T) { Config: func() (gh.Config, error) { return &ghmock.ConfigMock{ AuthenticationFunc: func() gh.AuthConfig { - return &config.AuthConfig{} + cfg := &config.AuthConfig{} + // Required because the http client tries to get the active token and otherwise + // this goes down to to go-gh config and panics. Pretty bad solution, it would + // be better if this were black box. + cfg.SetActiveToken("token", "stub") + return cfg }, // Cached responses are stored in a tempdir that gets automatically cleaned up CacheDirFunc: func() string {