fix: MFA authentication by preventing session token keyring overwrite (#1757)

* fix: Prevent session tokens from overwriting long-lived credentials in keyring

Session tokens (temporary credentials with SessionToken field) should not be
cached in the keyring as they overwrite long-lived credentials needed for
subsequent authentication attempts. Session tokens are already persisted to
provider-specific storage (AWS files, etc.) and loaded via LoadCredentials().

This fixes the issue where users had to run `atmos auth user configure`
repeatedly because authentication would fail after the first attempt.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* fix: Display credential expiration times in user's local timezone

Convert parsed credential expiration times from UTC to local timezone
before returning them for display. This ensures consistency across all
time displays in auth commands (whoami, list) and matches user expectations.

Fixed in:
- pkg/auth/types/aws_credentials.go: Convert AWS credential expiration to local time
- pkg/auth/types/github_oidc_credentials.go: Convert OIDC token expiration to local time

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Andriy Knysh <[email protected]>

Author: 3 people

.gitignore

@@ -99,3 +99,6 @@ performance-optimization/
 *.bak
 *.go.bak
 **/*.go.bak
+
+# Scratch/temporary analysis files
+.scratch/

pkg/auth/manager.go

@@ -910,6 +910,17 @@ func (m *manager) getChainStepName(index int) string {
 	return "unknown"
 }
 
+// isSessionToken checks if credentials are temporary session tokens.
+// Session tokens are identified by the presence of a SessionToken field.
+// These should not be cached in keyring as they overwrite long-lived credentials.
+func isSessionToken(creds types.ICredentials) bool {
+	if awsCreds, ok := creds.(*types.AWSCredentials); ok {
+		return awsCreds.SessionToken != ""
+	}
+	// Add other credential types as needed.
+	return false
+}
+
 // authenticateIdentityChain performs sequential authentication through an identity chain.
 func (m *manager) authenticateIdentityChain(ctx context.Context, startIndex int, initialCreds types.ICredentials) (types.ICredentials, error) {
 	log.Debug("Authenticating identity chain", "chainLength", len(m.chain), "startIndex", startIndex, "chain", m.chain)
@@ -936,11 +947,19 @@ func (m *manager) authenticateIdentityChain(ctx context.Context, startIndex int,
 
 		currentCreds = nextCreds
 
-		// Cache credentials for this level.
-		if err := m.credentialStore.Store(identityStep, currentCreds); err != nil {
-			log.Debug("Failed to cache credentials", "identityStep", identityStep, "error", err)
+		// Cache credentials for this level, but skip session tokens.
+		// Session tokens are already persisted to provider-specific storage (e.g., AWS files)
+		// and can be loaded via identity.LoadCredentials().
+		// Caching session tokens in keyring would overwrite long-lived credentials
+		// that are needed for subsequent authentication attempts.
+		if isSessionToken(currentCreds) {
+			log.Debug("Skipping keyring cache for session tokens", "identityStep", identityStep)
 		} else {
-			log.Debug("Cached credentials", "identityStep", identityStep)
+			if err := m.credentialStore.Store(identityStep, currentCreds); err != nil {
+				log.Debug("Failed to cache credentials", "identityStep", identityStep, "error", err)
+			} else {
+				log.Debug("Cached credentials", "identityStep", identityStep)
+			}
 		}
 
 		log.Debug("Chained identity", "from", m.getChainStepName(i-1), "to", identityStep)

pkg/auth/manager_session_token_test.go

@@ -0,0 +1,178 @@
+package auth
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cloudposse/atmos/pkg/auth/types"
+	"github.com/cloudposse/atmos/pkg/schema"
+)
+
+// TestSessionTokenDoesNotOverwriteLongLivedCredentialsInKeyring verifies that session tokens
+// (temporary credentials) do NOT overwrite long-lived credentials in the keyring.
+//
+// Intended behavior:
+// - Session tokens should NOT be cached in keyring
+// - Long-lived credentials should remain in keyring unchanged
+// - Session tokens should only exist in provider-specific storage (AWS files, etc.)
+//
+// This ensures users don't need to reconfigure credentials after authentication,
+// as long-lived credentials remain available for generating new session tokens.
+func TestSessionTokenDoesNotOverwriteLongLivedCredentialsInKeyring(t *testing.T) {
+	ctx := context.Background()
+
+	// Step 1: Store long-lived AWS credentials in keyring.
+	// This simulates what `atmos auth user configure` does.
+	longLivedCreds := &types.AWSCredentials{
+		AccessKeyID:     "AKIA_LONG_LIVED_KEY",
+		SecretAccessKey: "long_lived_secret_key",
+		MfaArn:          "arn:aws:iam::123456789012:mfa/test-user",
+		SessionDuration: "12h",
+		Region:          "us-east-1",
+		// NO SessionToken - this is a long-lived credential
+	}
+
+	// Create test credential store with long-lived credentials.
+	// Pre-populate keyring with long-lived credentials for both provider and identity.
+	// This simulates what `atmos auth user configure` does.
+	store := &testStore{
+		data: map[string]any{
+			"test-provider": longLivedCreds, // Provider credentials (long-lived)
+			"test-identity": longLivedCreds, // Identity credentials (long-lived) - should NOT be overwritten
+		},
+		expired: map[string]bool{
+			"test-provider": false,
+			"test-identity": false,
+		},
+	}
+
+	// Step 2: Create a mock identity that returns session tokens.
+	// This simulates what AWS user identity does after calling STS GetSessionToken.
+	sessionCreds := &types.AWSCredentials{
+		AccessKeyID:     "ASIA_SESSION_KEY",
+		SecretAccessKey: "session_secret_key",
+		SessionToken:    "session_token_12345", // Session tokens have this field
+		Region:          "us-east-1",
+		Expiration:      "2099-12-31T23:59:59Z",
+	}
+
+	mockIdentity := &mockIdentityReturningSessionTokens{
+		sessionCreds: sessionCreds,
+	}
+
+	// Step 3: Create auth manager with minimal setup.
+	authConfig := &schema.AuthConfig{
+		Providers: map[string]schema.Provider{
+			"test-provider": {
+				Kind: "test",
+			},
+		},
+		Identities: map[string]schema.Identity{
+			"test-identity": {
+				Kind: "test",
+				Via: &schema.IdentityVia{
+					Provider: "test-provider",
+				},
+			},
+		},
+	}
+
+	m := &manager{
+		config:          authConfig,
+		credentialStore: store,
+		providers:       map[string]types.Provider{},
+		identities: map[string]types.Identity{
+			"test-identity": mockIdentity,
+		},
+		chain: []string{"test-provider", "test-identity"},
+	}
+
+	// Step 4: Authenticate through the identity chain.
+	// This returns session tokens but should NOT cache them in keyring.
+	returnedCreds, err := m.authenticateIdentityChain(ctx, 1, longLivedCreds)
+	require.NoError(t, err, "authenticateIdentityChain should succeed")
+
+	// Verify that session tokens were returned (this is correct and expected).
+	awsCreds, ok := returnedCreds.(*types.AWSCredentials)
+	require.True(t, ok, "Returned credentials should be AWS credentials")
+	assert.NotEmpty(t, awsCreds.SessionToken, "Should return session tokens")
+	assert.Equal(t, "ASIA_SESSION_KEY", awsCreds.AccessKeyID, "Should return session access key")
+
+	// Step 5: Verify keyring STILL contains long-lived credentials (INTENDED BEHAVIOR).
+	// Session tokens should NOT have been cached in keyring.
+	retrievedCreds, err := store.Retrieve("test-identity")
+	require.NoError(t, err, "Should retrieve credentials from keyring")
+
+	retrievedAWSCreds, ok := retrievedCreds.(*types.AWSCredentials)
+	require.True(t, ok, "Retrieved credentials should be AWS credentials")
+
+	// INTENDED BEHAVIOR: Keyring should NOT contain session tokens.
+	assert.Empty(t, retrievedAWSCreds.SessionToken,
+		"Keyring should NOT contain session tokens - they should only be in provider storage (AWS files)")
+
+	// INTENDED BEHAVIOR: Keyring should STILL contain long-lived credentials.
+	assert.Equal(t, "AKIA_LONG_LIVED_KEY", retrievedAWSCreds.AccessKeyID,
+		"Keyring should preserve long-lived access key")
+
+	assert.Equal(t, "long_lived_secret_key", retrievedAWSCreds.SecretAccessKey,
+		"Keyring should preserve long-lived secret key")
+
+	// INTENDED BEHAVIOR: Keyring should preserve MFA ARN and session duration.
+	assert.Equal(t, "arn:aws:iam::123456789012:mfa/test-user", retrievedAWSCreds.MfaArn,
+		"Keyring should preserve MFA ARN for future session token generation")
+
+	assert.Equal(t, "12h", retrievedAWSCreds.SessionDuration,
+		"Keyring should preserve session duration for future session token generation")
+}
+
+// mockIdentityReturningSessionTokens is a mock identity that returns session tokens.
+// This simulates what AWS user identity does after calling STS GetSessionToken.
+type mockIdentityReturningSessionTokens struct {
+	sessionCreds *types.AWSCredentials
+}
+
+func (m *mockIdentityReturningSessionTokens) Kind() string {
+	return "test"
+}
+
+func (m *mockIdentityReturningSessionTokens) Authenticate(ctx context.Context, baseCreds types.ICredentials) (types.ICredentials, error) {
+	// Return session tokens (credentials WITH SessionToken field).
+	// This simulates what generateSessionToken() does in AWS user identity.
+	return m.sessionCreds, nil
+}
+
+func (m *mockIdentityReturningSessionTokens) Validate() error {
+	return nil
+}
+
+func (m *mockIdentityReturningSessionTokens) Environment() (map[string]string, error) {
+	return map[string]string{}, nil
+}
+
+func (m *mockIdentityReturningSessionTokens) PrepareEnvironment(ctx context.Context, environ map[string]string) (map[string]string, error) {
+	return environ, nil
+}
+
+func (m *mockIdentityReturningSessionTokens) PostAuthenticate(ctx context.Context, params *types.PostAuthenticateParams) error {
+	return nil
+}
+
+func (m *mockIdentityReturningSessionTokens) CredentialsExist() (bool, error) {
+	return true, nil
+}
+
+func (m *mockIdentityReturningSessionTokens) LoadCredentials(ctx context.Context) (types.ICredentials, error) {
+	// Return session credentials from "storage" (simulates loading from AWS files).
+	return m.sessionCreds, nil
+}
+
+func (m *mockIdentityReturningSessionTokens) Logout(ctx context.Context) error {
+	return nil
+}
+
+func (m *mockIdentityReturningSessionTokens) GetProviderName() (string, error) {
+	return "test-provider", nil
+}

pkg/auth/types/aws_credentials.go

@@ -45,7 +45,9 @@ func (c *AWSCredentials) GetExpiration() (*time.Time, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%w: failed parsing AWS credential expiration: %w", errUtils.ErrInvalidAuthConfig, err)
 	}
-	return &expTime, nil
+	// Convert to local timezone for display to user.
+	localTime := expTime.Local()
+	return &localTime, nil
 }
 
 // BuildWhoamiInfo implements ICredentials for AWSCredentials.

pkg/auth/types/github_oidc_credentials.go

@@ -49,7 +49,8 @@ func (c *OIDCCredentials) GetExpiration() (*time.Time, error) {
 	if claims.Exp == 0 {
 		return nil, nil
 	}
-	t := time.Unix(claims.Exp, 0).UTC()
+	// Convert to local timezone for display to user.
+	t := time.Unix(claims.Exp, 0).Local()
 	return &t, nil
 }