Handle MSAL token cache persistence failures gracefully (WSL support)#6079
Open
lewing wants to merge 2 commits intodotnet:mainfrom
Open
Handle MSAL token cache persistence failures gracefully (WSL support)#6079lewing wants to merge 2 commits intodotnet:mainfrom
lewing wants to merge 2 commits intodotnet:mainfrom
Conversation
When the D-Bus secrets service (gnome-keyring) is unavailable — common in WSL environments — MSAL's VerifyPersistence() throws MsalCachePersistenceException before any authentication can occur, causing darc login and all authenticated commands to fail hard. Changes: - CachedInteractiveBrowserCredential: catch MsalCachePersistenceException in GetToken/GetTokenAsync and recreate credentials without TokenCachePersistenceOptions (in-memory only). Extract shared RecreateCredentialsWithoutPersistence() and IsMsalCachePersistenceException() helpers. - AppCredential: chain AzureCliCredential after the interactive credential via ChainedTokenCredential so users with 'az login' can authenticate without interactive flows when the keyring is unavailable. Fixes dotnet#6060 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves darc authentication reliability on WSL by handling MSAL token cache persistence failures (e.g., missing org.freedesktop.secrets) and introducing an Azure CLI-based fallback path.
Changes:
- Catch
MsalCachePersistenceExceptionduring token acquisition and fall back to non-persistent (in-memory) credentials. - Convert certain post-fallback interactive failures into
CredentialUnavailableExceptionso credential chaining can continue. - Add
AzureCliCredentialinto the user credential chain to enableaz loginreuse.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs | Adds persistence-failure handling around GetToken/GetTokenAsync, refactors helpers for recreating credentials and detecting persistence exceptions. |
| src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs | Wraps the interactive credential in a ChainedTokenCredential with AzureCliCredential as an additional option. |
…lation - RecreateCredentialsWithoutPersistence now preserves _options.AuthenticationRecord to avoid extra consent/device-code prompts when an auth record exists on disk - Retry path exception filter now checks cancellationToken.IsCancellationRequested and inner OperationCanceledException to let user-initiated cancellations propagate instead of wrapping them as CredentialUnavailableException - Add explanatory comment on ChainedTokenCredential ordering rationale Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
premun
approved these changes
Mar 11, 2026
Member
|
LGTM @lewing did you test this? |
Member
Author
partially, I can do a full test before merging though. |
Member
|
Please run one if you have the environment ready. |
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.
Summary
darc loginand all authenticated darc commands fail hard on WSL when the D-Bus secrets service (org.freedesktop.secrets/ gnome-keyring) is unavailable. MSAL'sVerifyPersistence()throwsMsalCachePersistenceExceptionbefore any authentication can occur, with no recovery.Fixes #6060
Changes
CachedInteractiveBrowserCredential.csMsalCachePersistenceExceptioninGetToken/GetTokenAsync(not just inCacheAuthenticationRecord) and recreate credentials withoutTokenCachePersistenceOptions(in-memory only token cache)CredentialUnavailableExceptionsoChainedTokenCredentialcan try the next credentialRecreateCredentialsWithoutPersistence()andIsMsalCachePersistenceException()as shared helpers (previously the recreation was inline and the exception check was a local function)AppCredential.csAzureCliCredentialafter the interactive credential inCreateUserCredential()viaChainedTokenCredential, so users withaz logincan authenticate without interactive flows when the keyring is unavailableBehavior
MsalCachePersistenceExceptionAzureCliCredentialsilentlySecurity Notes
TokenCachePersistenceOptionsmeans tokens are held in-memory only when the keyring is unavailable — this is more secure (no persistence), at the cost of re-authentication per sessionAzureCliCredentialreuses the user's existingaz loginsession — no new credential storage introducedCredentialUnavailableExceptionwrapping is scoped narrowly: only when the persistence fallback path also fails at interactive auth, not on allAuthenticationFailedException