Fix azd auth status reporting unauthenticated in Cloud Shell#8459
Open
JeffreyCA wants to merge 2 commits into
Open
Fix azd auth status reporting unauthenticated in Cloud Shell#8459JeffreyCA wants to merge 2 commits into
JeffreyCA wants to merge 2 commits into
Conversation
LogInDetails was the only user-state auth method in auth.Manager missing a Cloud Shell fallback. Its siblings CredentialForCurrentUser and GetLoggedInServicePrincipalTenantID both fall back to the ambient Cloud Shell credential when readUserProperties returns ErrNoCurrentUser, but LogInDetails returned the error because in Cloud Shell the user never runs `azd auth login`. This single gap caused two symptoms in Cloud Shell: - `azd provision` failed because CurrentPrincipalType calls LogInDetails. - `azd ai agent init` blocked because it parses `azd auth status`, which reported unauthenticated. Add a Cloud Shell fallback to LogInDetails that derives the user identity from the ambient credential, reporting a User (email) login with a best-effort account from token claims. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates azd authentication status detection so Azure Cloud Shell sessions are reported as authenticated (using the existing ambient Cloud Shell credential) even when no azd auth login user properties are stored.
Changes:
- Add a Cloud Shell fallback path in
auth.Manager.LogInDetails()that derives login details from token claims obtained via the ambient credential. - Add unit tests covering Cloud Shell
LogInDetails()behavior both with and without a username claim in the token.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cli/azd/pkg/auth/manager.go | Adds Cloud Shell fallback logic and a helper to derive LogInDetails from ambient credential claims. |
| cli/azd/pkg/auth/manager_test.go | Adds TestLogInDetails cases validating Cloud Shell authentication reporting with/without username claims. |
Only fall back to the ambient Cloud Shell credential when readUserProperties reports no current user. Other errors (e.g. corrupted stored user properties) now surface as a wrapped error instead of being masked by the fallback or remapped to ErrNoCurrentUser, so real config corruption isn't silently hidden. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JeffreyCA
requested review from
RickWinter,
hemarina,
tg-msft and
vhvb1989
as code owners
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
hemarina
approved these changes
Jun 2, 2026
Contributor
hemarina
left a comment
hemarina
left a comment
There was a problem hiding this comment.
Thanks for the fix — this is a real Cloud Shell papercut and the scoping is right. Commit 2 already addressed the obvious concern about masking non-ErrNoCurrentUser errors, and the three new subtests cover the happy paths plus the corruption negative cleanly. Approving — leaving a couple of small items worth resolving before merge plus one optional follow-up.
| currentUser, err := readUserProperties(cfg) | ||
| if err != nil { | ||
| return nil, ErrNoCurrentUser | ||
| // In Cloud Shell azd uses the ambient credential, so report that user |
Contributor
There was a problem hiding this comment.
LogInDetails Godoc (lines 1399-1404) is now stale. It enumerates the legacy-auth and external-auth paths but doesn't mention this new Cloud Shell fallback. Mind adding a sentence so the behavior contract stays in sync? Something like:
// When running in Azure Cloud Shell and no azd-managed user is logged in,
// it derives the account from the ambient Cloud Shell credential and reports
// an authenticated user.| // rather than treating the session as unauthenticated. Only fall back | ||
| // when there is genuinely no logged-in user; other errors (e.g. corrupted | ||
| // stored user properties) should surface so they aren't silently hidden. | ||
| if errors.Is(err, ErrNoCurrentUser) { |
Contributor
There was a problem hiding this comment.
PR description is slightly inaccurate re: sibling parity (and there's a latent inconsistency worth noting). The description says:
"Its siblings
CredentialForCurrentUserandGetLoggedInServicePrincipalTenantIDboth fall back to the ambient Cloud Shell credential whenreadUserPropertiesreturnsErrNoCurrentUser."
That's true of CredentialForCurrentUser (line 276 — if errors.Is(err, ErrNoCurrentUser)), but GetLoggedInServicePrincipalTenantID (line 466) falls back on any error from readUserProperties:
currentUser, err := readUserProperties(authCfg)
if err != nil { // <-- catches corruption too
if runcontext.IsRunningInCloudShell() { ... }
return nil, ErrNoCurrentUser
}So after this PR, corrupted currentUser config in Cloud Shell will surface a real error from LogInDetails (good — what the new test 3 locks in), but be silently masked by GetLoggedInServicePrincipalTenantID.
No change required in this PR — the new behavior is the right one. Just worth either (a) updating the PR description so it doesn't claim parity that doesn't exist, or (b) filing a follow-up to tighten GetLoggedInServicePrincipalTenantID the same way.
|
|
||
| _, err := m.LogInDetails(t.Context()) | ||
| require.Error(t, err) | ||
| require.NotErrorIs(t, err, ErrNoCurrentUser) |
Contributor
There was a problem hiding this comment.
Nit (optional): This test asserts the error doesn't unwrap to ErrNoCurrentUser, which is the important part. Since the PR also establishes "reading current user properties: %w" as the wrap contract, consider locking that in so a future refactor that drops the wrap is caught:
require.ErrorContains(t, err, "reading current user properties")
vhvb1989
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #8458
This PR fixes azd incorrectly reporting an unauthenticated state in Azure Cloud Shell, even though Cloud Shell provides an ambient managed credential that azd already uses for token acquisition.
Without changes:
With changes:
Root cause
auth.Manager.LogInDetails()was the only user-state auth method missing a Cloud Shell fallback. Its siblingsCredentialForCurrentUserandGetLoggedInServicePrincipalTenantIDboth fall back to the ambient Cloud Shell credential whenreadUserPropertiesreturnsErrNoCurrentUser.LogInDetailsinstead returned the error, because in Cloud Shell the user never runsazd auth loginand therefore has no stored user properties.Impact
That single gap surfaced as two distinct, confusing failures in Cloud Shell:
azd provisionfailed during parameter loading, because the Bicep provider callsCurrentPrincipalType, which callsLogInDetailsand propagated the error as "fetching current principal type".azd ai agent initblocked with a "not logged in" message, because it gates on parsingazd auth status --output json, which reportedunauthenticatedfor the same reason.Both flows actually work in Cloud Shell once the underlying login-state check is corrected, so no extension-side change is required.
Fix
The fix adds a Cloud Shell fallback to
LogInDetailsthat mirrors the existing pattern in the other auth methods. When no user is logged in via azd's own flow but the process is running in Cloud Shell, it derives the identity from the ambient credential and reports a User (email) login, populating the account from the token claims on a best-effort basis. An empty account is intentionally not treated as an error, since a Cloud Shell session is always a valid authenticated user and the principal type is what downstream callers such asCurrentPrincipalTypeactually depend on.As a result,
azd auth statusnow reports authenticated in Cloud Shell,azd provisionresolves the principal type correctly, andazd ai agent initproceeds.Testing
Test coverage was added to
TestLogInDetailsfor both the case where the Cloud Shell token exposes a username claim and the case where it does not, asserting that the session is reported as an authenticated User login in both. Existing auth, provisioning, and command tests continue to pass, including under-race.