fix(auth): cover legacy ARM audience and seed refresh token in Azure CLI cache - #2890
fix(auth): cover legacy ARM audience and seed refresh token in Azure CLI cache#2890Andriy Knysh (aknysh) wants to merge 3 commits into
Conversation
…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 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAzure authentication now includes legacy ARM audience scopes and copies matching Atmos realm refresh tokens into the Azure CLI MSAL cache. Credential realms pass through cache generation. Regression tests and recovery documentation cover the changes. ChangesAzure CLI cache compatibility
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AzureIdentity
participant UpdateAzureCLIFiles
participant AtmosRealmCache
participant AzureCLIMSALCache
AzureIdentity->>UpdateAzureCLIFiles: pass credential realm
UpdateAzureCLIFiles->>AtmosRealmCache: load refresh tokens
AtmosRealmCache-->>UpdateAzureCLIFiles: return matching account tokens
UpdateAzureCLIFiles->>AzureCLIMSALCache: write legacy audiences and refresh tokens
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/auth/cloud/azure/cloud_environments.go`:
- Around line 60-69: Extend TestGetCloudEnvironment with China
LegacyManagementScopes assertions, verifying both
https://management.core.chinacloudapi.cn/.default and
https://management.core.chinacloudapi.cn//.default alongside the existing China
environment checks and other cloud coverage.
In `@pkg/auth/cloud/azure/refresh_token.go`:
- Around line 64-77: Add table-driven unit tests covering
copyMatchingRefreshTokens and its surrounding refresh-token loading behavior for
matching and mismatched account IDs, empty realm, empty home account ID, missing
cache, and invalid JSON. Verify both copied results and relevant empty/error
outcomes, reusing existing test helpers and fixtures where available.
- Around line 20-41: Add defer perf.Track(atmosConfig,
"pkg.CopyAtmosRefreshTokensInto")() immediately after entering
CopyAtmosRefreshTokensInto, followed by a blank line, using the existing
atmosConfig symbol and preserving the function’s current logic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bfd18431-acfe-45ea-bd8f-8b916ee3d86c
📒 Files selected for processing (8)
docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.mdpkg/auth/cloud/azure/cloud_environments.gopkg/auth/cloud/azure/refresh_token.gopkg/auth/cloud/azure/setup.gopkg/auth/cloud/azure/setup_test.gopkg/auth/identities/azure/subscription.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/providers/azure/token_audience_test.go
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/auth/cloud/azure/refresh_token_test.go`:
- Around line 149-154: Strengthen the assertions in the refresh-token copy test
by checking copied cache entries by key and exact value, including the expected
source entry and the unchanged "pre-existing" Azure CLI entry. Verify fields
such as home_account_id and secret rather than relying only on destination
length and non-empty values, while preserving the existing type validation.
- Line 138: Update the test setup around writeAtmosRealmCache to write the
source cache under tt.realm instead of the hard-coded "test-realm". Add a
table-test case with a non-default realm value so CopyAtmosRefreshTokensInto
verifies it reads the selected Atmos realm.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 616f478a-a701-45da-8d95-6a9653b77ed5
📒 Files selected for processing (3)
pkg/auth/cloud/azure/cloud_environments_test.gopkg/auth/cloud/azure/refresh_token.gopkg/auth/cloud/azure/refresh_token_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/auth/cloud/azure/refresh_token.go
- 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 <noreply@anthropic.com>
what
targetfield — the modern scope (https://management.azure.com/.default) plus the legacy audience forms (https://management.core.windows.net/.defaultand the double-slash variant), with matching forms for the US Government and China clouds (newLegacyManagementScopesfield onCloudEnvironment).~/.azure/atmos/<realm>/msal_token_cache.json) into the Azure CLI cache after login (newCopyAtmosRefreshTokensInto;UpdateAzureCLIFilesgains arealmparameter). Skipped for service principals, empty realms, or unmatched home account IDs.pkg/auth/providers/azure/token_audience_test.go).docs/fixes/2026-08-06-azure-cli-cache-legacy-audience-refresh-token.md.why
atmos auth login, Terraform providers that authenticate viaAzureCLICredentialrequest an ARM token for the legacy audiencehttps://management.core.windows.net/(the azidentity/azapi default). The cache write-back only seeded the modern scope, so MSAL's cache lookup missed andazapi-based modules (all modern Azure Verified Modules) failed mid-apply withAzureCLICredential: ERROR: Can't find token from MSAL cache— whileazurermresources in the same apply succeeded. Observed in a real cold-start apply of a state backend component.target, and ARM accepts both audiences interchangeably, so a single entry carrying every form satisfies every lookup.az-side lookup failed the same way. Atmos authenticates with the Azure CLI's own public client ID, so the refresh token in the Atmos realm cache is directly usable byaz— seeding it letsazself-mint tokens for any audience and survive access-token expiry.az loginalongsideatmos auth login, defeating the purpose of single-command auth.Manually verified end-to-end on a real Azure tenant:
az logout,az account clear, removed~/.azure/msal_token_cache.jsonand~/.azure/atmos/(confirmed withaz account showfailing).atmos auth loginalone — noaz loginat any point.jq '.RefreshToken | length' ~/.azure/msal_token_cache.jsonreturned1(previously0).azidentity/azapimake:az account get-access-token --resource https://management.core.windows.net/succeeded (previously failed withCan't find token from MSAL cache). Its expiry matched the login session's, proving MSAL served it from the seeded multi-audience entry via subset matching rather than minting a new token.atmos terraform planon anazapi-heavy component (the exact field failure): refresh and plan completed clean with no MSAL errors.references
azure/interactiveprovider, shipped in v1.225.0)Summary by CodeRabbit
Bug Fixes
az loginprompts.Documentation