Handle MSAL token cache persistence failures gracefully (WSL support)#6079
Open
lewing wants to merge 7 commits into
Open
Handle MSAL token cache persistence failures gracefully (WSL support)#6079lewing wants to merge 7 commits into
lewing wants to merge 7 commits into
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
previously 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. |
DeviceCodeCredential's default callback writes the device-code prompt to AzureEventSource, which is not routed to console output in CLI hosts (like darc) or MCP server processes. Result: when MSAL persistence is unavailable on WSL and the credential falls back to device-code, the user sees "falling back to device code authentication..." and then darc hangs silently — the device code and URL are emitted to a stream nobody is watching. Set DeviceCodeCallback on both DeviceCodeCredential constructions (initial + post-persistence-fallback) to route the message through the existing _logger, so the user sees "To sign in, use a web browser to open https://microsoft.com/devicelogin and enter the code XXX...". Tested by building darc locally on WSL without keyring; "darc login" now prints the device code instead of hanging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After RecreateCredentialsWithoutPersistence() rebuilds both credentials, CacheAuthenticationRecord retries Authenticate(). The retry was re-entering the InteractiveBrowserCredential path even though the first attempt had already proved the browser flow was unavailable in this environment (the _isDeviceCodeFallback flag was set, but only GetTokenCore checked it — Authenticate did not). On headless WSL the browser credential opens a Windows-side browser but the OAuth redirect to http://localhost:XXXX cannot reach the WSL-side listener, so the second InteractiveBrowserCredential.Authenticate call hangs forever. The device code prompt never gets a chance to print. Honor _isDeviceCodeFallback at the top of Authenticate so the second call goes straight to DeviceCodeCredential. With this fix, "darc login" on WSL prints the device code + URL immediately on the retry instead of hanging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On WSL the interactive browser flow only succeeds when:
1. A Windows-side launcher is available (wslu's `wslview` is the
canonical bridge), and
2. WSL2 localhost forwarding can route the OAuth redirect back into
the WSL network namespace (default NAT mode handles this).
When wslview is present the flow works end-to-end and benefits from
Windows session SSO (which is what gets past Conditional Access).
When wslview is absent, xdg-open silently does nothing and
InteractiveBrowserCredential.Authenticate() blocks forever waiting for
a redirect that will never arrive — in that specific case we skip
straight to device code so the user at least sees a code instead of
an indefinite hang.
Env var overrides:
DARC_FORCE_BROWSER_AUTH=1 always attempt browser flow
DARC_USE_DEVICE_CODE=1 always skip browser flow
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lewing
force-pushed
the
fix/darc-wsl-keyring-fallback
branch
from
6c8a602 to
9088d98
Compare
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