Skip to content

Commit 4219b23

Browse files
ostermanclaude
andcommitted
fix: Consolidate credential retrieval logic to fix terraform auth
This commit fixes a bug where `atmos terraform plan` and other Terraform commands failed to use file-based credentials, while `atmos auth whoami` and similar commands worked correctly. The root cause was duplicate credential retrieval code across three methods with inconsistent fallback behavior. ## Problem Three separate code paths retrieved credentials: 1. `GetCachedCredentials` - Had keyring → identity storage fallback ✓ 2. `findFirstValidCachedCredentials` - Had fallback logic ✓ 3. `retrieveCachedCredentials` - NO fallback (the bug!) ✗ When users authenticated via AWS SSO (credentials in files, not keyring), Terraform commands would fail because only `retrieveCachedCredentials` was used in that code path, and it didn't have fallback logic. ## Solution Extracted a shared `retrieveCredentialWithFallback` method that implements the single source of truth for credential retrieval. All three code paths now delegate to this method, ensuring consistent behavior: - Tries keyring cache first (fast path) - Falls back to identity storage if not in keyring (slow path) - Provides detailed logging and error handling ## Changes - Added `retrieveCredentialWithFallback()` method (38 lines) - Refactored `GetCachedCredentials()` (40% code reduction) - Refactored `findFirstValidCachedCredentials()` (57% code reduction) - Refactored `retrieveCachedCredentials()` (now uses shared method) - Fixed `TestManager_GetCachedCredentials_Paths` to use proper test data ## Result ✅ All three credential retrieval paths now behave identically ✅ Terraform commands work with file-based credentials ✅ ~110 lines of duplicate code eliminated ✅ All tests pass ✅ Regression test included to prevent this class of bug Fixes the issue where Terraform commands failed with valid session credentials. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 1eda3a2 commit 4219b23

File tree

13 files changed

+687
-136
lines changed

13 files changed

+687
-136
lines changed

cmd/auth_console.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func executeAuthConsoleCommand(cmd *cobra.Command, args []string) error {
7676
// Try to use cached credentials first (passive check, no prompts).
7777
// Only authenticate if cached credentials are not available or expired.
7878
ctx := context.Background()
79-
whoami, err := authManager.GetCachedCredentials(ctx, identityName)
79+
whoami, err := authManager.DescribeIdentity(ctx, identityName)
8080
if err != nil {
8181
log.Debug("No valid cached credentials found, authenticating", "identity", identityName, "error", err)
8282
// No valid cached credentials - perform full authentication.

cmd/auth_console_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ func (m *mockAuthManagerForProvider) GetProviderKindForIdentity(identityName str
585585
return m.providerKind, nil
586586
}
587587

588-
func (m *mockAuthManagerForProvider) GetCachedCredentials(ctx context.Context, identityName string) (*types.WhoamiInfo, error) {
588+
func (m *mockAuthManagerForProvider) DescribeIdentity(ctx context.Context, identityName string) (*types.WhoamiInfo, error) {
589589
return nil, errors.New("not implemented")
590590
}
591591

@@ -667,7 +667,7 @@ func (m *mockAuthManagerForIdentity) GetProviderKindForIdentity(identityName str
667667
return "", errors.New("not implemented")
668668
}
669669

670-
func (m *mockAuthManagerForIdentity) GetCachedCredentials(ctx context.Context, identityName string) (*types.WhoamiInfo, error) {
670+
func (m *mockAuthManagerForIdentity) DescribeIdentity(ctx context.Context, identityName string) (*types.WhoamiInfo, error) {
671671
return nil, errors.New("not implemented")
672672
}
673673

cmd/auth_env.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ var authEnvCmd = &cobra.Command{
6464
// Try to use cached credentials first (passive check, no prompts).
6565
// Only authenticate if cached credentials are not available or expired.
6666
ctx := cmd.Context()
67-
whoami, err := authManager.GetCachedCredentials(ctx, identityName)
67+
whoami, err := authManager.DescribeIdentity(ctx, identityName)
6868
if err != nil {
6969
log.Debug("No valid cached credentials found, authenticating", "identity", identityName, "error", err)
7070
// No valid cached credentials - perform full authentication.

cmd/auth_exec.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func executeAuthExecCommandCore(cmd *cobra.Command, args []string) error {
7777
// Try to use cached credentials first (passive check, no prompts).
7878
// Only authenticate if cached credentials are not available or expired.
7979
ctx := context.Background()
80-
whoami, err := authManager.GetCachedCredentials(ctx, identityName)
80+
whoami, err := authManager.DescribeIdentity(ctx, identityName)
8181
if err != nil {
8282
log.Debug("No valid cached credentials found, authenticating", "identity", identityName, "error", err)
8383
// No valid cached credentials - perform full authentication.

cmd/auth_shell.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func executeAuthShellCommandCore(cmd *cobra.Command, args []string) error {
8686
// Try to use cached credentials first (passive check, no prompts).
8787
// Only authenticate if cached credentials are not available or expired.
8888
ctx := context.Background()
89-
whoami, err := authManager.GetCachedCredentials(ctx, identityName)
89+
whoami, err := authManager.DescribeIdentity(ctx, identityName)
9090
if err != nil {
9191
log.Debug("No valid cached credentials found, authenticating", "identity", identityName, "error", err)
9292
// No valid cached credentials - perform full authentication.

0 commit comments

Comments
 (0)