Skip to content

fix: device code fallback for macOS browser auth and cross-platform URL opening#309

Merged
sellakumaran merged 19 commits intomainfrom
fix/macos-browser-auth-msal-credential-fallback
Mar 6, 2026
Merged

fix: device code fallback for macOS browser auth and cross-platform URL opening#309
sellakumaran merged 19 commits intomainfrom
fix/macos-browser-auth-msal-credential-fallback

Conversation

@sellakumaran
Copy link
Contributor

@sellakumaran sellakumaran commented Mar 4, 2026 •
edited
Loading

Fixes a365 setup blueprint failures on macOS and Linux where interactive browser authentication was unavailable.

Changes

MsalBrowserCredential

  • Catch PlatformNotSupportedException and fall back to device code flow so macOS/Linux headless environments always get a working auth path
  • Attempt silent token acquisition before showing device code to reduce repeat prompts within the same CLI invocation
  • Filter cached MSAL accounts by tenant ID before silent auth to avoid wrong-identity issues with multiple cached accounts

MicrosoftGraphTokenProvider

  • Fall back to MSAL when PowerShell Connect-MgGraph fails (non-TTY on Linux, null ref in DeviceCodeCredential, etc.); token is cached in-process so subsequent calls within the same run don't re-prompt

InteractiveGraphAuthService

  • Eagerly acquire token before constructing GraphServiceClient to surface auth failures at construction time
  • Inject optional credentialFactory for testability

AdminConsentHelper / BlueprintSubcommand

  • Add MSAL-based polling overload; pass RequiredPermissionGrantScopes to SP lookups so the correct token is used — fixes blueprintSpId = null and unblocks consent detection (previously timed out at 180s)

BrowserHelper

  • Cross-platform TryOpenUrl (Windows/macOS/Linux); replaces duplicated TryOpenBrowser methods in BlueprintSubcommand and A365CreateInstanceRunner

Tests

  • 5 unit tests covering eager-auth gap and credential caching behavior

PR review feedback addressed — disposal, logging, exit handling, and minor correctness fixes across several files.

Closes #308

@sellakumaran
fix: device code fallback for macOS browser auth and cross-platform U…
cbdfdba 
…RL opening

- MsalBrowserCredential: catch PlatformNotSupportedException and fall back to
  device code flow instead of re-throwing, so macOS/Linux headless environments
  always get a working auth path
- InteractiveGraphAuthService: eagerly acquire token before constructing
  GraphServiceClient to surface auth failures at construction time rather than
  silently returning a broken client (Gap 2 from PR #290)
- InteractiveGraphAuthService: inject optional credentialFactory for testability
- AuthenticationService: replace Console.WriteLine with structured logger calls
  in device code callback
- Add BrowserHelper with cross-platform TryOpenUrl (Windows/macOS/Linux)
- Remove duplicated TryOpenBrowser methods from BlueprintSubcommand and
  A365CreateInstanceRunner; both now call BrowserHelper.TryOpenUrl directly
- Add 5 unit tests covering the eager-auth gap and credential caching behavior
@sellakumaran sellakumaran requested a review from a team as a code owner March 4, 2026 19:38
Copilot AI review requested due to automatic review settings March 4, 2026 19:38
@sellakumaran sellakumaran requested a review from a team as a code owner March 4, 2026 19:38
@github-actions
Copy link

github-actions bot commented Mar 4, 2026 •
edited
Loading

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves interactive authentication reliability across macOS/Linux (including headless environments) by ensuring there is always a working fallback path, and by centralizing cross-platform URL opening logic used during admin consent flows.

Changes:

  • Add eager token acquisition in InteractiveGraphAuthService (plus injectable credential factory) to surface auth failures immediately and improve testability.
  • Implement device-code fallback when browser-based MSAL auth is unsupported, and convert device-code output to structured logging.
  • Introduce BrowserHelper.TryOpenUrl and replace duplicated “open browser” implementations with the shared helper.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/InteractiveGraphAuthServiceTests.cs Adds unit tests for eager-auth failure surfacing and tenant client caching behavior.
src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs Falls back to MSAL device code flow when browser auth is not supported.
src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs Eagerly acquires a token before returning a GraphServiceClient; adds optional credentialFactory.
src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs Replaces Console.WriteLine device-code messaging with structured logger calls.
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs Uses shared BrowserHelper.TryOpenUrl instead of local browser-opening code.
src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs New cross-platform URL opening helper (Windows/macOS/Linux).
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs Switches admin consent browser opening to BrowserHelper.TryOpenUrl.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 12:00
@sellakumaran @claude
fix: address PR review comments and handle Linux xdg_open_failed error
6c8fe1d 
- MsalBrowserCredential: also catch MsalClientException(linux_xdg_open_failed)
  for device code fallback on Linux/WSL (confirmed via WSL Ubuntu 24.04 repro);
  extract shared AcquireTokenWithDeviceCodeFallbackAsync to avoid duplication
- BrowserHelper: fall back to Console.Error when logger is null so the manual
  URL is never silently lost regardless of caller
- BlueprintSubcommand: log consent URL before TryOpenUrl and pass logger so
  users on headless platforms always see the URL to complete admin consent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: auto-fix public client flows, add requirements check to blueprin…
4da9b55 
…t, clean AADSTS7000218 error

- ClientAppValidator: add EnsurePublicClientFlowsEnabledAsync (Step 7) that auto-detects and
  enables 'Allow public client flows' on the app registration via az rest GET/PATCH — required
  for MSAL device code fallback on macOS/Linux; non-fatal if PATCH fails
- BlueprintSubcommand: run config requirements checks (LocationCheck + ClientAppCheck) before
  blueprint logic, with --skip-requirements opt-out; matches AllSubcommand pattern
- MsalBrowserCredential: catch AADSTS7000218/invalid_client before general MsalException to
  emit clean actionable error message pointing to 'a365 setup requirements' instead of stack trace
- Tests: add EnsurePublicClientFlowsEnabledAsync tests (enabled/disabled/patch-fails) and
  CreateCommand_ShouldHaveSkipRequirementsOption test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 4, 2026 20:36
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 13:07
@sellakumaran @claude
fix: eliminate double auth on Linux, improve PS module error handling
3272376 
- MsalBrowserCredential: add shared in-memory MSAL token cache for Linux
  so all MsalBrowserCredential instances within one CLI invocation share
  the same token cache, eliminating repeated device code prompts during
  multi-step operations (e.g. blueprint creation + client secret creation)

- BrowserHelper: remove exception object from LogWarning so xdg-open
  failures no longer emit a full stack trace in the output

- MicrosoftGraphTokenProvider: detect missing PowerShell module error and
  log actionable guidance to run 'a365 setup requirements'

- SetupHelpers: replace misleading "please sign in when prompted" message
  with one that mentions missing PS module as a common cause

- PowerShellModulesRequirementCheck: auto-install missing modules via
  Install-Module -Scope CurrentUser -Force -AllowClobber when detected;
  returns Success if auto-install succeeds, Failure with manual steps if not

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran
fix: add system requirement checks to all setup commands that use Gra…
2eae60e 
…ph PS auth

setup blueprint was running only config checks (Location + ClientApp),
missing PowerShellModulesRequirementCheck entirely. setup permissions
mcp/bot/custom and setup permissions copilotstudio had no requirement
checks at all. All four now gate on GetSystemRequirementChecks() before
executing Graph operations, so missing Microsoft.Graph.Authentication
is caught and auto-installed upfront rather than failing mid-run.

Closes partial work toward #106.
Copilot AI review requested due to automatic review settings March 4, 2026 22:13
@sellakumaran sellakumaran mentioned this pull request Mar 4, 2026
Open
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 14:19
@sellakumaran
Skip requirements on dry run; clarify browser fallback
35bd6a0 
Requirements checks are now skipped during dry runs to prevent unintended mutations. Updated `TryOpenUrl` documentation to clarify fallback behavior and logging when browser launch fails.
@sellakumaran @claude
fix: skip requirements on dry run, self-correct missing PS modules in…
f10f636 
… Graph token acquisition

- Skip system requirement checks (including auto-install) when --dry-run is set in
  PermissionsSubcommand (mcp/bot/custom) and CopilotStudioSubcommand to preserve
  non-mutating dry-run semantics
- MicrosoftGraphTokenProvider now detects a missing/broken Microsoft.Graph module at
  runtime, auto-installs both required Graph modules, and retries token acquisition once
  before failing; error message clarifies auto-install was attempted if retry also fails
- PowerShellModulesRequirementCheck.ExecutePowerShellCommandAsync returns stderr instead
  of null on failure so auto-install debug logs are actionable

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 4, 2026 22:30
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 14:54
@sellakumaran @claude
fix: extract real JWT from Graph request headers; unify PS auth scope…
1550c0e 
…s to reduce login prompts

Microsoft.Graph.Authentication v2+ returns an opaque (non-JWT) token from
$ctx.AccessToken that is rejected when used as a Bearer token. Switch to
always extracting the token from the Authorization header of a live
Invoke-MgGraphRequest call, which always contains the real JWT.

Add AgentIdentityBlueprint.UpdateAuthProperties.All to RequiredPermissionGrantScopes
so all PowerShell-based Graph operations (OAuth2 grants, service principal
lookups, and inheritable permissions) use the same scope set. This ensures
Connect-MgGraph authenticates once and the access token is reused from the
in-process cache for all downstream calls, reducing device code prompts from
4 to 3 on first run (1 PS + 2 MSAL for different resources).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: extract last stdout line as JWT token; add clean error for missi…
bae2765 
…ng config

- MicrosoftGraphTokenProvider: extract last non-empty stdout line as token.
  Connect-MgGraph in headless Linux/WSL falls back to device code and writes
  the 'To sign in...' prompt to stdout, contaminating the full StandardOutput
  string. Trimming the whole output returns a non-JWT string; taking only the
  last line (where $token is always written) fixes the JWT validation warning
  and the resulting SETUP_VALIDATION_FAILED for inheritable permissions.

- ConfigFileNotFoundException: new Agent365Exception subclass for missing
  a365.config.json. ConfigService.LoadAsync now throws this instead of a raw
  FileNotFoundException, so the global handler displays a clean actionable
  error ('run a365 config init') instead of a full stack trace.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 4, 2026 23:03
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 15:20
@sellakumaran @claude
fix: unify PS token cache key in RemoveStale and DeployCommand; updat…
5b58ab7 
…e test

Both RemoveStaleCustomPermissionsAsync (PermissionsSubcommand) and DeployCommand
used a hardcoded 2-scope array missing DelegatedPermissionGrant.ReadWrite.All.
This produced a different sorted cache key than RequiredPermissionGrantScopes,
causing a second Connect-MgGraph device-code prompt during 'setup blueprint'
even though a token was already cached from the inheritable permissions step.

Switch both sites to AuthenticationConstants.RequiredPermissionGrantScopes so
all Graph token acquisitions share one cache entry per CLI invocation.

Update Agent365ConfigServiceTests to expect ConfigFileNotFoundException instead
of FileNotFoundException, matching the new exception type thrown by ConfigService.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: improve auth prompt UX and clarify custom permissions message
ed2ace6 
MicrosoftGraphTokenProvider: replace misleading "Device Code: False" log
with platform-specific guidance so users know what to expect:
- Windows: "A browser window will open for authentication..."
- Linux/macOS: "A device code prompt will appear below..."
This avoids confusion when Connect-MgGraph auto-switches to device code
in headless environments even though useDeviceCode=false was requested.

PermissionsSubcommand: change "No custom blueprint permissions configured"
to "No custom blueprint permissions specified in config. Skipping." to make
clear this is about config content, not a system state or error condition.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 5, 2026 00:20
Copilot AI reviewed Mar 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 4, 2026 16:30
@sellakumaran @claude
fix: retry blueprint SP lookup on Azure AD propagation delay
4fd11c7 
LookupServicePrincipalByAppIdAsync can return null for 10-30s after
blueprint creation due to Azure AD eventual consistency. Use the existing
RetryHelper (already used for inheritable permissions verification) to
retry up to 5 times with 5s base delay / exponential backoff before
throwing the 'service principal may not have propagated' error.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: fix admin consent polling via MSAL token with Application.Read.All
ba0e867 
Root cause: SP lookup and consent grant queries used az CLI token (scopes=null)
which lacks Application.Read.All and DelegatedPermissionGrant.ReadWrite.All.
Graph silently returned HTTP 200 with empty array, leaving blueprintSpId=null
and causing polling to time out after 180s even after consent was granted.

Changes:
- BlueprintSubcommand: pass RequiredPermissionGrantScopes to SP lookups and
  CheckConsentExistsAsync; resolve blueprintSpId once at start of consent flow;
  use new MSAL PollAdminConsentAsync when SP ID is available
- AdminConsentHelper: add MSAL PollAdminConsentAsync overload using GraphApiService;
  add scopes param to CheckConsentExistsAsync; add progress/timeout log messages
- BlueprintLookupService: add optional scopes param to GetServicePrincipalByAppIdAsync
- GraphApiService: add debug logging for failed GET; fix JsonDocument disposal
- CommandExecutor: add optional outputTransform to ExecuteWithStreamingAsync
- MicrosoftGraphTokenProvider: reformat PS device code output to MSAL box style
- AuthenticationService: reduce token cache log noise (LogInfo -> LogDebug)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 5, 2026 16:46
Copilot AI reviewed Mar 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

sellakumaran and others added 2 commits March 5, 2026 10:46
@sellakumaran @claude
fix: fall back to MSAL when PS Connect-MgGraph fails on any platform
c1e2486 
When PowerShell Connect-MgGraph fails (NullRef in DeviceCodeCredential
on Linux, no TTY, module issues, or any other reason), fall back to
MsalBrowserCredential to acquire the Graph token. On Windows this uses
WAM; on Linux/macOS it uses device code with silent-first logic.

The token is stored in MicrosoftGraphTokenProvider's in-process cache
so subsequent calls (inheritable permissions, custom permissions) within
the same CLI invocation reuse it without re-prompting.

Also adds silent token acquisition attempt before device code in
MsalBrowserCredential.AcquireTokenWithDeviceCodeFallbackAsync to
reduce unnecessary device code prompts when a cached token exists.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: address PR review comments - disposal, logging, exit handling, d…
ac390bf 
…oc fixes

- ConfigFileNotFoundException: derive from FileNotFoundException so existing
  catch sites (CleanupCommand, DeployCommand, etc.) continue to work
- InteractiveGraphAuthService: remove unused Azure.Identity using; move
  credential factory resolution inside try/catch for consistent error wrapping
- BrowserHelper: include exception object in LogWarning for structured logging
- PowerShellModulesRequirementCheck: fix GenerateInstallationInstructions
  to produce valid PS syntax for multi-module Install-Module calls
- PermissionsSubcommand / CopilotStudioSubcommand: replace Environment.Exit(1)
  with ExceptionHandler.ExitWithCleanup(1) to flush output before exit
- MicrosoftGraphTokenProvider: dispose HttpResponseMessage after token extraction
- MsalBrowserCredential: use platform-neutral path notation in cache doc comment

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 5, 2026 19:40
Copilot AI reviewed Mar 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 5 comments.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 5, 2026 11:50
@sellakumaran
Merge with main.
538f309
@sellakumaran @claude
fix: address second round of PR review comments
1867f3a 
- ConfigFileNotFoundException: add explicit using System.IO to avoid
  implicit global usings dependency
- SetupHelpers: remove unnecessary ! null-forgiving operator on awaited
  task (it only asserts the Task itself is non-null, not the result)
- BrowserHelper: use ArgumentList instead of Arguments for open/xdg-open
  to avoid argument parsing issues with special characters in URLs
- MsalBrowserCredential: filter cached MSAL account by tenant ID before
  silent auth to avoid authenticating as wrong identity when multiple
  accounts are cached; fall through to device code if no tenant match
- MicrosoftGraphTokenProvider: reuse IsPowerShellModuleMissingError in
  ProcessResult to eliminate duplicated string-matching logic

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 5, 2026 19:58
Copilot AI reviewed Mar 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 7 comments.

You can also share your feedback on Copilot code review. Take the survey.

sellakumaran and others added 2 commits March 5, 2026 12:17
@sellakumaran @claude
chore: add CHANGELOG, NuGet release notes, and review process updates
64b8c2d 
- CHANGELOG.md: Keep a Changelog format covering [Unreleased], 1.1.0, 1.0.0
- Directory.Build.props: PackageReleaseNotes points to CHANGELOG.md (fixes NuGet warning)
- src/DEVELOPER.md: update Release Process to describe workflow; add CHANGELOG to PR checklist
- .github/copilot-instructions.md: add CHANGELOG reminder to Code Review Mindset
- .claude/agents/pr-code-reviewer.md: add CHANGELOG check in Step 2
- .claude/agents/code-reviewer.md: add CHANGELOG check to Self-Verification
- .claude/agents/code-review-manager.md: add CHANGELOG to Project Context standards

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran @claude
fix: address PR review comments - StringComparison, PSGallery, encodi…
cccd810 
…ng, docs

- MsalBrowserCredential.cs: StringComparison.Ordinal on Contains calls; comment clarifying same-tenant account edge case
- InteractiveGraphAuthService.cs: StringComparison.Ordinal on all Contains calls in exception filters
- PowerShellModulesRequirementCheck.cs: pin Install-Module to -Repository PSGallery
- MicrosoftGraphTokenProvider.cs: pin both Install-Module calls to -Repository PSGallery
- GraphApiService.cs: fix U+FFFD encoding artifact in comment
- CommandExecutor.cs: document outputPrefix applies only to first line (outputTransform must not return multi-line)
- BlueprintLookupService.cs: comment that DisplayName is not populated in this lookup path

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sellakumaran sellakumaran enabled auto-merge (squash) March 5, 2026 21:26
gwharris7
gwharris7 approved these changes Mar 6, 2026
View reviewed changes
Copy link
Contributor

@gwharris7 gwharris7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was able to successfully run a365 setup blueprint from ubuntu

@sellakumaran sellakumaran merged commit 6b92e6a into main Mar 6, 2026
8 checks passed
@sellakumaran sellakumaran deleted the fix/macos-browser-auth-msal-credential-fallback branch March 6, 2026 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@gwharris7 gwharris7 gwharris7 approved these changes

@ajmfehr ajmfehr ajmfehr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

macOS Browser Auth Fallback Not Applied to Blueprint Authentication Paths

4 participants

@sellakumaran @gwharris7 @ajmfehr