diff --git a/cmd/auth_list.go b/cmd/auth_list.go index 96c0950100..858b5eb1bc 100644 --- a/cmd/auth_list.go +++ b/cmd/auth_list.go @@ -8,14 +8,17 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/viper" "gopkg.in/yaml.v3" errUtils "github.com/cloudposse/atmos/errors" authList "github.com/cloudposse/atmos/pkg/auth/list" authTypes "github.com/cloudposse/atmos/pkg/auth/types" cfg "github.com/cloudposse/atmos/pkg/config" + "github.com/cloudposse/atmos/pkg/flags" log "github.com/cloudposse/atmos/pkg/logger" "github.com/cloudposse/atmos/pkg/perf" + "github.com/cloudposse/atmos/pkg/profile" "github.com/cloudposse/atmos/pkg/schema" ) @@ -136,7 +139,7 @@ func executeAuthListCommand(cmd *cobra.Command, args []string) error { } // Load auth manager. - authManager, err := loadAuthManagerForList() + authManager, atmosConfig, err := loadAuthManagerForList(cmd) if err != nil { return err } @@ -151,6 +154,15 @@ func executeAuthListCommand(cmd *cobra.Command, args []string) error { return err } + // When no providers or identities are configured, check if profiles exist + // that might contain auth configuration and suggest using --profile. + // Use the unfiltered lists to avoid false positives when filters narrow results to empty. + if len(providers) == 0 && len(identities) == 0 { + if suggestion := suggestProfilesForAuth(atmosConfig); suggestion != nil { + return suggestion + } + } + // Get output format. format, _ := cmd.Flags().GetString("format") @@ -364,19 +376,54 @@ func renderYAML(providers map[string]schema.Provider, identities map[string]sche return string(data), nil } -// loadAuthManager loads the auth manager (helper from auth_whoami.go). -func loadAuthManagerForList() (authTypes.AuthManager, error) { +// suggestProfilesForAuth checks if profiles exist and returns a helpful error +// suggesting the user try --profile when no auth providers/identities are configured. +func suggestProfilesForAuth(atmosConfig *schema.AtmosConfiguration) error { + defer perf.Track(atmosConfig, "cmd.suggestProfilesForAuth")() + + mgr := profile.NewProfileManager() + profiles, err := mgr.ListProfiles(atmosConfig) + if err != nil || len(profiles) == 0 { + return nil + } + + // Collect profile names. + profileNames := make([]string, 0, len(profiles)) + for _, p := range profiles { + profileNames = append(profileNames, p.Name) + } + + return errUtils.Build(errUtils.ErrAuthNotConfigured). + WithExplanation("No authentication providers or identities are configured in the current configuration"). + WithHintf("Available profiles: `%s`", strings.Join(profileNames, "`, `")). + WithHint("Try: `atmos auth list --profile ` to load auth configuration from a profile"). + WithHint("Run `atmos profile list` for detailed information about each profile"). + WithExitCode(1). + Err() +} + +// loadAuthManagerForList loads the auth manager and returns the atmos config for profile discovery. +func loadAuthManagerForList(cmd *cobra.Command) (authTypes.AuthManager, *schema.AtmosConfiguration, error) { defer perf.Track(nil, "cmd.loadAuthManagerForList")() - atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false) + v := viper.GetViper() + globalFlags := flags.ParseGlobalFlags(cmd, v) + configAndStacksInfo := schema.ConfigAndStacksInfo{ + AtmosBasePath: globalFlags.BasePath, + AtmosConfigFilesFromArg: globalFlags.Config, + AtmosConfigDirsFromArg: globalFlags.ConfigPath, + ProfilesFromArg: globalFlags.Profile, + } + + atmosConfig, err := cfg.InitCliConfig(configAndStacksInfo, false) if err != nil { - return nil, fmt.Errorf("%w: failed to load atmos config: %w", errUtils.ErrInvalidAuthConfig, err) + return nil, nil, fmt.Errorf("%w: failed to load atmos config: %w", errUtils.ErrInvalidAuthConfig, err) } manager, err := createAuthManager(&atmosConfig.Auth, atmosConfig.CliConfigPath) if err != nil { - return nil, fmt.Errorf("%w: failed to create auth manager: %w", errUtils.ErrInvalidAuthConfig, err) + return nil, nil, fmt.Errorf("%w: failed to create auth manager: %w", errUtils.ErrInvalidAuthConfig, err) } - return manager, nil + return manager, &atmosConfig, nil } diff --git a/cmd/auth_list_test.go b/cmd/auth_list_test.go index f45a8803e2..b95264a2aa 100644 --- a/cmd/auth_list_test.go +++ b/cmd/auth_list_test.go @@ -2,12 +2,17 @@ package cmd import ( "os" + "path/filepath" + "strings" "testing" + cockroachErrors "github.com/cockroachdb/errors" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + errUtils "github.com/cloudposse/atmos/errors" "github.com/cloudposse/atmos/pkg/schema" ) @@ -430,6 +435,138 @@ func TestProvidersFlagCompletion_ReturnsSortedProviders(t *testing.T) { assert.Equal(t, []string{"apple-provider", "mango-provider", "zebra-provider"}, results) } +func TestExecuteAuthListCommand_SuggestsProfilesWhenNoAuthConfigured(t *testing.T) { + // Setup: Create a temp dir with minimal atmos.yaml (no auth providers/identities) + // but with profiles that could contain auth config. + tempDir := t.TempDir() + configFile := filepath.Join(tempDir, "atmos.yaml") + + configContent := `auth: + realm: test + logs: + level: Debug +` + err := os.WriteFile(configFile, []byte(configContent), 0o600) + require.NoError(t, err) + + // Create profile directories (simulating profiles that may contain auth config). + for _, profileName := range []string{"developers", "devops", "superadmin"} { + profileDir := filepath.Join(tempDir, "profiles", profileName) + err := os.MkdirAll(profileDir, 0o755) + require.NoError(t, err) + + // Add a minimal atmos.yaml in each profile so it's discovered. + profileConfig := filepath.Join(profileDir, "atmos.yaml") + err = os.WriteFile(profileConfig, []byte("# profile config\n"), 0o600) + require.NoError(t, err) + } + + // Set environment to use test config. + t.Chdir(tempDir) + + _ = NewTestKit(t) + + cmd := createTestAuthListCmd() + cmd.RunE = executeAuthListCommand + + // Execute the command - should return an error suggesting profiles. + err = cmd.Execute() + + require.Error(t, err) + assert.ErrorIs(t, err, errUtils.ErrAuthNotConfigured) + + // Verify hints contain profile names and usage suggestion. + hints := cockroachErrors.GetAllHints(err) + allHints := strings.Join(hints, " ") + assert.Contains(t, allHints, "developers") + assert.Contains(t, allHints, "devops") + assert.Contains(t, allHints, "superadmin") + assert.Contains(t, allHints, "--profile") +} + +func TestExecuteAuthListCommand_NoErrorWhenNoProfilesExist(t *testing.T) { + // Setup: Create a temp dir with minimal atmos.yaml (no auth, no profiles). + tempDir := t.TempDir() + configFile := filepath.Join(tempDir, "atmos.yaml") + + configContent := `auth: + realm: test +` + err := os.WriteFile(configFile, []byte(configContent), 0o600) + require.NoError(t, err) + + // No profiles directory created. + + t.Chdir(tempDir) + + _ = NewTestKit(t) + + cmd := createTestAuthListCmd() + cmd.RunE = executeAuthListCommand + + // Execute the command - should NOT return an error (no profiles to suggest). + err = cmd.Execute() + + assert.NoError(t, err) +} + +func TestExecuteAuthListCommand_ProfileFlagSuppressesNoAuthError(t *testing.T) { + // Regression test: when --profile loads a profile that contains auth config, + // ErrAuthNotConfigured must NOT be returned, even though the base atmos.yaml + // has no auth providers/identities. + tempDir := t.TempDir() + + // Base atmos.yaml: no auth providers/identities. + baseConfig := filepath.Join(tempDir, "atmos.yaml") + err := os.WriteFile(baseConfig, []byte(`auth: + realm: test +`), 0o600) + require.NoError(t, err) + + // Profile "devops" contains auth providers and identities. + profileDir := filepath.Join(tempDir, "profiles", "devops") + require.NoError(t, os.MkdirAll(profileDir, 0o755)) + profileConfig := filepath.Join(profileDir, "atmos.yaml") + err = os.WriteFile(profileConfig, []byte(`auth: + providers: + my-sso: + kind: aws/iam-identity-center + region: us-east-1 + start_url: https://example.awsapps.com/start + identities: + dev-role: + kind: aws/permission-set + provider: my-sso + permission_set: DevOps +`), 0o600) + require.NoError(t, err) + + t.Chdir(tempDir) + + _ = NewTestKit(t) + + cmd := createTestAuthListCmd() + cmd.RunE = executeAuthListCommand + + // Set profile via Viper (matching how global flag binding works in real execution). + // In production, RootCmd's persistent --profile flag is bound to Viper via BindToViper. + // loadAuthManagerForList calls flags.ParseGlobalFlags which reads v.GetStringSlice("profile"). + v := viper.GetViper() + v.Set("profile", []string{"devops"}) + t.Cleanup(func() { v.Set("profile", []string{}) }) + + // Execute — should NOT return ErrAuthNotConfigured because the profile + // contributes auth providers/identities to the merged configuration. + err = cmd.Execute() + // The command should succeed (no ErrAuthNotConfigured) or fail for a different + // reason (e.g., mock provider not available). Either way, it must not be + // ErrAuthNotConfigured which would mean --profile was ignored. + if err != nil { + assert.NotErrorIs(t, err, errUtils.ErrAuthNotConfigured, + "--profile should load auth config and suppress ErrAuthNotConfigured") + } +} + func TestIdentitiesFlagCompletion_ReturnsSortedIdentities(t *testing.T) { // Setup: Create a test config file with identities in non-alphabetical order. tempDir := t.TempDir() diff --git a/internal/exec/utils_auth.go b/internal/exec/utils_auth.go index b5a194d4ee..2727c3f9e2 100644 --- a/internal/exec/utils_auth.go +++ b/internal/exec/utils_auth.go @@ -188,6 +188,13 @@ func buildGlobalAuthSection(atmosConfig *schema.AtmosConfiguration) map[string]a if atmosConfig.Auth.Keyring.Type != "" { globalAuthSection["keyring"] = atmosConfig.Auth.Keyring } + // Only include realm when explicitly configured (env var or atmos.yaml). + // Auto-computed realms (from config-path hash or default) are path-dependent + // and should not appear in component describe output. + if atmosConfig.Auth.Realm != "" && + (atmosConfig.Auth.RealmSource == "env" || atmosConfig.Auth.RealmSource == "config") { + globalAuthSection["realm"] = atmosConfig.Auth.Realm + } return globalAuthSection } diff --git a/internal/exec/utils_auth_test.go b/internal/exec/utils_auth_test.go index a0a5186d45..7334b8dbe4 100644 --- a/internal/exec/utils_auth_test.go +++ b/internal/exec/utils_auth_test.go @@ -148,6 +148,90 @@ func TestBuildGlobalAuthSection(t *testing.T) { "keyring": schema.KeyringConfig{Type: "file"}, }, }, + { + name: "realm included when explicitly configured", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "my-project", + RealmSource: "config", + }, + }, + expected: map[string]any{ + "realm": "my-project", + }, + }, + { + name: "realm included when set via env", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "env-realm", + RealmSource: "env", + }, + }, + expected: map[string]any{ + "realm": "env-realm", + }, + }, + { + name: "realm excluded when auto-computed from config-path", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "b80ea18be93f8201", + RealmSource: "config-path", + }, + }, + expected: map[string]any{}, + }, + { + name: "realm excluded when default", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "default", + RealmSource: "default", + }, + }, + expected: map[string]any{}, + }, + { + name: "realm excluded when empty", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "", + }, + }, + expected: map[string]any{}, + }, + { + name: "all sections including explicit realm", + config: &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "prod-realm", + RealmSource: "config", + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + Identities: map[string]schema.Identity{ + "dev": {Kind: "aws"}, + }, + Logs: schema.Logs{Level: "info"}, + Keyring: schema.KeyringConfig{Type: "file"}, + }, + }, + expected: map[string]any{ + "realm": "prod-realm", + "providers": map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + "identities": map[string]schema.Identity{ + "dev": {Kind: "aws"}, + }, + "logs": map[string]any{ + "level": "info", + "file": "", + }, + "keyring": schema.KeyringConfig{Type: "file"}, + }, + }, { name: "empty maps are excluded", config: &schema.AtmosConfiguration{ @@ -715,6 +799,144 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_NilAuthManager(t *testing.T) { assert.Nil(t, result) } +func TestGetMergedAuthConfigWithFetcher_RealmPropagated(t *testing.T) { + // Verify realm is propagated through CopyGlobalAuthConfig when no component auth exists. + // This is the --all path: each component iteration must preserve the realm. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "my-project", + RealmSource: "config", + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + }, + } + info := &schema.ConfigAndStacksInfo{ + Stack: "dev-us-west-2", + ComponentFromArg: "vpc", + } + + // Mock fetcher returns component config without auth section. + mockFetcher := func(_ *ExecuteDescribeComponentParams) (map[string]any, error) { + return map[string]any{ + "vars": map[string]any{"test": "value"}, + }, nil + } + + result, err := getMergedAuthConfigWithFetcher(atmosConfig, info, mockFetcher) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "my-project", result.Realm) + assert.Equal(t, "config", result.RealmSource) +} + +func TestGetMergedAuthConfigWithFetcher_RealmPropagatedWithEmptyStack(t *testing.T) { + // When stack is empty (global auth only path), realm must still be preserved. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "env-realm", + RealmSource: "env", + }, + } + info := &schema.ConfigAndStacksInfo{ + Stack: "", + ComponentFromArg: "", + } + + mockFetcher := func(_ *ExecuteDescribeComponentParams) (map[string]any, error) { + t.Fatal("fetcher should not be called when stack is empty") + return nil, nil + } + + result, err := getMergedAuthConfigWithFetcher(atmosConfig, info, mockFetcher) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "env-realm", result.Realm) + assert.Equal(t, "env", result.RealmSource) +} + +func TestMergeGlobalAuthConfig_RealmPropagated(t *testing.T) { + // Verify explicitly configured realm is included in the merged auth section map. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "my-project", + RealmSource: "config", + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + }, + } + componentSection := map[string]any{} + + result := mergeGlobalAuthConfig(atmosConfig, componentSection) + assert.Contains(t, result, "realm") + assert.Equal(t, "my-project", result["realm"]) + assert.Contains(t, result, "providers") +} + +func TestMergeGlobalAuthConfig_NoRealmConfigured(t *testing.T) { + // When no realm is configured, the merged map should not contain a "realm" key. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + }, + } + componentSection := map[string]any{} + + result := mergeGlobalAuthConfig(atmosConfig, componentSection) + assert.NotContains(t, result, "realm") + assert.Contains(t, result, "providers") +} + +func TestMergeGlobalAuthConfig_AutoRealmExcluded(t *testing.T) { + // Auto-computed realm (from config-path hash) should not appear in merged output. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Realm: "b80ea18be93f8201", + RealmSource: "config-path", + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + }, + } + componentSection := map[string]any{} + + result := mergeGlobalAuthConfig(atmosConfig, componentSection) + assert.NotContains(t, result, "realm") + assert.Contains(t, result, "providers") +} + +func TestGetMergedAuthConfigWithFetcher_NoRealmPreservesEmptyRealm(t *testing.T) { + // When no realm is configured, the merged config should have empty realm — same as before the fix. + atmosConfig := &schema.AtmosConfiguration{ + Auth: schema.AuthConfig{ + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws-iam"}, + }, + }, + } + info := &schema.ConfigAndStacksInfo{ + Stack: "dev-us-west-2", + ComponentFromArg: "vpc", + } + + mockFetcher := func(_ *ExecuteDescribeComponentParams) (map[string]any, error) { + return map[string]any{ + "vars": map[string]any{"test": "value"}, + }, nil + } + + result, err := getMergedAuthConfigWithFetcher(atmosConfig, info, mockFetcher) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Empty(t, result.Realm) + assert.Empty(t, result.RealmSource) + // Providers should still be present. + assert.Len(t, result.Providers, 1) +} + func TestGetMergedAuthConfigWithFetcher_MergeReturnsError(t *testing.T) { // This test verifies the path where MergeComponentAuthFromConfig returns an error. // When that happens, getMergedAuthConfigWithFetcher propagates the error. diff --git a/pkg/auth/config_helpers.go b/pkg/auth/config_helpers.go index 86680731f5..52612e6819 100644 --- a/pkg/auth/config_helpers.go +++ b/pkg/auth/config_helpers.go @@ -19,7 +19,9 @@ func CopyGlobalAuthConfig(globalAuth *schema.AuthConfig) *schema.AuthConfig { } config := &schema.AuthConfig{ - Logs: globalAuth.Logs, + Realm: globalAuth.Realm, + RealmSource: globalAuth.RealmSource, + Logs: globalAuth.Logs, Keyring: schema.KeyringConfig{ Type: globalAuth.Keyring.Type, }, diff --git a/pkg/auth/config_helpers_test.go b/pkg/auth/config_helpers_test.go index d90027828c..12ff2174e0 100644 --- a/pkg/auth/config_helpers_test.go +++ b/pkg/auth/config_helpers_test.go @@ -25,8 +25,10 @@ func TestCopyGlobalAuthConfig(t *testing.T) { }, }, { - name: "copies all fields", + name: "copies all fields including realm", globalAuth: &schema.AuthConfig{ + Realm: "my-project", + RealmSource: "config", Providers: map[string]schema.Provider{ "test-provider": { Kind: "aws/iam-identity-center", @@ -50,6 +52,8 @@ func TestCopyGlobalAuthConfig(t *testing.T) { }, }, verify: func(t *testing.T, result *schema.AuthConfig) { + assert.Equal(t, "my-project", result.Realm) + assert.Equal(t, "config", result.RealmSource) assert.Len(t, result.Providers, 1) assert.Contains(t, result.Providers, "test-provider") assert.Len(t, result.Identities, 1) @@ -59,6 +63,28 @@ func TestCopyGlobalAuthConfig(t *testing.T) { assert.Len(t, result.IdentityCaseMap, 1) }, }, + { + name: "copies realm from env source", + globalAuth: &schema.AuthConfig{ + Realm: "env-realm", + RealmSource: "env", + }, + verify: func(t *testing.T, result *schema.AuthConfig) { + assert.Equal(t, "env-realm", result.Realm) + assert.Equal(t, "env", result.RealmSource) + }, + }, + { + name: "empty realm is preserved", + globalAuth: &schema.AuthConfig{ + Realm: "", + RealmSource: "", + }, + verify: func(t *testing.T, result *schema.AuthConfig) { + assert.Empty(t, result.Realm) + assert.Empty(t, result.RealmSource) + }, + }, { name: "deep copies Keyring.Spec map", globalAuth: &schema.AuthConfig{ @@ -91,6 +117,8 @@ func TestCopyGlobalAuthConfig(t *testing.T) { func TestCopyGlobalAuthConfig_DeepCopyMutation(t *testing.T) { // Test that modifying the copy doesn't mutate the original. original := &schema.AuthConfig{ + Realm: "original-realm", + RealmSource: "config", Keyring: schema.KeyringConfig{ Type: "file", Spec: map[string]interface{}{ @@ -106,12 +134,16 @@ func TestCopyGlobalAuthConfig_DeepCopyMutation(t *testing.T) { copy := CopyGlobalAuthConfig(original) // Modify the copy. + copy.Realm = "modified-realm" + copy.RealmSource = "env" copy.Keyring.Spec["path"] = "/modified/path" copy.Keyring.Spec["new_key"] = "new_value" copy.IdentityCaseMap["original"] = "Modified" copy.IdentityCaseMap["new"] = "New" // Verify original is unchanged. + assert.Equal(t, "original-realm", original.Realm) + assert.Equal(t, "config", original.RealmSource) assert.Equal(t, "/original/path", original.Keyring.Spec["path"]) assert.Len(t, original.Keyring.Spec, 1) assert.NotContains(t, original.Keyring.Spec, "new_key") @@ -317,6 +349,117 @@ func TestMergeComponentAuthFromConfig_NilGlobalAuth(t *testing.T) { } } +func TestMergeComponentAuthFromConfig_RealmSurvivesMapstructureRoundTrip(t *testing.T) { + // Realm has mapstructure:"realm" so it must survive the struct→map→merge→map→struct round-trip. + // RealmSource has mapstructure:"-" so it is lost — this is expected because GetRealm() recomputes it. + globalAuth := &schema.AuthConfig{ + Realm: "my-project", + RealmSource: "config", + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws/iam-identity-center", Region: "us-east-1"}, + }, + Identities: map[string]schema.Identity{ + "global-id": {Kind: "aws/user", Default: true}, + }, + IdentityCaseMap: map[string]string{ + "global-id": "global-id", + }, + } + + atmosConfig := &schema.AtmosConfiguration{ + Settings: schema.AtmosSettings{ + ListMergeStrategy: "replace", + }, + } + + // Component with its own auth section that overrides some fields. + componentConfig := map[string]any{ + cfg.AuthSectionName: map[string]any{ + "identities": map[string]any{ + "component-id": map[string]any{ + "kind": "aws/assume-role", + }, + }, + }, + } + + result, err := MergeComponentAuthFromConfig(globalAuth, componentConfig, atmosConfig, cfg.AuthSectionName) + assert.NoError(t, err) + assert.NotNil(t, result) + + // Realm must survive the merge round-trip. + assert.Equal(t, "my-project", result.Realm) + + // RealmSource is lost (mapstructure:"-") — this is expected. + // GetRealm() recomputes it from the Realm value when creating the manager. + assert.Empty(t, result.RealmSource) + + // Other fields still work. + assert.Len(t, result.Identities, 2) + assert.Contains(t, result.Identities, "global-id") + assert.Contains(t, result.Identities, "component-id") +} + +func TestMergeComponentAuthFromConfig_NoRealmConfigured(t *testing.T) { + // When no realm is configured, everything should work as before — no realm in result. + globalAuth := &schema.AuthConfig{ + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws/iam-identity-center"}, + }, + Identities: map[string]schema.Identity{ + "dev": {Kind: "aws/user", Default: true}, + }, + IdentityCaseMap: map[string]string{ + "dev": "dev", + }, + } + + atmosConfig := &schema.AtmosConfiguration{ + Settings: schema.AtmosSettings{ + ListMergeStrategy: "replace", + }, + } + + componentConfig := map[string]any{ + cfg.AuthSectionName: map[string]any{ + "identities": map[string]any{ + "staging": map[string]any{ + "kind": "aws/assume-role", + }, + }, + }, + } + + result, err := MergeComponentAuthFromConfig(globalAuth, componentConfig, atmosConfig, cfg.AuthSectionName) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Empty(t, result.Realm) + assert.Empty(t, result.RealmSource) + assert.Len(t, result.Identities, 2) +} + +func TestCopyGlobalAuthConfig_NoRealmConfigured(t *testing.T) { + // When no realm is configured at all, CopyGlobalAuthConfig should produce a valid config + // with empty realm fields — same as before this fix was applied. + globalAuth := &schema.AuthConfig{ + Providers: map[string]schema.Provider{ + "aws": {Kind: "aws/iam-identity-center"}, + }, + Identities: map[string]schema.Identity{ + "dev": {Kind: "aws/user", Default: true}, + }, + Logs: schema.Logs{Level: "Info"}, + } + + result := CopyGlobalAuthConfig(globalAuth) + assert.NotNil(t, result) + assert.Empty(t, result.Realm) + assert.Empty(t, result.RealmSource) + assert.Len(t, result.Providers, 1) + assert.Len(t, result.Identities, 1) + assert.Equal(t, "Info", result.Logs.Level) +} + func TestAuthConfigToMap(t *testing.T) { tests := []struct { name string diff --git a/pkg/auth/factory/factory.go b/pkg/auth/factory/factory.go index bc861ca18a..179131a7ff 100644 --- a/pkg/auth/factory/factory.go +++ b/pkg/auth/factory/factory.go @@ -158,6 +158,9 @@ func NewIdentity(name string, config *schema.Identity) (types.Identity, error) { case "mock/aws": return mockawsProviders.NewIdentity(name, config), nil default: + if config.Kind == "" { + return nil, fmt.Errorf("%w: identity is not configured", errUtils.ErrInvalidIdentityKind) + } return nil, fmt.Errorf("%w: unsupported identity kind: %s", errUtils.ErrInvalidIdentityKind, config.Kind) } } diff --git a/pkg/auth/factory/factory_test.go b/pkg/auth/factory/factory_test.go index c439f8cf74..6f313c43cc 100644 --- a/pkg/auth/factory/factory_test.go +++ b/pkg/auth/factory/factory_test.go @@ -165,6 +165,7 @@ func TestNewIdentity_Factory(t *testing.T) { config *schema.Identity expectError bool errorType error + errorMsg string }{ { name: "aws-permission-set-valid", @@ -257,6 +258,7 @@ func TestNewIdentity_Factory(t *testing.T) { config: &schema.Identity{Kind: ""}, expectError: true, errorType: errUtils.ErrInvalidIdentityKind, + errorMsg: "identity is not configured", }, { name: "empty-name-allowed", @@ -275,6 +277,9 @@ func TestNewIdentity_Factory(t *testing.T) { if tt.errorType != nil { assert.ErrorIs(t, err, tt.errorType) } + if tt.errorMsg != "" && err != nil { + assert.Contains(t, err.Error(), tt.errorMsg) + } assert.Nil(t, identity) } else { assert.NoError(t, err) diff --git a/pkg/auth/manager.go b/pkg/auth/manager.go index 6ffec43f83..b9130a75fd 100644 --- a/pkg/auth/manager.go +++ b/pkg/auth/manager.go @@ -276,6 +276,14 @@ func (m *manager) Authenticate(ctx context.Context, identityName string) (*types m.triggerIntegrations(ctx, identityName, finalCreds) } + // Clean up legacy (pre-realm) keyring and file entries to prevent realm mismatch warnings. + // This runs after successful authentication so legacy credentials remain as a fallback + // if authentication fails. + m.deleteLegacyCredentialFiles() + for _, step := range chain { + m.deleteLegacyKeyringEntry(step) + } + return m.buildWhoamiInfo(identityName, finalCreds), nil } @@ -572,6 +580,29 @@ func (m *manager) initializeProviders() error { // legacy path behavior with no realm subdirectory. func (m *manager) initializeIdentities() error { for name, identityConfig := range m.config.Identities { + // Check for unconfigured identities (empty kind) before attempting factory creation. + // This produces a clear, actionable error instead of the confusing "unsupported identity kind: ". + if identityConfig.Kind == "" { + builder := errUtils.Build(errUtils.ErrInvalidIdentityConfig). + WithContext("identity", name) + + if m.stackInfo != nil && len(m.stackInfo.ProfilesFromArg) > 0 { + profileNames := strings.Join(m.stackInfo.ProfilesFromArg, ", ") + builder = builder. + WithExplanationf("Identity %q is not configured in the `%s` profile.", name, profileNames). + WithHint("Switch to a profile that includes this identity") + } else { + builder = builder. + WithExplanationf("Identity %q is not configured. Did you forget to specify a profile?", name) + } + + err := builder. + WithHint("Run `atmos profile list` to see available profiles"). + WithExitCode(1).Err() + errUtils.CheckErrorAndPrint(err, "Initialize Identities", "") + return err + } + identity, err := factory.NewIdentity(name, &identityConfig) if err != nil { errUtils.CheckErrorAndPrint(err, "Initialize Identities", "") diff --git a/pkg/auth/manager_test.go b/pkg/auth/manager_test.go index df5165789a..55d97b5408 100644 --- a/pkg/auth/manager_test.go +++ b/pkg/auth/manager_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + cockroachErrors "github.com/cockroachdb/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -2150,3 +2151,76 @@ func TestManager_ResolveProviderConfig(t *testing.T) { }) } } + +func TestInitializeIdentities(t *testing.T) { + tests := []struct { + name string + identities map[string]schema.Identity + stackInfo *schema.ConfigAndStacksInfo + expectError bool + expectedSentinel error + expectedDetails []string + expectedCount int + }{ + { + name: "empty kind with profile suggests profile mismatch", + identities: map[string]schema.Identity{"core-root/terraform": {Kind: ""}}, + stackInfo: &schema.ConfigAndStacksInfo{ + ProfilesFromArg: []string{"marketplace"}, + }, + expectError: true, + expectedSentinel: errUtils.ErrInvalidIdentityConfig, + expectedDetails: []string{"core-root/terraform", "is not configured in the", "marketplace"}, + }, + { + name: "empty kind without profile suggests specifying one", + identities: map[string]schema.Identity{"core-root/terraform": {Kind: ""}}, + stackInfo: &schema.ConfigAndStacksInfo{}, + expectError: true, + expectedSentinel: errUtils.ErrInvalidIdentityConfig, + expectedDetails: []string{"core-root/terraform", "is not configured", "Did you forget to specify a profile?"}, + }, + { + name: "empty kind with nil stackInfo suggests specifying profile", + identities: map[string]schema.Identity{"core-root/terraform": {Kind: ""}}, + stackInfo: nil, + expectError: true, + expectedSentinel: errUtils.ErrInvalidIdentityConfig, + expectedDetails: []string{"Did you forget to specify a profile?"}, + }, + { + name: "valid kind succeeds", + identities: map[string]schema.Identity{"mock-identity": {Kind: "mock"}}, + stackInfo: &schema.ConfigAndStacksInfo{}, + expectError: false, + expectedCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &manager{ + config: &schema.AuthConfig{ + Identities: tt.identities, + }, + identities: make(map[string]types.Identity), + stackInfo: tt.stackInfo, + } + + err := m.initializeIdentities() + + if tt.expectError { + assert.Error(t, err) + assert.ErrorIs(t, err, tt.expectedSentinel) + details := cockroachErrors.GetAllDetails(err) + require.NotEmpty(t, details) + for _, expected := range tt.expectedDetails { + assert.Contains(t, details[0], expected) + } + } else { + assert.NoError(t, err) + assert.Len(t, m.identities, tt.expectedCount) + } + }) + } +} diff --git a/pkg/auth/manager_whoami.go b/pkg/auth/manager_whoami.go index e7fd7e4540..ff98ba99c3 100644 --- a/pkg/auth/manager_whoami.go +++ b/pkg/auth/manager_whoami.go @@ -46,6 +46,8 @@ func (m *manager) buildWhoamiInfo(identityName string, creds types.ICredentials) // Note: We keep info.Credentials populated for validation purposes. // The Credentials field is marked with json:"-" yaml:"-" tags to prevent // accidental serialization, so there's no security risk in keeping it. + // Clean up legacy (pre-realm) keyring entry to prevent realm mismatch warnings. + m.deleteLegacyKeyringEntry(identityName) } } else { log.Debug("Skipping keyring cache for session tokens in WhoamiInfo", logKeyIdentity, identityName) diff --git a/pkg/auth/realm_mismatch.go b/pkg/auth/realm_mismatch.go index 6c9d786604..c57fcd403d 100644 --- a/pkg/auth/realm_mismatch.go +++ b/pkg/auth/realm_mismatch.go @@ -1,6 +1,7 @@ package auth import ( + "errors" "fmt" "os" "path/filepath" @@ -120,6 +121,77 @@ func hasCredentialFiles(awsDir string) bool { return false } +// deleteLegacyKeyringEntry removes a pre-realm keyring entry after successful +// authentication. This runs in the post-success path of the Authenticate flow, +// after authenticateChain and PostAuthenticate have completed, so legacy +// credentials remain as a fallback if authentication fails. +func (m *manager) deleteLegacyKeyringEntry(alias string) { + if m.realm.Value == "" || m.credentialStore == nil { + return + } + if err := m.credentialStore.Delete(alias, ""); err != nil { + log.Warn("Failed to delete legacy keyring entry", "alias", alias, "error", err) + } else { + log.Debug("Deleted legacy keyring entry (pre-realm)", "alias", alias) + } +} + +// deleteLegacyCredentialFiles removes pre-realm credential files after successful +// authentication. This runs in the post-success path of the Authenticate flow, +// after authenticateChain and PostAuthenticate have completed, so legacy files +// remain as a fallback if authentication fails. +// Scans {baseDir}/aws/{provider}/ for credential and config files and removes them. +// TODO: Refactor to use a provider/store-owned cleanup hook instead of hardcoding +// AWS-specific paths here, keeping the auth manager cloud-agnostic. +func (m *manager) deleteLegacyCredentialFiles() { + if m.realm.Value == "" { + return + } + + baseDir := xdg.LookupXDGConfigDir("") + if baseDir == "" { + return + } + + awsDir := filepath.Join(baseDir, awsDirNameForMismatch) + providerDirs, err := os.ReadDir(awsDir) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + log.Warn("Failed to read legacy credential directory", "path", awsDir, "error", err) + } + return + } + + for _, provider := range providerDirs { + if !provider.IsDir() { + continue + } + cleanLegacyProviderDir(filepath.Join(awsDir, provider.Name())) + } + // Remove aws directory if now empty. + removeIfEmpty(awsDir) +} + +// cleanLegacyProviderDir removes credential, config, and lock files from a legacy provider directory. +func cleanLegacyProviderDir(providerDir string) { + for _, filename := range []string{"credentials", "config", "credentials.lock", "config.lock"} { + filePath := filepath.Join(providerDir, filename) + if err := os.Remove(filePath); err == nil { + log.Debug("Deleted legacy credential file (pre-realm)", "path", filePath) + } else if !errors.Is(err, os.ErrNotExist) { + log.Warn("Failed to delete legacy credential file", "path", filePath, "error", err) + } + } + removeIfEmpty(providerDir) +} + +// removeIfEmpty removes a directory if it is empty, logging unexpected errors. +func removeIfEmpty(dir string) { + if err := os.Remove(dir); err != nil && !errors.Is(err, os.ErrNotExist) { + log.Debug("Could not remove legacy directory (may not be empty)", "path", dir, "error", err) + } +} + // logRealmMismatchWarning emits a warning about credentials existing under a different realm. func logRealmMismatchWarning(currentRealm, alternateRealm string) { currentDisplay := currentRealm diff --git a/pkg/auth/realm_mismatch_test.go b/pkg/auth/realm_mismatch_test.go index 19434356ce..6bb811a915 100644 --- a/pkg/auth/realm_mismatch_test.go +++ b/pkg/auth/realm_mismatch_test.go @@ -8,8 +8,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "github.com/cloudposse/atmos/pkg/auth/realm" + "github.com/cloudposse/atmos/pkg/auth/types" ) func TestCheckRealmMismatchFiles_NonEmptyRealm_FindsNoRealmCreds(t *testing.T) { @@ -291,6 +293,169 @@ func TestCheckNoRealmCredentials(t *testing.T) { }) } +func TestDeleteLegacyKeyringEntry_CleansUpEmptyRealm(t *testing.T) { + ctrl := gomock.NewController(t) + store := types.NewMockCredentialStore(ctrl) + + // Expect Delete to be called with the alias and empty realm (legacy entry). + store.EXPECT().Delete("my-identity", "").Return(nil) + + m := &manager{ + credentialStore: store, + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + m.deleteLegacyKeyringEntry("my-identity") +} + +func TestDeleteLegacyKeyringEntry_NoOpForEmptyRealm(t *testing.T) { + ctrl := gomock.NewController(t) + store := types.NewMockCredentialStore(ctrl) + + // No Delete call expected — realm is empty, so no cleanup should happen. + + m := &manager{ + credentialStore: store, + realm: realm.RealmInfo{Value: "", Source: "auto"}, + } + + m.deleteLegacyKeyringEntry("my-identity") +} + +func TestDeleteLegacyKeyringEntry_NilStore(t *testing.T) { + m := &manager{ + credentialStore: nil, + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + // Should not panic. + m.deleteLegacyKeyringEntry("my-identity") +} + +func TestDeleteLegacyCredentialFiles_CleansUpNoRealmFiles(t *testing.T) { + // Setup: credential files at {baseDir}/aws/{provider}/credentials (no realm). + parent := t.TempDir() + atmosDir := filepath.Join(parent, "atmos") + providerDir := filepath.Join(atmosDir, "aws", "my-provider") + require.NoError(t, os.MkdirAll(providerDir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "credentials"), []byte("test"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "config"), []byte("test"), 0o600)) + t.Setenv("ATMOS_XDG_CONFIG_HOME", parent) + + m := &manager{ + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + m.deleteLegacyCredentialFiles() + + // Legacy files should be deleted. + _, err := os.Stat(filepath.Join(providerDir, "credentials")) + assert.True(t, os.IsNotExist(err), "legacy credentials file should be deleted") + _, err = os.Stat(filepath.Join(providerDir, "config")) + assert.True(t, os.IsNotExist(err), "legacy config file should be deleted") +} + +func TestDeleteLegacyCredentialFiles_NoOpForEmptyRealm(t *testing.T) { + parent := t.TempDir() + atmosDir := filepath.Join(parent, "atmos") + providerDir := filepath.Join(atmosDir, "aws", "my-provider") + require.NoError(t, os.MkdirAll(providerDir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "credentials"), []byte("test"), 0o600)) + t.Setenv("ATMOS_XDG_CONFIG_HOME", parent) + + m := &manager{ + realm: realm.RealmInfo{Value: "", Source: "auto"}, + } + + m.deleteLegacyCredentialFiles() + + // Files should NOT be deleted when realm is empty. + _, err := os.Stat(filepath.Join(providerDir, "credentials")) + assert.NoError(t, err, "files should not be deleted when realm is empty") +} + +func TestDeleteLegacyKeyringEntry_DeleteFails(t *testing.T) { + ctrl := gomock.NewController(t) + store := types.NewMockCredentialStore(ctrl) + + // Expect Delete to be called but return an error. + store.EXPECT().Delete("my-identity", "").Return(assert.AnError) + + m := &manager{ + credentialStore: store, + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + // Should not panic even when Delete fails. + m.deleteLegacyKeyringEntry("my-identity") +} + +func TestDeleteLegacyCredentialFiles_CleansUpLockFilesAndEmptyDirs(t *testing.T) { + // Setup: credential files including lock files at {baseDir}/aws/{provider}/. + parent := t.TempDir() + atmosDir := filepath.Join(parent, "atmos") + providerDir := filepath.Join(atmosDir, "aws", "my-provider") + require.NoError(t, os.MkdirAll(providerDir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "credentials"), []byte("test"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "credentials.lock"), []byte("test"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "config"), []byte("test"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(providerDir, "config.lock"), []byte("test"), 0o600)) + t.Setenv("ATMOS_XDG_CONFIG_HOME", parent) + + m := &manager{ + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + m.deleteLegacyCredentialFiles() + + // All legacy files should be deleted. + for _, filename := range []string{"credentials", "credentials.lock", "config", "config.lock"} { + _, err := os.Stat(filepath.Join(providerDir, filename)) + assert.True(t, os.IsNotExist(err), "legacy %s file should be deleted", filename) + } + + // Provider and aws directories should be cleaned up since they're empty. + _, err := os.Stat(providerDir) + assert.True(t, os.IsNotExist(err), "empty provider dir should be removed") + _, err = os.Stat(filepath.Join(atmosDir, "aws")) + assert.True(t, os.IsNotExist(err), "empty aws dir should be removed") +} + +func TestDeleteLegacyCredentialFiles_SkipsNonDirectoryEntries(t *testing.T) { + // Setup: aws dir with a regular file (not a provider directory). + parent := t.TempDir() + atmosDir := filepath.Join(parent, "atmos") + awsDir := filepath.Join(atmosDir, "aws") + require.NoError(t, os.MkdirAll(awsDir, 0o700)) + // Create a stray file in the aws dir (should be skipped). + require.NoError(t, os.WriteFile(filepath.Join(awsDir, "stray-file.txt"), []byte("not a dir"), 0o600)) + t.Setenv("ATMOS_XDG_CONFIG_HOME", parent) + + m := &manager{ + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + // Should not panic, stray file should remain. + m.deleteLegacyCredentialFiles() + + _, err := os.Stat(filepath.Join(awsDir, "stray-file.txt")) + assert.NoError(t, err, "non-directory entries should not be removed") +} + +func TestDeleteLegacyCredentialFiles_NoAwsDir(t *testing.T) { + // Setup: base dir exists but no aws subdirectory. + parent := t.TempDir() + atmosDir := filepath.Join(parent, "atmos") + require.NoError(t, os.MkdirAll(atmosDir, 0o700)) + t.Setenv("ATMOS_XDG_CONFIG_HOME", parent) + + m := &manager{ + realm: realm.RealmInfo{Value: "my-realm", Source: "config"}, + } + + // Should not panic. + m.deleteLegacyCredentialFiles() +} + func TestLogRealmMismatchWarning_NonEmptyToEmpty(t *testing.T) { // Verify it doesn't panic. The actual log output goes to the logger. logRealmMismatchWarning("my-realm", "(no realm)")