From 51eb25253aee12f0ef16a719b0ad218f249b9619 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 10 Mar 2026 10:00:00 -0500 Subject: [PATCH 1/5] Handle MSAL token cache persistence failures gracefully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #6060 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AppCredentials/AppCredential.cs | 3 +- .../CachedInteractiveBrowserCredential.cs | 83 +++++++++++++++---- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs index 1c7ff52abe..b4780f7148 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs @@ -50,7 +50,8 @@ public static AppCredential CreateUserCredential(string appId, string userScope public static AppCredential CreateUserCredential(string appId, TokenRequestContext requestContext) { var authRecordPath = Path.Combine(AUTH_CACHE, $"{AUTH_RECORD_PREFIX}-{appId}"); - var credential = GetInteractiveCredential(appId, authRecordPath); + var interactiveCredential = GetInteractiveCredential(appId, authRecordPath); + var credential = new ChainedTokenCredential(interactiveCredential, new AzureCliCredential()); return new AppCredential(credential, requestContext); } diff --git a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs index 915aa96215..51edf767d6 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs @@ -52,6 +52,54 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell { CacheAuthenticationRecord(requestContext, cancellationToken); + try + { + return GetTokenCore(requestContext, cancellationToken); + } + catch (Exception e) when (IsMsalCachePersistenceException(e)) + { + RecreateCredentialsWithoutPersistence(); + try + { + return GetTokenCore(requestContext, cancellationToken); + } + catch (AuthenticationFailedException retryEx) + { + // After persistence fallback, if interactive auth still fails, signal credential unavailability + // so ChainedTokenCredential can try the next credential (e.g. AzureCliCredential) + throw new CredentialUnavailableException( + "Interactive authentication failed after token cache persistence fallback. " + + "Ensure a browser or device code flow is available, or use 'az login' as a fallback.", retryEx); + } + } + } + + public override async ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) + { + CacheAuthenticationRecord(requestContext, cancellationToken); + + try + { + return await GetTokenCoreAsync(requestContext, cancellationToken); + } + catch (Exception e) when (IsMsalCachePersistenceException(e)) + { + RecreateCredentialsWithoutPersistence(); + try + { + return await GetTokenCoreAsync(requestContext, cancellationToken); + } + catch (AuthenticationFailedException retryEx) + { + throw new CredentialUnavailableException( + "Interactive authentication failed after token cache persistence fallback. " + + "Ensure a browser or device code flow is available, or use 'az login' as a fallback.", retryEx); + } + } + } + + private AccessToken GetTokenCore(TokenRequestContext requestContext, CancellationToken cancellationToken) + { if (Volatile.Read(ref _isDeviceCodeFallback) == 1) { return _deviceCodeCredential.GetToken(requestContext, cancellationToken); @@ -68,10 +116,8 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell } } - public override async ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) + private async ValueTask GetTokenCoreAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) { - CacheAuthenticationRecord(requestContext, cancellationToken); - if (Volatile.Read(ref _isDeviceCodeFallback) == 1) { return await _deviceCodeCredential.GetTokenAsync(requestContext, cancellationToken); @@ -109,9 +155,6 @@ private void CacheAuthenticationRecord(TokenRequestContext requestContext, Cance Directory.CreateDirectory(authRecordDir); } - static bool IsMsalCachePersistenceException(Exception e) => - e is MsalCachePersistenceException || (e.InnerException is not null && IsMsalCachePersistenceException(e.InnerException)); - AuthenticationRecord authRecord; try { @@ -121,16 +164,7 @@ static bool IsMsalCachePersistenceException(Exception e) => catch (Exception e) when (IsMsalCachePersistenceException(e)) { // If we cannot persist the token cache, fall back to interactive authentication without persistence - _browserCredential = new InteractiveBrowserCredential(new InteractiveBrowserCredentialOptions() - { - TenantId = _options.TenantId, - ClientId = _options.ClientId, - }); - _deviceCodeCredential = new DeviceCodeCredential(new() - { - TenantId = _options.TenantId, - ClientId = _options.ClientId, - }); + RecreateCredentialsWithoutPersistence(); authRecord = Authenticate(requestContext, cancellationToken); } @@ -153,4 +187,21 @@ private AuthenticationRecord Authenticate(TokenRequestContext requestContext, Ca return _deviceCodeCredential.Authenticate(requestContext, cancellationToken); } } + + private void RecreateCredentialsWithoutPersistence() + { + _browserCredential = new InteractiveBrowserCredential(new InteractiveBrowserCredentialOptions() + { + TenantId = _options.TenantId, + ClientId = _options.ClientId, + }); + _deviceCodeCredential = new DeviceCodeCredential(new() + { + TenantId = _options.TenantId, + ClientId = _options.ClientId, + }); + } + + private static bool IsMsalCachePersistenceException(Exception e) => + e is MsalCachePersistenceException || (e.InnerException is not null && IsMsalCachePersistenceException(e.InnerException)); } From 73dc02918a86dcc3ff166431b3f8197ce2443f55 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 10 Mar 2026 10:39:52 -0500 Subject: [PATCH 2/5] Address review feedback: preserve AuthenticationRecord, handle cancellation - 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> --- .../Maestro.Common/AppCredentials/AppCredential.cs | 4 ++++ .../CachedInteractiveBrowserCredential.cs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs index b4780f7148..011cfa1dc3 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs @@ -51,6 +51,10 @@ public static AppCredential CreateUserCredential(string appId, TokenRequestConte { var authRecordPath = Path.Combine(AUTH_CACHE, $"{AUTH_RECORD_PREFIX}-{appId}"); var interactiveCredential = GetInteractiveCredential(appId, authRecordPath); + // Interactive credential is primary; AzureCliCredential is a last-resort fallback for + // environments where interactive auth is completely unavailable (e.g. WSL without keyring + // AND no browser). The interactive credential uses cached auth records for silent token + // renewal, so it won't re-prompt when a valid cache exists. var credential = new ChainedTokenCredential(interactiveCredential, new AzureCliCredential()); return new AppCredential(credential, requestContext); diff --git a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs index 51edf767d6..2523f6d003 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs @@ -64,9 +64,12 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell return GetTokenCore(requestContext, cancellationToken); } catch (AuthenticationFailedException retryEx) + when (!cancellationToken.IsCancellationRequested && !ContainsCancellationException(retryEx)) { - // After persistence fallback, if interactive auth still fails, signal credential unavailability - // so ChainedTokenCredential can try the next credential (e.g. AzureCliCredential) + // After persistence fallback, if interactive auth still fails due to environment issues + // (e.g. no browser), signal credential unavailability so ChainedTokenCredential can + // try the next credential (e.g. AzureCliCredential). User-initiated cancellations + // propagate directly so the caller sees the real failure. throw new CredentialUnavailableException( "Interactive authentication failed after token cache persistence fallback. " + "Ensure a browser or device code flow is available, or use 'az login' as a fallback.", retryEx); @@ -90,6 +93,7 @@ public override async ValueTask GetTokenAsync(TokenRequestContext r return await GetTokenCoreAsync(requestContext, cancellationToken); } catch (AuthenticationFailedException retryEx) + when (!cancellationToken.IsCancellationRequested && !ContainsCancellationException(retryEx)) { throw new CredentialUnavailableException( "Interactive authentication failed after token cache persistence fallback. " @@ -194,6 +198,7 @@ private void RecreateCredentialsWithoutPersistence() { TenantId = _options.TenantId, ClientId = _options.ClientId, + AuthenticationRecord = _options.AuthenticationRecord, }); _deviceCodeCredential = new DeviceCodeCredential(new() { @@ -204,4 +209,7 @@ private void RecreateCredentialsWithoutPersistence() private static bool IsMsalCachePersistenceException(Exception e) => e is MsalCachePersistenceException || (e.InnerException is not null && IsMsalCachePersistenceException(e.InnerException)); + + private static bool ContainsCancellationException(Exception e) => + e is OperationCanceledException || (e.InnerException is not null && ContainsCancellationException(e.InnerException)); } From 02ddec4f51794507a075d1e7a803304ef4571e7e Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 21 May 2026 17:43:38 -0500 Subject: [PATCH 3/5] Surface device code via logger so CLI/MCP hosts can see it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../CachedInteractiveBrowserCredential.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs index 64e4f068fe..cc4373d0ab 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs @@ -50,6 +50,13 @@ public CachedInteractiveBrowserCredential( TenantId = _options.TenantId, ClientId = _options.ClientId, TokenCachePersistenceOptions = _options.TokenCachePersistenceOptions, + DeviceCodeCallback = (info, _) => + { + // Surface the device code through the same logger the rest of the auth flow uses; + // the default callback writes to AzureEventSource which is not visible in CLI/MCP hosts. + _logger.LogInformation("{Message}", info.Message); + return Task.CompletedTask; + }, }); } @@ -211,6 +218,13 @@ private void RecreateCredentialsWithoutPersistence() { TenantId = _options.TenantId, ClientId = _options.ClientId, + DeviceCodeCallback = (info, _) => + { + // Surface the device code through the same logger the rest of the auth flow uses; + // the default callback writes to AzureEventSource which is not visible in CLI/MCP hosts. + _logger.LogInformation("{Message}", info.Message); + return Task.CompletedTask; + }, }); } From bb914a327b8cd14e6d6f49353e220b7eb9ba1df6 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 21 May 2026 17:47:54 -0500 Subject: [PATCH 4/5] Skip browser retry after persistence-fallback recreation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../CachedInteractiveBrowserCredential.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs index cc4373d0ab..3c38fa87d2 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs @@ -192,6 +192,16 @@ private void CacheAuthenticationRecord(TokenRequestContext requestContext, Cance private AuthenticationRecord Authenticate(TokenRequestContext requestContext, CancellationToken cancellationToken) { + // If a previous attempt already proved the browser flow is unavailable in this + // environment (e.g. headless WSL / no GUI), skip straight to device code. Without + // this, the retry after RecreateCredentialsWithoutPersistence() would re-enter the + // browser path and hang waiting for an OAuth redirect that will never arrive. + if (Volatile.Read(ref _isDeviceCodeFallback) == 1) + { + _logger.LogInformation("Using device code authentication (browser flow previously unavailable)..."); + return _deviceCodeCredential.Authenticate(requestContext, cancellationToken); + } + try { _logger.LogInformation("Waiting for authentication in the browser..."); From 9088d98bbc4bea06cbe511ae682a706164d302f7 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 21 May 2026 18:31:12 -0500 Subject: [PATCH 5/5] Skip browser flow only when WSL has no Windows-browser launcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../CachedInteractiveBrowserCredential.cs | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs index 3c38fa87d2..0bb031a6f4 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CachedInteractiveBrowserCredential.cs @@ -58,6 +58,77 @@ public CachedInteractiveBrowserCredential( return Task.CompletedTask; }, }); + + // On WSL the interactive browser flow only succeeds when a Windows-side browser + // launcher (wslu's `wslview`) is installed AND WSL2 localhost forwarding can route + // the OAuth redirect back into the WSL network namespace. With wslu present that + // path works on default NAT-mode WSL2. Without wslu, xdg-open silently does nothing + // and `_browserCredential.Authenticate()` blocks forever waiting for a redirect + // that will never arrive. In that specific case, skip straight to device code so + // the user at least sees a code rather than an indefinite hang. + if (IsWslWithoutBrowserLauncher()) + { + Interlocked.Exchange(ref _isDeviceCodeFallback, 1); + } + } + + private static bool IsWslWithoutBrowserLauncher() + { + // Opt-out: user knows their setup supports browser auth even if we don't detect it. + if (string.Equals(Environment.GetEnvironmentVariable("DARC_FORCE_BROWSER_AUTH"), "1", StringComparison.Ordinal)) + { + return false; + } + + // Opt-in: user explicitly wants device code regardless of environment. + if (string.Equals(Environment.GetEnvironmentVariable("DARC_USE_DEVICE_CODE"), "1", StringComparison.Ordinal)) + { + return true; + } + + bool isWsl = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("WSL_DISTRO_NAME")) + || !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("WSL_INTEROP")); + if (!isWsl) + { + return false; + } + + // On WSL: only skip browser flow if no usable launcher is on PATH. + // wslview (from the wslu package) is the canonical Windows-browser bridge. + // BROWSER env var override is also respected by Azure.Identity's launcher. + if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("BROWSER"))) + { + return false; + } + return !ExistsOnPath("wslview"); + } + + private static bool ExistsOnPath(string executable) + { + var path = Environment.GetEnvironmentVariable("PATH"); + if (string.IsNullOrEmpty(path)) + { + return false; + } + foreach (var dir in path.Split(Path.PathSeparator)) + { + if (string.IsNullOrEmpty(dir)) + { + continue; + } + try + { + if (File.Exists(Path.Combine(dir, executable))) + { + return true; + } + } + catch + { + // Ignore inaccessible PATH entries. + } + } + return false; } public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken)