Merge pull request cli#11038 from anuraaga/active-token-user

andyfeller · web-flow · commit e5913fe02fb7 · 2025-06-27T08:27:36.000-04:00
fix: get token for active user instead of blank if possible
diff --git 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
@@ -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
@@ -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 {
