Commit 6b92e6a
fix: device code fallback for macOS browser auth and cross-platform URL opening (#309)
* fix: device code fallback for macOS browser auth and cross-platform URL 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
* fix: address PR review comments and handle Linux xdg_open_failed error
- 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>
* fix: auto-fix public client flows, add requirements check to blueprint, 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>
* fix: eliminate double auth on Linux, improve PS module error handling
- 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>
* fix: add system requirement checks to all setup commands that use Graph 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.
* Skip requirements on dry run; clarify browser fallback
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.
* fix: skip requirements on dry run, self-correct missing PS modules in 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>
* fix: extract real JWT from Graph request headers; unify PS auth scopes 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>
* fix: extract last stdout line as JWT token; add clean error for missing 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>
* fix: unify PS token cache key in RemoveStale and DeployCommand; update 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>
* fix: improve auth prompt UX and clarify custom permissions message
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>
* fix: retry blueprint SP lookup on Azure AD propagation delay
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>
* fix: fix admin consent polling via MSAL token with Application.Read.All
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>
* fix: fall back to MSAL when PS Connect-MgGraph fails on any platform
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>
* fix: address PR review comments - disposal, logging, exit handling, doc 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>
* fix: address second round of PR review comments
- 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>
* chore: add CHANGELOG, NuGet release notes, and review process updates
- 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>
* fix: address PR review comments - StringComparison, PSGallery, encoding, 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>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>1 parent def9e6e commit 6b92e6a
35 files changed
Lines changed: 1270 additions & 247 deletions
File tree
- .claude/agents
- .github
- src
- Microsoft.Agents.A365.DevTools.Cli
- Commands
- SetupSubcommands
- Constants
- Exceptions
- Helpers
- Services
- Helpers
- Internal
- Requirements/RequirementChecks
- Tests/Microsoft.Agents.A365.DevTools.Cli.Tests
- Commands
- Services
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
249 | 249 | | |
250 | 250 | | |
251 | 251 | | |
| 252 | + | |
252 | 253 | | |
253 | 254 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
147 | | - | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
148 | 153 | | |
149 | 154 | | |
150 | 155 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
| 103 | + | |
103 | 104 | | |
104 | 105 | | |
105 | 106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
833 | 833 | | |
834 | 834 | | |
835 | 835 | | |
836 | | - | |
837 | | - | |
838 | | - | |
839 | | - | |
| 836 | + | |
840 | 837 | | |
841 | | - | |
842 | | - | |
843 | | - | |
844 | | - | |
845 | | - | |
846 | | - | |
| 838 | + | |
847 | 839 | | |
848 | | - | |
849 | | - | |
850 | | - | |
851 | | - | |
852 | | - | |
853 | | - | |
854 | | - | |
| 840 | + | |
855 | 841 | | |
856 | | - | |
857 | | - | |
858 | | - | |
859 | | - | |
860 | | - | |
861 | | - | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
862 | 860 | | |
863 | 861 | | |
864 | 862 | | |
| |||
951 | 949 | | |
952 | 950 | | |
953 | 951 | | |
| 952 | + | |
954 | 953 | | |
955 | 954 | | |
956 | 955 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
414 | 414 | | |
415 | 415 | | |
416 | 416 | | |
417 | | - | |
418 | | - | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
419 | 420 | | |
420 | 421 | | |
421 | 422 | | |
| |||
Lines changed: 75 additions & 34 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
154 | 154 | | |
155 | 155 | | |
156 | 156 | | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
157 | 162 | | |
158 | 163 | | |
159 | 164 | | |
160 | 165 | | |
161 | 166 | | |
162 | 167 | | |
| 168 | + | |
163 | 169 | | |
164 | | - | |
| 170 | + | |
165 | 171 | | |
166 | 172 | | |
167 | 173 | | |
| |||
215 | 221 | | |
216 | 222 | | |
217 | 223 | | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
218 | 255 | | |
219 | 256 | | |
220 | 257 | | |
| |||
281 | 318 | | |
282 | 319 | | |
283 | 320 | | |
284 | | - | |
| 321 | + | |
285 | 322 | | |
286 | 323 | | |
287 | 324 | | |
| |||
773 | 810 | | |
774 | 811 | | |
775 | 812 | | |
776 | | - | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
777 | 816 | | |
778 | 817 | | |
779 | 818 | | |
| |||
1339 | 1378 | | |
1340 | 1379 | | |
1341 | 1380 | | |
| 1381 | + | |
| 1382 | + | |
| 1383 | + | |
| 1384 | + | |
| 1385 | + | |
| 1386 | + | |
| 1387 | + | |
| 1388 | + | |
| 1389 | + | |
| 1390 | + | |
| 1391 | + | |
| 1392 | + | |
| 1393 | + | |
| 1394 | + | |
1342 | 1395 | | |
1343 | 1396 | | |
1344 | 1397 | | |
1345 | 1398 | | |
1346 | 1399 | | |
1347 | 1400 | | |
1348 | 1401 | | |
1349 | | - | |
1350 | | - | |
1351 | | - | |
1352 | | - | |
1353 | | - | |
1354 | | - | |
1355 | | - | |
1356 | | - | |
1357 | | - | |
1358 | 1402 | | |
1359 | 1403 | | |
1360 | | - | |
| 1404 | + | |
1361 | 1405 | | |
1362 | 1406 | | |
1363 | 1407 | | |
1364 | | - | |
| 1408 | + | |
| 1409 | + | |
1365 | 1410 | | |
1366 | 1411 | | |
1367 | 1412 | | |
| |||
1373 | 1418 | | |
1374 | 1419 | | |
1375 | 1420 | | |
1376 | | - | |
| 1421 | + | |
| 1422 | + | |
1377 | 1423 | | |
1378 | 1424 | | |
1379 | 1425 | | |
| |||
1428 | 1474 | | |
1429 | 1475 | | |
1430 | 1476 | | |
1431 | | - | |
| 1477 | + | |
| 1478 | + | |
1432 | 1479 | | |
1433 | | - | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
| 1487 | + | |
| 1488 | + | |
| 1489 | + | |
| 1490 | + | |
| 1491 | + | |
1434 | 1492 | | |
1435 | 1493 | | |
1436 | 1494 | | |
| |||
1541 | 1599 | | |
1542 | 1600 | | |
1543 | 1601 | | |
1544 | | - | |
1545 | | - | |
1546 | | - | |
1547 | | - | |
1548 | | - | |
1549 | | - | |
1550 | | - | |
1551 | | - | |
1552 | | - | |
1553 | | - | |
1554 | | - | |
1555 | | - | |
1556 | | - | |
1557 | | - | |
1558 | | - | |
1559 | | - | |
1560 | | - | |
1561 | 1602 | | |
1562 | 1603 | | |
1563 | 1604 | | |
| |||
Lines changed: 16 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
63 | 64 | | |
64 | 65 | | |
65 | 66 | | |
66 | | - | |
| 67 | + | |
67 | 68 | | |
68 | 69 | | |
69 | 70 | | |
| |||
72 | 73 | | |
73 | 74 | | |
74 | 75 | | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
75 | 90 | | |
76 | 91 | | |
77 | 92 | | |
| |||
0 commit comments