[PR #281] [MERGED] fix: merge credentials from keychain and config in auth status #404

New issue

Closed

opened 2026-02-26 22:30:47 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 22:30:47 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/281
Author: @rudrankriyam
Created: 1/29/2026
Status: ✅ Merged
Merged: 1/29/2026
Merged by: @rudrankriyam

Base: main ← Head: fix/auth-status-merge-credentials

📝 Commits (2)

d02dbc2 fix: merge credentials from keychain and config in auth status
8c58ba6 fix: normalize defaults and surface config warnings

📊 Changes

5 files changed (+267 additions, -24 deletions)

View changed files

📝 Agents.md (+7 -0)
📝 internal/auth/doctor.go (+29 -11)
📝 internal/auth/keychain.go (+100 -10)
📝 internal/auth/keychain_test.go (+118 -1)
📝 internal/cli/auth/auth.go (+13 -2)

📄 Description

Summary

Fix asc auth status to show credentials from both keychain and config file
Previously, if any keychain credentials existed, config-stored credentials were completely ignored
This caused credentials added with --bypass-keychain to be invisible
Also fixed test isolation bug where TestStoreCredentialsFallbackToConfig was writing to user's actual config

Problem

When running:

asc auth login --bypass-keychain --name client --key-id XXX --issuer-id YYY --private-key ~/.asc/AuthKey.p8

The credential was stored in config, but asc auth status only showed keychain credentials:

Stored credentials:
  - key-b5d62zk8ww (Key ID: B5D62ZK8WW) (stored in keychain)

Root Cause

ListCredentials() would return ONLY keychain credentials when any existed:

credentials, err := listFromKeychain()
if err == nil {
    if len(credentials) > 0 {
        return credentials, nil  // BUG: Never checks config
    }
}

Additionally, TestStoreCredentialsFallbackToConfig set HOME but not ASC_CONFIG_PATH, causing it to write test data to the local repo's .asc/config.json due to config path resolution order.

Solution

Modified ListCredentials() to merge credentials from both keychain and config
Keychain credentials take precedence when the same name exists in both
Fixed test to use ASC_CONFIG_PATH for proper isolation

Test plan

Added TestListCredentials_MergesKeychainAndConfig test
Fixed TestStoreCredentialsFallbackToConfig to use ASC_CONFIG_PATH
All existing tests pass
Manual verification: asc auth status now shows both keychain and config credentials

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/281 **Author:** [@rudrankriyam](https://github.com/rudrankriyam) **Created:** 1/29/2026 **Status:** ✅ Merged **Merged:** 1/29/2026 **Merged by:** [@rudrankriyam](https://github.com/rudrankriyam) **Base:** `main` ← **Head:** `fix/auth-status-merge-credentials` --- ### 📝 Commits (2) - [`d02dbc2`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/d02dbc21c37f167b5e0e8a3bd60727f43c995c1e) fix: merge credentials from keychain and config in auth status - [`8c58ba6`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/8c58ba6d7aebf389eebfb31e83d2e01f5e1b2a78) fix: normalize defaults and surface config warnings ### 📊 Changes **5 files changed** (+267 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `Agents.md` (+7 -0) 📝 `internal/auth/doctor.go` (+29 -11) 📝 `internal/auth/keychain.go` (+100 -10) 📝 `internal/auth/keychain_test.go` (+118 -1) 📝 `internal/cli/auth/auth.go` (+13 -2) </details> ### 📄 Description ## Summary - Fix `asc auth status` to show credentials from both keychain and config file - Previously, if any keychain credentials existed, config-stored credentials were completely ignored - This caused credentials added with `--bypass-keychain` to be invisible - Also fixed test isolation bug where `TestStoreCredentialsFallbackToConfig` was writing to user's actual config ## Problem When running: ```bash asc auth login --bypass-keychain --name client --key-id XXX --issuer-id YYY --private-key ~/.asc/AuthKey.p8 ``` The credential was stored in config, but `asc auth status` only showed keychain credentials: ``` Stored credentials: - key-b5d62zk8ww (Key ID: B5D62ZK8WW) (stored in keychain) ``` ## Root Cause `ListCredentials()` would return ONLY keychain credentials when any existed: ```go credentials, err := listFromKeychain() if err == nil { if len(credentials) > 0 { return credentials, nil // BUG: Never checks config } } ``` Additionally, `TestStoreCredentialsFallbackToConfig` set `HOME` but not `ASC_CONFIG_PATH`, causing it to write test data to the local repo's `.asc/config.json` due to config path resolution order. ## Solution 1. Modified `ListCredentials()` to merge credentials from both keychain and config 2. Keychain credentials take precedence when the same name exists in both 3. Fixed test to use `ASC_CONFIG_PATH` for proper isolation ## Test plan - [x] Added `TestListCredentials_MergesKeychainAndConfig` test - [x] Fixed `TestStoreCredentialsFallbackToConfig` to use `ASC_CONFIG_PATH` - [x] All existing tests pass - [x] Manual verification: `asc auth status` now shows both keychain and config credentials --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>