fix: automatic device code fallback when Conditional Access Policy blocks browser/WAM auth (#294)

.claude/agents/pr-code-reviewer.md

@@ -592,6 +592,68 @@ A method with return type `bool?` (where `null` signals "fall back to az CLI") r
   return null; // 401/403/5xx — caller falls back to az CLI
   ```
 
+### 19. Unreachable Catch Clause — Inner Method Already Handles the Exception Internally
+
+A catch block in the caller catches exception type `E`, but the method being called already handles that same `E` internally before propagating. The outer catch becomes dead code. A related risk: if the inner method attempts a fallback (e.g., device code) and that fallback also throws the same exception shape, the outer catch re-triggers the same fallback — a "double attempt" that can produce confusing error messages or partial state.
+
+- **Pattern to catch**:
+  1. Outer method `A` has `catch (ExcType ex) when (inner condition)` wrapping a call to inner method `B`
+  2. Inner method `B` is in the same diff or codebase and already contains `catch (ExcType ex) when (same condition)` — meaning `B` never actually throws it
+  3. The outer catch attempts the same fallback logic `B` already tried
+- **Severity**: `high` — dead code that creates a double-attempt risk; misleads reviewers into thinking coverage exists
+- **Check**: For every new `catch` block in the diff, read the source of the called method (use `Read` tool) and verify whether the exception type/condition can actually propagate out. If the inner method swallows it, the outer catch is dead code.
+- **Fix**: Remove the outer catch block; rely on the inner implementation's guarantee.
+
+**Example (from PR #323 — C1/C2):**
+```csharp
+// AuthenticationService.cs — DEAD CODE:
+// MsalBrowserCredential.GetTokenAsync already catches MsalServiceException(AADSTS53003)
+// internally and falls back to device code before propagating. This catch never fires
+// in production and risks a double device code attempt if that internal fallback also fails.
+catch (MsalAuthenticationFailedException ex) when (
+    useInteractiveBrowser &&
+    ex.InnerException is MsalServiceException svcEx &&
+    svcEx.Message.Contains("AADSTS53003", StringComparison.Ordinal))
+{
+    // Unreachable — remove this catch block
+}
+```
+
+### 20. `MsalServiceException.ErrorCode` Used When AADSTS Code Is Expected
+
+`MsalServiceException.ErrorCode` is the OAuth 2.0 error code (e.g., `"access_denied"`, `"invalid_request"`) — it is NOT the AADSTS error code (e.g., `"AADSTS53003"`, `"AADSTS70011"`). When the intent is to log or display the specific policy-level error for diagnostics, using `ex.ErrorCode` in the message placeholder is misleading — the AADSTS code appears only in `ex.Message`.
+
+- **Pattern to catch**: Inside `catch (MsalServiceException ex) when (ex.Message.Contains("AADSTS..."))`, a log call that uses `ex.ErrorCode` as the structured log parameter when the intent is to surface the AADSTS code
+- **Severity**: `medium` — log entries record `"access_denied"` instead of `"AADSTS53003"`, making incident investigation harder
+- **Check**: For every `catch (MsalServiceException ex)` in the diff, look for `ex.ErrorCode` in log calls. If the when-clause matched on an AADSTS code in `ex.Message`, the log should use the AADSTS code, not `ex.ErrorCode`.
+- **Fix**: Extract the AADSTS code from the message or use the constants matched in the when-clause:
+  ```csharp
+  var aadErrorCode = ex.Message.Contains(AuthenticationConstants.ConditionalAccessPolicyBlockedError, StringComparison.Ordinal)
+      ? AuthenticationConstants.ConditionalAccessPolicyBlockedError
+      : AuthenticationConstants.DeviceCompliancePolicyBlockedError;
+  _logger?.LogWarning("Blocked by Conditional Access Policy ({ErrorCode}).", aadErrorCode);
+  ```
+
+### 21. Log Message / Comment Covers Fewer Cases Than the Code Handles
+
+When a catch block, comment, or doc string covers multiple distinct error conditions (e.g., both AADSTS53003 and AADSTS53000), but the associated log messages and comments only mention one of them — the omitted condition goes undocumented, misleading operators during incident triage.
+
+- **Pattern to catch**:
+  - A `when`-clause that checks for two or more constants/codes (e.g., `Contains("AADSTS53003") || Contains("AADSTS53000")`) but the log message only names one (`"Conditional Access Policy"` without mentioning device compliance)
+  - XML doc comments on constants that claim "Device code flow bypasses this policy" or "not subject to these policies" when that statement is configuration-dependent and not universally true
+- **Severity**: `medium` — operators see `"Conditional Access Policy"` in logs when the real trigger was AADSTS53000 (device compliance); inaccurate docs create false confidence in fallback guarantees
+- **Fix 1** — Log message: include all covered codes or use a broader description:
+  ```csharp
+  _logger.LogWarning(
+      "Authentication blocked by a Conditional Access or device compliance policy ({ErrorCode}). " +
+      "Retrying with device code authentication...", aadErrorCode);
+  ```
+- **Fix 2** — Doc comment: replace absolute "bypasses" claims with conditional language:
+  ```csharp
+  // Before: "Device code flow bypasses CAP — the CLI falls back automatically."
+  // After:  "Device code flow may succeed depending on your tenant's CAP configuration."
+  ```
+
 **MANDATORY REPORTING RULE**: Whenever the diff contains any test file (`.Tests.cs`), you MUST emit a named finding for this check — even if no violation is found. The finding must appear in the review output with one of three statuses:
   - **`high` severity** if a violation is found (missing warmup, dead executor mock, etc.)
   - **`info` — FIXED** if the PR is fixing a prior violation (warmup added to previously-cold classes) — list each class fixed and its measured or estimated speedup

CHANGELOG.md

@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - `a365 cleanup` blueprint deletion now succeeds for Global Administrators even when the blueprint was created by a different user
 - `a365 setup all` no longer times out for non-admin users — the CLI immediately surfaces a consent URL to share with an administrator instead of waiting for a browser prompt
 - `a365 setup all` requests admin consent once for all resources instead of prompting once per resource
+- Browser and WAM authentication blocked by Conditional Access Policy (AADSTS53003, AADSTS53000) now automatically falls back to device code flow (#294)
 - macOS/Linux: device code fallback when browser authentication is unavailable (#309)
 - Linux: MSAL fallback when PowerShell `Connect-MgGraph` fails in non-TTY environments (#309)
 - Admin consent polling no longer times out after 180s — blueprint service principal now resolved with correct MSAL token (#309)

src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs

@@ -201,4 +201,19 @@ public static string[] GetRequiredRedirectUris(string clientAppId)
     /// for testing purposes only. It should NOT be deployed to production Azure environments.
     /// </summary>
     public const string BearerTokenEnvironmentVariable = "BEARER_TOKEN";
+
+    /// <summary>
+    /// AADSTS53003: Access blocked by Conditional Access Policy.
+    /// MSAL throws MsalServiceException with ErrorCode "access_denied" and this code in the Message.
+    /// Device code flow may succeed depending on your tenant's Conditional Access Policy configuration.
+    /// Reference: https://learn.microsoft.com/en-us/entra/identity-platform/reference-error-codes
+    /// </summary>
+    public const string ConditionalAccessPolicyBlockedError = "AADSTS53003";
+
+    /// <summary>
+    /// AADSTS53000: Device does not comply with device compliance policy (a subset of CAP).
+    /// Treated identically to AADSTS53003 for fallback purposes.
+    /// Device code flow may succeed depending on your tenant's Conditional Access Policy configuration.
+    /// </summary>
+    public const string DeviceCompliancePolicyBlockedError = "AADSTS53000";
 }

src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs

@@ -252,7 +252,6 @@ private async Task<TokenInfo> AuthenticateInteractivelyAsync(
                 var deviceCodeCredential = CreateDeviceCodeCredential(effectiveClientId, effectiveTenantId);
                 tokenResult = await deviceCodeCredential.GetTokenAsync(tokenRequestContext, default);
             }
-
             _logger.LogInformation("Authentication successful!");
 
             return new TokenInfo

src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs

@@ -153,6 +153,19 @@ public MicrosoftGraphTokenProvider(
                 var script = BuildPowerShellScript(tenantId, validatedScopes, useDeviceCode, clientAppId);
                 var result = await ExecuteWithFallbackAsync(script, ct);
                 token = ProcessResult(result);
+
+                // If PowerShell browser auth was blocked by Conditional Access Policy, retry with
+                // device code. This covers the case where clientAppId is null (MSAL skipped) and the
+                // user is on a CAP-enforced tenant where browser auth is blocked.
+                if (string.IsNullOrWhiteSpace(token) && !useDeviceCode && IsConditionalAccessError(result))
+                {
+                    _logger.LogWarning(
+                        "PowerShell browser authentication blocked by a Conditional Access or device compliance policy (AADSTS53003/AADSTS53000). " +
+                        "Retrying with device code authentication...");
+                    var deviceCodeScript = BuildPowerShellScript(tenantId, validatedScopes, useDeviceCode: true, clientAppId);
+                    var deviceCodeResult = await ExecuteWithFallbackAsync(deviceCodeScript, ct);
+                    token = ProcessResult(deviceCodeResult);
+                }
             }
 
             if (string.IsNullOrWhiteSpace(token))
@@ -476,6 +489,13 @@ private static string BuildPowerShellArguments(string shell, string script)
         return token;
     }
 
+    private static bool IsConditionalAccessError(CommandResult result)
+    {
+        return !string.IsNullOrWhiteSpace(result.StandardError) &&
+               (result.StandardError.Contains(AuthenticationConstants.ConditionalAccessPolicyBlockedError, StringComparison.Ordinal) ||
+                result.StandardError.Contains(AuthenticationConstants.DeviceCompliancePolicyBlockedError, StringComparison.Ordinal));
+    }
+
     private static bool IsPowerShellNotFoundError(CommandResult result)
     {
         if (string.IsNullOrWhiteSpace(result.StandardError))

src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs

@@ -420,6 +420,22 @@ public override async ValueTask<AccessToken> GetTokenAsync(
             _logger?.LogWarning("Browser cannot be opened on this platform: {Message}", ex.Message);
             return await AcquireTokenWithDeviceCodeFallbackAsync(scopes, cancellationToken);
         }
+        catch (MsalServiceException ex) when (
+            ex.Message.Contains(AuthenticationConstants.ConditionalAccessPolicyBlockedError, StringComparison.Ordinal) ||
+            ex.Message.Contains(AuthenticationConstants.DeviceCompliancePolicyBlockedError, StringComparison.Ordinal))
+        {
+            // Conditional Access Policy (AADSTS53003) or device compliance policy (AADSTS53000)
+            // blocks interactive browser/WAM authentication. Device code flow may still be affected
+            // by these policies depending on your tenant configuration — attempting fallback.
+            var aadErrorCode = ex.Message.Contains(AuthenticationConstants.ConditionalAccessPolicyBlockedError, StringComparison.Ordinal)
+                ? AuthenticationConstants.ConditionalAccessPolicyBlockedError
+                : AuthenticationConstants.DeviceCompliancePolicyBlockedError;
+            _logger?.LogWarning(
+                "Interactive authentication blocked by Conditional Access Policy ({ErrorCode}). " +
+                "Falling back to device code authentication.",
+                aadErrorCode);
+            return await AcquireTokenWithDeviceCodeFallbackAsync(scopes, cancellationToken);
+        }
         catch (MsalException ex)
         {
             _logger?.LogDebug(ex, "MSAL authentication failed");

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AuthenticationServiceTests.cs

@@ -915,8 +915,67 @@ public async Task GetAccessTokenAsync_WhenBrowserThrowsNonPlatformException_Does
             forceRefresh: true,
             useInteractiveBrowser: true);
 
-        // Assert — no fallback; error propagates as AzureAuthenticationException
-        await act.Should().ThrowAsync<AzureAuthenticationException>();
+        // Assert — non-CAP errors must NOT trigger device code fallback; only AADSTS53003/53000 should
+        await act.Should().ThrowAsync<AzureAuthenticationException>(
+            because: "unrecognized auth errors (e.g. user cancelled) must not silently fall back " +
+                     "to device code — only Conditional Access Policy errors qualify for automatic fallback");
+    }
+
+    [Fact]
+    public async Task GetAccessTokenAsync_WhenBrowserThrowsMsalServiceExceptionWithUnrelatedCode_DoesNotFallBack()
+    {
+        // Arrange — browser fails with a real MsalServiceException but for an unrelated reason
+        // (e.g. AADSTS70011 "invalid scope") — must NOT fall back to device code
+        var msalServiceEx = new Microsoft.Identity.Client.MsalServiceException(
+            "invalid_request",
+            "AADSTS70011: The provided request must include a 'scope' input parameter.");
+        var browserCredential = new ThrowingTokenCredential(
+            new MsalAuthenticationFailedException("Invalid scope request.", msalServiceEx));
+
+        var deviceCodeCredential = new StubTokenCredential("should-not-be-used", DateTimeOffset.UtcNow.AddHours(1));
+
+        var logger = Substitute.For<ILogger<AuthenticationService>>();
+        var sut = new TestableAuthenticationService(logger, browserCredential, deviceCodeCredential);
+
+        // Act
+        Func<Task> act = async () => await sut.GetAccessTokenAsync(
+            "https://agent365.svc.cloud.microsoft",
+            forceRefresh: true,
+            useInteractiveBrowser: true);
+
+        // Assert
+        await act.Should().ThrowAsync<AzureAuthenticationException>(
+            because: "only AADSTS53003 and AADSTS53000 qualify for automatic device code fallback; " +
+                     "other MsalServiceException codes must surface as errors so the user can diagnose them");
+    }
+
+    [Fact]
+    public async Task GetAccessTokenAsync_WhenDeviceCodeFailsWithCapError_ThrowsAzureAuthenticationException()
+    {
+        // Arrange — useInteractiveBrowser: false means device code is the first (and only) path.
+        // If device code itself fails, there is no further fallback — the error must surface.
+        var msalServiceEx = new Microsoft.Identity.Client.MsalServiceException(
+            "access_denied",
+            "AADSTS53003: Access has been blocked by Conditional Access policies.");
+        var browserCredential = new StubTokenCredential("unused", DateTimeOffset.UtcNow.AddHours(1));
+        var deviceCodeCredential = new ThrowingTokenCredential(
+            new MsalAuthenticationFailedException(
+                "Device code authentication failed.",
+                msalServiceEx));
+
+        var logger = Substitute.For<ILogger<AuthenticationService>>();
+        var sut = new TestableAuthenticationService(logger, browserCredential, deviceCodeCredential);
+
+        // Act
+        Func<Task> act = async () => await sut.GetAccessTokenAsync(
+            "https://agent365.svc.cloud.microsoft",
+            forceRefresh: true,
+            useInteractiveBrowser: false);
+
+        // Assert
+        await act.Should().ThrowAsync<AzureAuthenticationException>(
+            because: "when device code itself fails there is no further fallback — " +
+                     "the error must surface to the user rather than looping");
     }
 
     #endregion