From 2e6cb980093c4efe5ebd50dd58310f0809cb9fb7 Mon Sep 17 00:00:00 2001 From: aknysh Date: Thu, 6 Aug 2026 10:43:47 -0400 Subject: [PATCH 1/3] fix(auth): cover legacy ARM audience and seed refresh token in Azure CLI cache The Azure CLI cache write-back after `atmos auth login` seeded access tokens only under the modern ARM scope (management.azure.com) and no refresh token. Field impact: azapi-based Terraform modules (all modern AVM modules) failed mid-apply with "AzureCLICredential: ERROR: Can't find token from MSAL cache" (azidentity/az request the legacy management.core.windows.net audience by default), and after ~1h every az-side lookup failed the same way (nothing to refresh from). - CloudEnvironment gains LegacyManagementScopes (public/usgov/china, single- and double-slash forms); both cache writers store the seeded management token with all ARM scope forms in its MSAL `target` -- MSAL matches requested scopes as a subset, and ARM accepts either audience, so one entry now satisfies every lookup. - New CopyAtmosRefreshTokensInto copies the account's refresh tokens from the Atmos realm MSAL cache into the az cache (Atmos uses the Azure CLI public client, so the RT is directly usable by az) -- az can then self-mint any audience and survive access-token expiry. Skipped for service principals, empty realms, or unknown home account IDs. UpdateAzureCLIFiles gains a realm parameter. - Regression tests reproduce both field failures (written first, confirmed failing) and pin the fix. Co-Authored-By: Claude Fable 5 --- ...cli-cache-legacy-audience-refresh-token.md | 53 ++++++++ pkg/auth/cloud/azure/cloud_environments.go | 56 +++++---- pkg/auth/cloud/azure/refresh_token.go | 78 ++++++++++++ pkg/auth/cloud/azure/setup.go | 55 +++++--- pkg/auth/cloud/azure/setup_test.go | 28 +++-- pkg/auth/identities/azure/subscription.go | 2 +- pkg/auth/providers/azure/device_code_cache.go | 7 +- .../providers/azure/token_audience_test.go | 117 ++++++++++++++++++ 8 files changed, 343 insertions(+), 53 deletions(-) create mode 100644 docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.md create mode 100644 pkg/auth/cloud/azure/refresh_token.go create mode 100644 pkg/auth/providers/azure/token_audience_test.go diff --git a/docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.md b/docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.md new file mode 100644 index 0000000000..12f56e6df6 --- /dev/null +++ b/docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.md @@ -0,0 +1,53 @@ +# Fix: Seeded Azure CLI cache misses the legacy ARM audience and carries no refresh token + +**Date:** 2026-08-06 + +## Summary + +After `atmos auth login` (device-code or interactive), the Azure CLI cache write-back +seeded access tokens only under the modern ARM scope +(`https://management.azure.com/.default`) and seeded no refresh token. Two field +failures followed: `azapi`-based Terraform modules (all modern Azure Verified Modules) +died mid-apply with `AzureCLICredential: ERROR: Can't find token from MSAL cache`, +because azidentity/az request the **legacy** ARM audience +(`https://management.core.windows.net/`) by default; and after ~1 hour every az-side +lookup failed the same way, because az had no refresh token to self-mint replacements. +The management token entry now carries the legacy audience scope forms in its MSAL +`target`, and the refresh token MSAL persists in the Atmos realm cache is copied into +the az cache. + +## Context + +Reproduced end to end during a real engagement: `azurerm` provider resources applied +fine (its az shell-out requests the modern scope) while the `azapi` resource in the +same apply failed — `az account get-access-token` (default → legacy resource) missed +the cache while `--scope https://management.azure.com/.default` hit it. The failure is +cache-*lookup*, not token validity: ARM accepts both audience forms interchangeably, +and MSAL matches requested scopes as a subset of an entry's space-separated `target`. +So one seeded management token entry listing all ARM scope forms +(`management.azure.com/.default`, plus the legacy single- and double-slash forms az +derives from the trailing-slash resource) satisfies every lookup. + +The refresh-token gap is fixable because Atmos authenticates with the Azure CLI public +client: the refresh token MSAL persists in `~/.azure/atmos//msal_token_cache.json` +is exactly the credential az's own login would have stored. + +## Changes + +- `pkg/auth/cloud/azure/cloud_environments.go`: `CloudEnvironment` gains + `LegacyManagementScopes` (public/usgovernment/china variants, single- and + double-slash forms). +- `pkg/auth/providers/azure/device_code_cache.go` and + `pkg/auth/cloud/azure/setup.go`: the seeded management token's `target` now joins + the modern scope with the legacy forms. +- `pkg/auth/cloud/azure/refresh_token.go` (new): `CopyAtmosRefreshTokensInto` copies + the account's refresh-token entries from the Atmos realm cache into the az cache + (skipped for service principals, empty realms, or unknown home account IDs). + Both cache writers invoke it; `UpdateAzureCLIFiles` gains a `realm` parameter. +- Regression tests (`token_audience_test.go`) reproduce both field failures and pin + the fix; the sovereign-cloud test now asserts the multi-audience `target`. + +## Recovery (pre-fix versions) + +`az login --tenant ` gives az its own refresh token; keep such a session +alongside `atmos auth login` when applying azapi/AVM-based components. diff --git a/pkg/auth/cloud/azure/cloud_environments.go b/pkg/auth/cloud/azure/cloud_environments.go index 679e700e08..63d19e1037 100644 --- a/pkg/auth/cloud/azure/cloud_environments.go +++ b/pkg/auth/cloud/azure/cloud_environments.go @@ -16,6 +16,11 @@ type CloudEnvironment struct { LoginEndpoint string // ManagementScope is the Azure Resource Manager API scope. ManagementScope string + // LegacyManagementScopes are the legacy ARM audience scope forms + // (management.core.*) that az and azidentity derive by default. ARM accepts + // both audiences interchangeably, so the seeded management token is stored + // with all forms in its MSAL `target` for cache-lookup coverage. + LegacyManagementScopes []string // GraphAPIScope is the Microsoft Graph API scope. GraphAPIScope string // KeyVaultScope is the Azure KeyVault API scope. @@ -31,34 +36,37 @@ type CloudEnvironment struct { // Well-known Azure cloud environments. var cloudEnvironments = map[string]*CloudEnvironment{ "public": { - Name: "public", - LoginEndpoint: "login.microsoftonline.com", - ManagementScope: "https://management.azure.com/.default", - GraphAPIScope: "https://graph.microsoft.com/.default", - KeyVaultScope: "https://vault.azure.net/.default", - BlobStorageSuffix: "blob.core.windows.net", - PortalURL: "https://portal.azure.com/", - AzureProfileEnvName: "AzureCloud", + Name: "public", + LoginEndpoint: "login.microsoftonline.com", + ManagementScope: "https://management.azure.com/.default", + LegacyManagementScopes: []string{"https://management.core.windows.net/.default", "https://management.core.windows.net//.default"}, + GraphAPIScope: "https://graph.microsoft.com/.default", + KeyVaultScope: "https://vault.azure.net/.default", + BlobStorageSuffix: "blob.core.windows.net", + PortalURL: "https://portal.azure.com/", + AzureProfileEnvName: "AzureCloud", }, "usgovernment": { - Name: "usgovernment", - LoginEndpoint: "login.microsoftonline.us", - ManagementScope: "https://management.usgovcloudapi.net/.default", - GraphAPIScope: "https://graph.microsoft.us/.default", - KeyVaultScope: "https://vault.usgovcloudapi.net/.default", - BlobStorageSuffix: "blob.core.usgovcloudapi.net", - PortalURL: "https://portal.azure.us/", - AzureProfileEnvName: "AzureUSGovernment", + Name: "usgovernment", + LoginEndpoint: "login.microsoftonline.us", + ManagementScope: "https://management.usgovcloudapi.net/.default", + LegacyManagementScopes: []string{"https://management.core.usgovcloudapi.net/.default", "https://management.core.usgovcloudapi.net//.default"}, + GraphAPIScope: "https://graph.microsoft.us/.default", + KeyVaultScope: "https://vault.usgovcloudapi.net/.default", + BlobStorageSuffix: "blob.core.usgovcloudapi.net", + PortalURL: "https://portal.azure.us/", + AzureProfileEnvName: "AzureUSGovernment", }, "china": { - Name: "china", - LoginEndpoint: "login.chinacloudapi.cn", - ManagementScope: "https://management.chinacloudapi.cn/.default", - GraphAPIScope: "https://microsoftgraph.chinacloudapi.cn/.default", - KeyVaultScope: "https://vault.azure.cn/.default", - BlobStorageSuffix: "blob.core.chinacloudapi.cn", - PortalURL: "https://portal.azure.cn/", - AzureProfileEnvName: "AzureChinaCloud", + Name: "china", + LoginEndpoint: "login.chinacloudapi.cn", + ManagementScope: "https://management.chinacloudapi.cn/.default", + LegacyManagementScopes: []string{"https://management.core.chinacloudapi.cn/.default", "https://management.core.chinacloudapi.cn//.default"}, + GraphAPIScope: "https://microsoftgraph.chinacloudapi.cn/.default", + KeyVaultScope: "https://vault.azure.cn/.default", + BlobStorageSuffix: "blob.core.chinacloudapi.cn", + PortalURL: "https://portal.azure.cn/", + AzureProfileEnvName: "AzureChinaCloud", }, } diff --git a/pkg/auth/cloud/azure/refresh_token.go b/pkg/auth/cloud/azure/refresh_token.go new file mode 100644 index 0000000000..a9c0491f7e --- /dev/null +++ b/pkg/auth/cloud/azure/refresh_token.go @@ -0,0 +1,78 @@ +package azure + +import ( + "encoding/json" + "os" + "path/filepath" + + log "github.com/cloudposse/atmos/pkg/logger" +) + +// CopyAtmosRefreshTokensInto copies refresh-token entries for the given account +// from the Atmos realm MSAL cache into an Azure CLI MSAL cache map. +// +// The Atmos providers authenticate with the Azure CLI public client, so the +// refresh token MSAL persists in the realm cache is directly usable by az. +// Seeding it lets az self-mint tokens for ANY audience (including the legacy +// ARM audience azidentity/azapi request) and survive access-token expiry — +// without it, az fails with "Can't find token from MSAL cache" as soon as a +// lookup misses the seeded access tokens. +func CopyAtmosRefreshTokensInto(azCache map[string]interface{}, home, realm, homeAccountID string) { + if realm == "" || homeAccountID == "" { + // Without a realm the Atmos cache path IS the az cache (self-copy); + // without a home account ID entries can't be matched safely. + return + } + + source := loadAtmosRefreshTokens(home, realm) + if len(source) == 0 { + return + } + + dest, ok := azCache["RefreshToken"].(map[string]interface{}) + if !ok { + dest = map[string]interface{}{} + azCache["RefreshToken"] = dest + } + + if copied := copyMatchingRefreshTokens(dest, source, homeAccountID); copied > 0 { + log.Debug("Copied refresh tokens into Azure CLI cache", "count", copied) + } +} + +// loadAtmosRefreshTokens reads the RefreshToken section of the Atmos realm +// MSAL cache; missing or unparsable caches are non-fatal (empty result). +func loadAtmosRefreshTokens(home, realm string) map[string]interface{} { + atmosCachePath := filepath.Join(home, ".azure", "atmos", realm, "msal_token_cache.json") + data, err := os.ReadFile(atmosCachePath) + if err != nil { + log.Debug("No Atmos MSAL cache to copy refresh tokens from", "path", atmosCachePath, "error", err) + return nil + } + + var atmosCache map[string]interface{} + if err := json.Unmarshal(data, &atmosCache); err != nil { + log.Debug("Failed to parse Atmos MSAL cache", "error", err) + return nil + } + + source, _ := atmosCache["RefreshToken"].(map[string]interface{}) + return source +} + +// copyMatchingRefreshTokens copies entries whose home_account_id matches. +func copyMatchingRefreshTokens(dest, source map[string]interface{}, homeAccountID string) int { + copied := 0 + for key, raw := range source { + entry, ok := raw.(map[string]interface{}) + if !ok { + continue + } + if hid, _ := entry["home_account_id"].(string); hid != homeAccountID { + continue + } + dest[key] = entry + copied++ + } + return copied +} diff --git a/pkg/auth/cloud/azure/setup.go b/pkg/auth/cloud/azure/setup.go index 5729f280e0..b8915d03cd 100644 --- a/pkg/auth/cloud/azure/setup.go +++ b/pkg/auth/cloud/azure/setup.go @@ -203,7 +203,7 @@ func SetEnvironmentVariables(authContext *schema.AuthContext, stackInfo *schema. // This should be called from PostAuthenticate to ensure CLI compatibility. // The cloudEnvName selects the Azure cloud environment ("public", "usgovernment", "china"). // If empty, defaults to "public". -func UpdateAzureCLIFiles(creds types.ICredentials, tenantID, subscriptionID, cloudEnvName string) error { +func UpdateAzureCLIFiles(creds types.ICredentials, tenantID, subscriptionID, cloudEnvName, realm string) error { azureCreds, ok := creds.(*types.AzureCredentials) if !ok { return nil // Not Azure credentials, nothing to do. @@ -239,7 +239,12 @@ func UpdateAzureCLIFiles(creds types.ICredentials, tenantID, subscriptionID, clo cloudEnv := GetCloudEnvironment(cloudEnvName) // Update MSAL token cache with management, Graph API, and KeyVault tokens. - updateMSALCacheFromCreds(home, azureCreds, userOID, tenantID, cloudEnv) + updateMSALCacheFromCreds(&msalCredsContext{ + Home: home, + UserOID: userOID, + TenantID: tenantID, + Realm: realm, + }, azureCreds, cloudEnv) // Update azureProfile.json. if err := updateAzureProfile(home, ProfileUpdateParams{ @@ -253,36 +258,51 @@ func UpdateAzureCLIFiles(creds types.ICredentials, tenantID, subscriptionID, clo // Non-fatal. } - // For service principal auth, also update service_principal_entries.json. - // This allows Azure CLI commands to work with OIDC tokens during the CI workflow. - if azureCreds.IsServicePrincipal && azureCreds.ClientID != "" && azureCreds.FederatedToken != "" { - if err := updateServicePrincipalEntries(home, azureCreds.ClientID, tenantID, azureCreds.FederatedToken); err != nil { - log.Debug("Failed to update service principal entries", "error", err) - // Non-fatal. - } - } + maybeUpdateServicePrincipalEntries(home, tenantID, azureCreds) return nil } +// maybeUpdateServicePrincipalEntries updates service_principal_entries.json for +// service principal auth. This allows Azure CLI commands to work with OIDC +// tokens during the CI workflow. Non-fatal on failure. +func maybeUpdateServicePrincipalEntries(home, tenantID string, azureCreds *types.AzureCredentials) { + if !azureCreds.IsServicePrincipal || azureCreds.ClientID == "" || azureCreds.FederatedToken == "" { + return + } + if err := updateServicePrincipalEntries(home, azureCreds.ClientID, tenantID, azureCreds.FederatedToken); err != nil { + log.Debug("Failed to update service principal entries", "error", err) + } +} + +// The msalCredsContext struct carries the cache-location and account context +// for updateMSALCacheFromCreds. +type msalCredsContext struct { + Home string + UserOID string + TenantID string + Realm string +} + // updateMSALCacheFromCreds updates the MSAL token cache using Azure credentials and cloud environment. -func updateMSALCacheFromCreds(home string, azureCreds *types.AzureCredentials, userOID, tenantID string, cloudEnv *CloudEnvironment) { +func updateMSALCacheFromCreds(ctx *msalCredsContext, azureCreds *types.AzureCredentials, cloudEnv *CloudEnvironment) { if err := updateMSALCache(&msalCacheUpdate{ - Home: home, + Home: ctx.Home, AccessToken: azureCreds.AccessToken, Expiration: azureCreds.Expiration, GraphToken: azureCreds.GraphAPIToken, GraphExpiration: azureCreds.GraphAPIExpiration, KeyVaultToken: azureCreds.KeyVaultToken, KeyVaultExpiration: azureCreds.KeyVaultExpiration, - UserOID: userOID, - TenantID: tenantID, + UserOID: ctx.UserOID, + TenantID: ctx.TenantID, AuthMethod: azureCreds.AuthMethod, HomeAccountID: azureCreds.HomeAccountID, ClientID: azureCreds.ClientID, IsServicePrincipal: azureCreds.IsServicePrincipal, LoginEndpoint: cloudEnv.LoginEndpoint, - ManagementScope: cloudEnv.ManagementScope, + ManagementScope: strings.Join(append([]string{cloudEnv.ManagementScope}, cloudEnv.LegacyManagementScopes...), " "), + Realm: ctx.Realm, GraphAPIScope: cloudEnv.GraphAPIScope, KeyVaultScope: cloudEnv.KeyVaultScope, }); err != nil { @@ -305,6 +325,8 @@ type msalCacheUpdate struct { // AuthMethod is the AzureCredentials.AuthMethod that minted these tokens; // selects the MSAL cache account_source label. AuthMethod string + // Realm is the Atmos credential-isolation realm (source of refresh tokens). + Realm string // HomeAccountID is MSAL's "{home-oid}.{home-tenant-id}" for the authenticated // account, when the provider received it from MSAL. For guest (B2B) users this // differs from "{UserOID}.{TenantID}" and MUST be used for the cache Account @@ -345,6 +367,9 @@ func updateMSALCache(params *msalCacheUpdate) error { } else { // User authentication uses standard format. addUserAccountAndTokens(sections, params) + // Copy refresh tokens from the Atmos realm cache so az can self-mint + // any audience and survive access-token expiry. + CopyAtmosRefreshTokensInto(cache, params.Home, params.Realm, params.HomeAccountID) } // Write updated cache. diff --git a/pkg/auth/cloud/azure/setup_test.go b/pkg/auth/cloud/azure/setup_test.go index 0c0f55d134..d8feb44c7b 100644 --- a/pkg/auth/cloud/azure/setup_test.go +++ b/pkg/auth/cloud/azure/setup_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "time" @@ -1447,7 +1448,7 @@ func TestUpdateMSALCacheFromCreds_SovereignCloud(t *testing.T) { KeyVaultExpiration: now.Add(3 * time.Hour).Format(time.RFC3339), } - updateMSALCacheFromCreds(tmpDir, azureCreds, "user-oid-gov", "gov-tenant", cloudEnv) + updateMSALCacheFromCreds(&msalCredsContext{Home: tmpDir, UserOID: "user-oid-gov", TenantID: "gov-tenant"}, azureCreds, cloudEnv) // Verify MSAL cache was created with sovereign cloud scopes. msalCachePath := filepath.Join(tmpDir, ".azure", "msal_token_cache.json") @@ -1469,8 +1470,11 @@ func TestUpdateMSALCacheFromCreds_SovereignCloud(t *testing.T) { continue } target, _ := tokenEntry["target"].(string) - if target == cloudEnv.ManagementScope { + if strings.Contains(target, cloudEnv.ManagementScope) { foundGovScope = true + // The target now also carries the legacy ARM audience forms for + // az/azidentity cache-lookup coverage. + assert.Contains(t, target, "https://management.core.usgovcloudapi.net/.default") // Verify the environment/realm uses the government login endpoint. realm, _ := tokenEntry["realm"].(string) assert.Equal(t, "gov-tenant", realm) @@ -1539,7 +1543,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { t.Run("non-Azure credentials returns nil", func(t *testing.T) { // Pass a non-Azure credential type. - err := UpdateAzureCLIFiles(nil, "tenant", "sub", "") + err := UpdateAzureCLIFiles(nil, "tenant", "sub", "", "") assert.NoError(t, err) }) @@ -1547,7 +1551,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { creds := &types.AzureCredentials{ AccessToken: "not-a-jwt", } - err := UpdateAzureCLIFiles(creds, "tenant", "sub", "") + err := UpdateAzureCLIFiles(creds, "tenant", "sub", "", "") assert.NoError(t, err, "Invalid token should return nil (non-fatal)") }) @@ -1565,7 +1569,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { SubscriptionID: "sub-def", } - err := UpdateAzureCLIFiles(creds, "tenant-abc", "sub-def", "") + err := UpdateAzureCLIFiles(creds, "tenant-abc", "sub-def", "", "") assert.NoError(t, err) // Verify files were created in the isolated temp home directory. @@ -1591,7 +1595,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { SubscriptionID: "gov-sub", } - err := UpdateAzureCLIFiles(creds, "gov-tenant", "gov-sub", "usgovernment") + err := UpdateAzureCLIFiles(creds, "gov-tenant", "gov-sub", "usgovernment", "") assert.NoError(t, err) }) @@ -1612,7 +1616,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { FederatedToken: "federated-token-value", } - err := UpdateAzureCLIFiles(creds, "sp-tenant", "sp-sub", "") + err := UpdateAzureCLIFiles(creds, "sp-tenant", "sp-sub", "", "") assert.NoError(t, err) }) @@ -1638,7 +1642,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { AuthMethod: types.AzureAuthMethodCLI, } - err := UpdateAzureCLIFiles(creds, "cli-tenant", "cli-sub", "") + err := UpdateAzureCLIFiles(creds, "cli-tenant", "cli-sub", "", "") assert.NoError(t, err) _, msalErr := os.Stat(filepath.Join(subHome, ".azure", "msal_token_cache.json")) @@ -1669,7 +1673,7 @@ func TestUpdateAzureCLIFiles(t *testing.T) { HomeAccountID: "home-oid.home-tenant", } - err := UpdateAzureCLIFiles(creds, "target-tenant", "target-sub", "") + err := UpdateAzureCLIFiles(creds, "target-tenant", "target-sub", "", "") assert.NoError(t, err) data, readErr := os.ReadFile(filepath.Join(subHome, ".azure", "msal_token_cache.json")) @@ -1780,7 +1784,7 @@ func TestUpdateMSALCacheFromCreds_WriteFailureIsNonFatal(t *testing.T) { cloudEnv := GetCloudEnvironment("") require.NotPanics(t, func() { - updateMSALCacheFromCreds(tmpDir, azureCreds, "user-oid-123", "tenant-123", cloudEnv) + updateMSALCacheFromCreds(&msalCredsContext{Home: tmpDir, UserOID: "user-oid-123", TenantID: "tenant-123"}, azureCreds, cloudEnv) }) _, err := os.ReadFile(msalCachePath) @@ -1926,7 +1930,7 @@ func TestUpdateAzureCLIFiles_ProfileWriteFailureIsNonFatal(t *testing.T) { Expiration: now.Add(1 * time.Hour).Format(time.RFC3339), } - err := UpdateAzureCLIFiles(creds, "tenant-abc", "sub-def", "") + err := UpdateAzureCLIFiles(creds, "tenant-abc", "sub-def", "", "") require.NoError(t, err, "profile write failure must be non-fatal") _, statErr := os.Stat(profilePath) @@ -1965,7 +1969,7 @@ func TestUpdateAzureCLIFiles_ServicePrincipalEntriesWriteFailureIsNonFatal(t *te FederatedToken: "federated-token-value", } - err := UpdateAzureCLIFiles(creds, "sp-tenant", "sp-sub", "") + err := UpdateAzureCLIFiles(creds, "sp-tenant", "sp-sub", "", "") require.NoError(t, err, "service principal entries write failure must be non-fatal") _, statErr := os.Stat(entriesPath) diff --git a/pkg/auth/identities/azure/subscription.go b/pkg/auth/identities/azure/subscription.go index 5ad96e91e1..d68379a08a 100644 --- a/pkg/auth/identities/azure/subscription.go +++ b/pkg/auth/identities/azure/subscription.go @@ -190,7 +190,7 @@ func (i *subscriptionIdentity) PostAuthenticate(ctx context.Context, params *aut // This ensures azuread and azapi providers can authenticate using Azure CLI credentials. azureCreds, ok := params.Credentials.(*authTypes.AzureCredentials) if ok { - if err := azureCloud.UpdateAzureCLIFiles(params.Credentials, azureCreds.TenantID, i.subscriptionID, azureCreds.CloudEnvironment); err != nil { + if err := azureCloud.UpdateAzureCLIFiles(params.Credentials, azureCreds.TenantID, i.subscriptionID, azureCreds.CloudEnvironment, i.realm); err != nil { log.Debug("Failed to update Azure CLI files", "error", err) // Non-fatal - continue with normal flow. } diff --git a/pkg/auth/providers/azure/device_code_cache.go b/pkg/auth/providers/azure/device_code_cache.go index 6116e7bb47..2cb819fc01 100644 --- a/pkg/auth/providers/azure/device_code_cache.go +++ b/pkg/auth/providers/azure/device_code_cache.go @@ -243,6 +243,10 @@ func (p *deviceCodeProvider) updateAzureCLICache(update *tokenCacheUpdate) error cache, accessTokenSection, accountSection := p.loadAndInitializeCLICache(msalCachePath) cacheKey := p.populateCLICacheWithTokens(accessTokenSection, accountSection, userOID, username, update) + // Copy refresh tokens from the Atmos realm cache so az can self-mint any + // audience and survive access-token expiry. + azureCloud.CopyAtmosRefreshTokensInto(cache, home, p.realm, update.HomeAccountID) + // Write updated cache. updatedData, err := json.MarshalIndent(cache, "", " ") if err != nil { @@ -330,7 +334,8 @@ func (p *deviceCodeProvider) populateCLICacheWithTokens( // IMPORTANT: Use only ".default" scope to match Azure CLI's token lookup. // Azure CLI looks up tokens using the management scope as the cache key. // Using a different scope format (like adding user_impersonation) causes lookup failures. - cacheKey := addTokenToCLICache(accessTokenSection, update.AccessToken, update.ExpiresAt, p.cloudEnv.ManagementScope, ids) + cacheKey := addTokenToCLICache(accessTokenSection, update.AccessToken, update.ExpiresAt, + strings.Join(append([]string{p.cloudEnv.ManagementScope}, p.cloudEnv.LegacyManagementScopes...), " "), ids) // Add Graph API and KeyVault tokens if available. addOptionalCLITokens(accessTokenSection, update, ids, p.cloudEnv) diff --git a/pkg/auth/providers/azure/token_audience_test.go b/pkg/auth/providers/azure/token_audience_test.go new file mode 100644 index 0000000000..5a6cfbe00b --- /dev/null +++ b/pkg/auth/providers/azure/token_audience_test.go @@ -0,0 +1,117 @@ +package azure + +import ( + "encoding/base64" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + azureCloud "github.com/cloudposse/atmos/pkg/auth/cloud/azure" + "github.com/cloudposse/atmos/pkg/schema" +) + +// Regression tests for the field failure where azapi-based modules (all modern +// AVM modules) died mid-apply with "AzureCLICredential: ERROR: Can't find token +// from MSAL cache": the seeded Azure CLI cache held the management token only +// under the modern ARM scope (management.azure.com), while azidentity/az request +// the LEGACY ARM audience (management.core.windows.net) by default — and no +// refresh token was seeded, so az could neither serve nor mint it. + +func seededProvider(t *testing.T) (*deviceCodeProvider, string) { + t.Helper() + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + + p, err := NewDeviceCodeProvider("test-provider", &schema.Provider{ + Kind: "azure/device-code", + Spec: map[string]interface{}{"tenant_id": "tenant-123"}, + }) + require.NoError(t, err) + p.SetRealm("test-realm") + return p, tmpHome +} + +func seededToken(t *testing.T) string { + t.Helper() + header := base64.RawURLEncoding.EncodeToString([]byte(`{"typ":"JWT","alg":"RS256"}`)) + payload := base64.RawURLEncoding.EncodeToString([]byte(`{"oid":"user-oid","upn":"user@example.com"}`)) + return header + "." + payload + ".sig" +} + +func azCache(t *testing.T, tmpHome string) map[string]map[string]map[string]interface{} { + t.Helper() + data, err := os.ReadFile(filepath.Join(tmpHome, ".azure", "msal_token_cache.json")) + require.NoError(t, err) + var cache map[string]map[string]map[string]interface{} + require.NoError(t, json.Unmarshal(data, &cache)) + return cache +} + +func TestAzureCLICache_ManagementTokenCoversLegacyAudience(t *testing.T) { + p, tmpHome := seededProvider(t) + + require.NoError(t, p.updateAzureCLICache(&tokenCacheUpdate{ + AccessToken: seededToken(t), + ExpiresAt: time.Now().UTC().Add(1 * time.Hour), + HomeAccountID: "home-oid.home-tenant", + })) + + cache := azCache(t, tmpHome) + var target string + for _, entry := range cache["AccessToken"] { + if tgt, _ := entry["target"].(string); strings.Contains(tgt, "management.azure.com") { + target = tgt + } + } + require.NotEmpty(t, target, "management token entry expected") + assert.Contains(t, target, "https://management.core.windows.net/.default", + "target must include the legacy ARM audience so az's default lookup (and azidentity/azapi) hit the cache") + assert.Contains(t, target, "https://management.core.windows.net//.default", + "target must include the double-slash legacy form az derives from the trailing-slash resource") +} + +func TestAzureCLICache_RefreshTokenCopiedFromAtmosCache(t *testing.T) { + p, tmpHome := seededProvider(t) + + // Seed the ATMOS realm MSAL cache with a refresh token for the account, + // as MSAL persists after a device-code/interactive login. + atmosCacheDir := filepath.Join(tmpHome, ".azure", "atmos", "test-realm") + require.NoError(t, os.MkdirAll(atmosCacheDir, 0o700)) + atmosCache := map[string]interface{}{ + "RefreshToken": map[string]interface{}{ + "home-oid.home-tenant-login.microsoftonline.com-refreshtoken-04b07795-8ddb-461a-bbee-02f9e1bf7b46--": map[string]interface{}{ + "home_account_id": "home-oid.home-tenant", + "environment": "login.microsoftonline.com", + "client_id": "04b07795-8ddb-461a-bbee-02f9e1bf7b46", + "credential_type": "RefreshToken", + "secret": "fake-refresh-token", + "family_id": "1", + }, + }, + } + data, err := json.Marshal(atmosCache) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(atmosCacheDir, "msal_token_cache.json"), data, 0o600)) + + require.NoError(t, p.updateAzureCLICache(&tokenCacheUpdate{ + AccessToken: seededToken(t), + ExpiresAt: time.Now().UTC().Add(1 * time.Hour), + HomeAccountID: "home-oid.home-tenant", + })) + + cache := azCache(t, tmpHome) + require.NotEmpty(t, cache["RefreshToken"], + "refresh token must be copied into the az cache so az can self-mint any audience and survive access-token expiry") + for _, entry := range cache["RefreshToken"] { + assert.Equal(t, "home-oid.home-tenant", entry["home_account_id"]) + assert.Equal(t, "fake-refresh-token", entry["secret"]) + } + _ = azureCloud.GetCloudEnvironment("") // anchor import +} From 762b4a65aa2648f2f0d091021fd2a641427ffc7e Mon Sep 17 00:00:00 2001 From: aknysh Date: Thu, 6 Aug 2026 11:30:57 -0400 Subject: [PATCH 2/3] test(auth): address review feedback on Azure refresh-token copy - Assert LegacyManagementScopes for all clouds (public, usgovernment, china, and both default-to-public cases) in TestGetCloudEnvironment. - Add table-driven tests for CopyAtmosRefreshTokensInto covering matching/mismatched account IDs, empty realm, empty home account ID, missing cache file, invalid JSON, missing RefreshToken section, non-object entries, and preservation of pre-existing az entries. - Add perf.Track to CopyAtmosRefreshTokensInto per coding guidelines. Co-Authored-By: Claude Fable 5 --- .../cloud/azure/cloud_environments_test.go | 7 + pkg/auth/cloud/azure/refresh_token.go | 3 + pkg/auth/cloud/azure/refresh_token_test.go | 157 ++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 pkg/auth/cloud/azure/refresh_token_test.go diff --git a/pkg/auth/cloud/azure/cloud_environments_test.go b/pkg/auth/cloud/azure/cloud_environments_test.go index fd9df99cfa..40ed58a13a 100644 --- a/pkg/auth/cloud/azure/cloud_environments_test.go +++ b/pkg/auth/cloud/azure/cloud_environments_test.go @@ -18,6 +18,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName string expectedLogin string expectedMgmt string + expectedLegacyMgmt []string expectedGraph string expectedKeyVault string expectedBlobSufx string @@ -30,6 +31,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName: "public", expectedLogin: "login.microsoftonline.com", expectedMgmt: "https://management.azure.com/.default", + expectedLegacyMgmt: []string{"https://management.core.windows.net/.default", "https://management.core.windows.net//.default"}, expectedGraph: "https://graph.microsoft.com/.default", expectedKeyVault: "https://vault.azure.net/.default", expectedBlobSufx: "blob.core.windows.net", @@ -42,6 +44,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName: "usgovernment", expectedLogin: "login.microsoftonline.us", expectedMgmt: "https://management.usgovcloudapi.net/.default", + expectedLegacyMgmt: []string{"https://management.core.usgovcloudapi.net/.default", "https://management.core.usgovcloudapi.net//.default"}, expectedGraph: "https://graph.microsoft.us/.default", expectedKeyVault: "https://vault.usgovcloudapi.net/.default", expectedBlobSufx: "blob.core.usgovcloudapi.net", @@ -54,6 +57,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName: "china", expectedLogin: "login.chinacloudapi.cn", expectedMgmt: "https://management.chinacloudapi.cn/.default", + expectedLegacyMgmt: []string{"https://management.core.chinacloudapi.cn/.default", "https://management.core.chinacloudapi.cn//.default"}, expectedGraph: "https://microsoftgraph.chinacloudapi.cn/.default", expectedKeyVault: "https://vault.azure.cn/.default", expectedBlobSufx: "blob.core.chinacloudapi.cn", @@ -66,6 +70,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName: "public", expectedLogin: "login.microsoftonline.com", expectedMgmt: "https://management.azure.com/.default", + expectedLegacyMgmt: []string{"https://management.core.windows.net/.default", "https://management.core.windows.net//.default"}, expectedGraph: "https://graph.microsoft.com/.default", expectedKeyVault: "https://vault.azure.net/.default", expectedBlobSufx: "blob.core.windows.net", @@ -78,6 +83,7 @@ func TestGetCloudEnvironment(t *testing.T) { expectedName: "public", expectedLogin: "login.microsoftonline.com", expectedMgmt: "https://management.azure.com/.default", + expectedLegacyMgmt: []string{"https://management.core.windows.net/.default", "https://management.core.windows.net//.default"}, expectedGraph: "https://graph.microsoft.com/.default", expectedKeyVault: "https://vault.azure.net/.default", expectedBlobSufx: "blob.core.windows.net", @@ -93,6 +99,7 @@ func TestGetCloudEnvironment(t *testing.T) { assert.Equal(t, tt.expectedName, env.Name) assert.Equal(t, tt.expectedLogin, env.LoginEndpoint) assert.Equal(t, tt.expectedMgmt, env.ManagementScope) + assert.Equal(t, tt.expectedLegacyMgmt, env.LegacyManagementScopes) assert.Equal(t, tt.expectedGraph, env.GraphAPIScope) assert.Equal(t, tt.expectedKeyVault, env.KeyVaultScope) assert.Equal(t, tt.expectedBlobSufx, env.BlobStorageSuffix) diff --git a/pkg/auth/cloud/azure/refresh_token.go b/pkg/auth/cloud/azure/refresh_token.go index a9c0491f7e..3cc9e63c9d 100644 --- a/pkg/auth/cloud/azure/refresh_token.go +++ b/pkg/auth/cloud/azure/refresh_token.go @@ -6,6 +6,7 @@ import ( "path/filepath" log "github.com/cloudposse/atmos/pkg/logger" + "github.com/cloudposse/atmos/pkg/perf" ) // CopyAtmosRefreshTokensInto copies refresh-token entries for the given account @@ -18,6 +19,8 @@ import ( // without it, az fails with "Can't find token from MSAL cache" as soon as a // lookup misses the seeded access tokens. func CopyAtmosRefreshTokensInto(azCache map[string]interface{}, home, realm, homeAccountID string) { + defer perf.Track(nil, "pkg/auth/cloud/azure.CopyAtmosRefreshTokensInto")() + if realm == "" || homeAccountID == "" { // Without a realm the Atmos cache path IS the az cache (self-copy); // without a home account ID entries can't be matched safely. diff --git a/pkg/auth/cloud/azure/refresh_token_test.go b/pkg/auth/cloud/azure/refresh_token_test.go new file mode 100644 index 0000000000..8dfa5e1f62 --- /dev/null +++ b/pkg/auth/cloud/azure/refresh_token_test.go @@ -0,0 +1,157 @@ +package azure + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeAtmosRealmCache writes an MSAL cache file at the Atmos realm path under home. +func writeAtmosRealmCache(t *testing.T, home, realm string, content []byte) { + t.Helper() + dir := filepath.Join(home, ".azure", "atmos", realm) + require.NoError(t, os.MkdirAll(dir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "msal_token_cache.json"), content, 0o600)) +} + +func rtEntry(homeAccountID string) map[string]interface{} { + return map[string]interface{}{ + "home_account_id": homeAccountID, + "credential_type": "RefreshToken", + "secret": "fake-refresh-token", + } +} + +func TestCopyAtmosRefreshTokensInto(t *testing.T) { + const accountID = "oid-123.tenant-456" + + marshal := func(t *testing.T, v interface{}) []byte { + t.Helper() + data, err := json.Marshal(v) + require.NoError(t, err) + return data + } + + tests := []struct { + name string + realm string + homeAccountID string + atmosCache []byte // nil = no cache file written + azCache map[string]interface{} + expectCopied int + }{ + { + name: "matching entry is copied", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{"rt-key": rtEntry(accountID)}, + }), + azCache: map[string]interface{}{}, + expectCopied: 1, + }, + { + name: "mismatched home account ID is skipped", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{"rt-key": rtEntry("other-account")}, + }), + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "empty realm is a no-op", + realm: "", + homeAccountID: accountID, + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "empty home account ID is a no-op", + realm: "test-realm", + homeAccountID: "", + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{"rt-key": rtEntry(accountID)}, + }), + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "missing Atmos cache file is a no-op", + realm: "test-realm", + homeAccountID: accountID, + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "invalid JSON in Atmos cache is a no-op", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: []byte("{not json"), + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "missing RefreshToken section is a no-op", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{"AccessToken": map[string]interface{}{}}), + azCache: map[string]interface{}{}, + expectCopied: 0, + }, + { + name: "non-object entries are skipped", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{ + "bad-key": "not an object", + "good-key": rtEntry(accountID), + }, + }), + azCache: map[string]interface{}{}, + expectCopied: 1, + }, + { + name: "existing az RefreshToken section is preserved and appended to", + realm: "test-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{"rt-key": rtEntry(accountID)}, + }), + azCache: map[string]interface{}{ + "RefreshToken": map[string]interface{}{"pre-existing": rtEntry("other-account")}, + }, + expectCopied: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + home := t.TempDir() + if tt.atmosCache != nil { + writeAtmosRealmCache(t, home, "test-realm", tt.atmosCache) + } + + preExisting := 0 + if dest, ok := tt.azCache["RefreshToken"].(map[string]interface{}); ok { + preExisting = len(dest) + } + + CopyAtmosRefreshTokensInto(tt.azCache, home, tt.realm, tt.homeAccountID) + + dest, _ := tt.azCache["RefreshToken"].(map[string]interface{}) + assert.Len(t, dest, preExisting+tt.expectCopied) + for _, raw := range dest { + entry, ok := raw.(map[string]interface{}) + require.True(t, ok, "copied entries must be objects") + assert.NotEmpty(t, entry["home_account_id"]) + } + }) + } +} From ccd7705521f57840ef178f4d3fef6111c9471e19 Mon Sep 17 00:00:00 2001 From: aknysh Date: Thu, 6 Aug 2026 11:40:14 -0400 Subject: [PATCH 3/3] test(auth): strengthen refresh-token copy test assertions - Write the source cache under the table case's realm (was hard-coded), and add a matching case with a non-default realm so realm selection is actually exercised. - Assert copied cache entries exactly by key and value (including the preserved pre-existing Azure CLI entry) instead of length only. Co-Authored-By: Claude Fable 5 --- pkg/auth/cloud/azure/refresh_token_test.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/auth/cloud/azure/refresh_token_test.go b/pkg/auth/cloud/azure/refresh_token_test.go index 8dfa5e1f62..ac1815b27f 100644 --- a/pkg/auth/cloud/azure/refresh_token_test.go +++ b/pkg/auth/cloud/azure/refresh_token_test.go @@ -43,6 +43,7 @@ func TestCopyAtmosRefreshTokensInto(t *testing.T) { atmosCache []byte // nil = no cache file written azCache map[string]interface{} expectCopied int + expectDest map[string]interface{} // nil = length check only }{ { name: "matching entry is copied", @@ -53,6 +54,18 @@ func TestCopyAtmosRefreshTokensInto(t *testing.T) { }), azCache: map[string]interface{}{}, expectCopied: 1, + expectDest: map[string]interface{}{"rt-key": rtEntry(accountID)}, + }, + { + name: "matching entry in a non-default realm is copied", + realm: "custom-realm", + homeAccountID: accountID, + atmosCache: marshal(t, map[string]interface{}{ + "RefreshToken": map[string]interface{}{"rt-key": rtEntry(accountID)}, + }), + azCache: map[string]interface{}{}, + expectCopied: 1, + expectDest: map[string]interface{}{"rt-key": rtEntry(accountID)}, }, { name: "mismatched home account ID is skipped", @@ -128,6 +141,10 @@ func TestCopyAtmosRefreshTokensInto(t *testing.T) { "RefreshToken": map[string]interface{}{"pre-existing": rtEntry("other-account")}, }, expectCopied: 1, + expectDest: map[string]interface{}{ + "pre-existing": rtEntry("other-account"), + "rt-key": rtEntry(accountID), + }, }, } @@ -135,7 +152,7 @@ func TestCopyAtmosRefreshTokensInto(t *testing.T) { t.Run(tt.name, func(t *testing.T) { home := t.TempDir() if tt.atmosCache != nil { - writeAtmosRealmCache(t, home, "test-realm", tt.atmosCache) + writeAtmosRealmCache(t, home, tt.realm, tt.atmosCache) } preExisting := 0 @@ -147,6 +164,9 @@ func TestCopyAtmosRefreshTokensInto(t *testing.T) { dest, _ := tt.azCache["RefreshToken"].(map[string]interface{}) assert.Len(t, dest, preExisting+tt.expectCopied) + if tt.expectDest != nil { + assert.Equal(t, tt.expectDest, dest) + } for _, raw := range dest { entry, ok := raw.(map[string]interface{}) require.True(t, ok, "copied entries must be objects")