diff --git a/.claude/agents/code-review-manager.md b/.claude/agents/code-review-manager.md index b07d26cc..12642b7a 100644 --- a/.claude/agents/code-review-manager.md +++ b/.claude/agents/code-review-manager.md @@ -15,6 +15,7 @@ You are a Senior Code Review Manager with 15+ years of experience leading engine - Warnings are treated as errors - IDisposable objects must be properly disposed - Cross-platform compatibility required (Windows, macOS, Linux) +- `CHANGELOG.md` must be updated in `[Unreleased]` for user-facing changes (features, bug fixes, behavioral changes) **Your Primary Responsibilities**: diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index d1f65017..797003d2 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -249,5 +249,6 @@ Before completing your review: 3. Are your suggestions backed by specific reasoning? 4. Have you balanced criticism with recognition of good practices? 5. Would following your suggestions result in production-ready code? +6. For user-facing changes (features, bug fixes, behavioral changes): has `CHANGELOG.md` been updated in the `[Unreleased]` section? Flag as `low` severity if not. If you need to see additional context (like related files, configuration, or tests), ask for it explicitly. Your goal is to ensure the code is secure, maintainable, performant, and correctly implements CLI patterns. diff --git a/.claude/agents/pr-code-reviewer.md b/.claude/agents/pr-code-reviewer.md index 3fa27645..6b77c6e0 100644 --- a/.claude/agents/pr-code-reviewer.md +++ b/.claude/agents/pr-code-reviewer.md @@ -144,7 +144,12 @@ For each changed file, analyze: - Path separators - OS-specific code -7. **Test Coverage Gaps** +7. **CHANGELOG.md Check** (for user-facing changes) + - If the PR adds features, fixes bugs, or changes observable behavior, verify `CHANGELOG.md` has an entry in the `[Unreleased]` section + - Internal refactors, test-only changes, and tooling/CI-only changes do not require a CHANGELOG entry + - Flag as `low` severity if missing from a user-facing PR + +8. **Test Coverage Gaps** - Based on the conditional logic, what specific test scenarios are needed? - Generate concrete test code examples diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 632b9050..dbcd9144 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -100,6 +100,7 @@ ### Code Review Mindset - Be cautious about deleting code; avoid `git restore` without review - Do not create unnecessary documentation files +- For user-facing changes (features, bug fixes, behavioral changes): verify `CHANGELOG.md` has an entry in the `[Unreleased]` section --- diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..e84f1fe2 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,53 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). + +## [Unreleased] + +### Fixed +- 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) +- `ConfigFileNotFoundException` now derives from `FileNotFoundException` so existing catch sites continue to work (#309) + +## [1.1.0] - 2026-02 + +### Added +- Custom blueprint permissions configuration and management — configure any resource's OAuth2 grants and inheritable permissions via `a365.config.json` (#298) +- `setup requirements` subcommand with per-category checks: PowerShell modules, location, client app configuration, Frontier Program enrollment (#293) +- `setup permissions copilotstudio` subcommand for Power Platform `CopilotStudio.Copilots.Invoke` permission (#298) +- Persistent MSAL token cache to reduce repeated WAM login prompts on Windows (#261) +- Auto-detect endpoint name from project settings; globally unique names to prevent accidental collisions (#289) +- `.NET` runtime roll-forward — CLI now works on .NET 9 and later without reinstalling (#276) +- Mock tooling server MCP protocol compliance for Python and Node.js agents (#263) + +### Fixed +- Prevent `InternalServerError` loop when `--update-endpoint` fails on create (#304) +- Correct endpoint name derivation for `needsDeployment=false` scenarios (#296) +- Browser auth falls back to device code on macOS when WAM/browser is unavailable (#290) +- `PublishCommand` now returns non-zero exit code on all error paths (#266) +- Azure CLI Graph token cached across publish command Graph API calls (#267) +- PowerShell 5.1 install compatibility and macOS auth testability improvements (#292) +- MOS token cache timezone comparison bug in `TryGetCachedToken` (#278) +- Location config validated before endpoint registration and deletion (#281) +- `CustomClientAppId` correctly set in `BlueprintSubcommand` to fix inheritable permissions (#272) +- Endpoint names trimmed of trailing hyphens to comply with Azure Bot Service naming rules (#257) +- Python projects without `pyproject.toml` handled in `a365 deploy` (#253) + +## [1.0.0] - 2025-12 + +### Added +- `a365 setup blueprint` — creates and configures an Agent Identity Blueprint in Azure AD +- `a365 setup permissions mcp` / `bot` — configures OAuth2 grants and inheritable permissions +- `a365 deploy` — multi-platform deployment (`.NET`, `Node.js`, `Python`) with auto-detection +- `a365 config init` — initialize project configuration +- `a365 cleanup` — remove Azure resources and blueprint configuration +- Interactive browser authentication via MSAL with WAM on Windows +- Microsoft Graph operations using PowerShell `Microsoft.Graph` module +- Admin consent polling with automatic detection + +[Unreleased]: https://github.com/microsoft/Agent365-devTools/compare/v1.1.0...HEAD +[1.1.0]: https://github.com/microsoft/Agent365-devTools/compare/v1.0.0...v1.1.0 +[1.0.0]: https://github.com/microsoft/Agent365-devTools/releases/tag/v1.0.0 diff --git a/src/DEVELOPER.md b/src/DEVELOPER.md index 3a6d10e0..cec5b1ad 100644 --- a/src/DEVELOPER.md +++ b/src/DEVELOPER.md @@ -833,32 +833,30 @@ Follow Semantic Versioning: `MAJOR.MINOR.PATCH[-PRERELEASE]` ### Create Release -1. Update version in `Microsoft.Agents.A365.DevTools.Cli.csproj`: - ```xml - 1.0.0-beta.2 - ``` +Version is managed automatically by [Nerdbank.GitVersioning](https://github.com/dotnet/Nerdbank.GitVersioning) via `src/version.json`. The NuGet publish process is fully automated through GitHub Actions. -2. Build and pack: - ```bash - dotnet clean - dotnet build -c Release - dotnet pack -c Release - ``` +**Steps to release:** -3. Test locally: - ```bash - dotnet tool uninstall -g Microsoft.Agents.A365.DevTools.Cli - dotnet tool install -g Microsoft.Agents.A365.DevTools.Cli \ - --add-source ./bin/Release \ - --prerelease - ``` +1. **Update CHANGELOG.md** — move items from `[Unreleased]` to a new version section (e.g., `[1.2.0] - YYYY-MM`). Update the comparison links at the bottom. -4. Publish to NuGet (when ready): - ```bash - dotnet nuget push ./bin/Release/Microsoft.Agents.A365.DevTools.Cli.1.0.0-beta.2.nupkg \ - --source https://api.nuget.org/v3/index.json \ - --api-key YOUR_API_KEY - ``` +2. **Merge to main** — CI runs automatically: builds, tests, and uploads the NuGet package as a build artifact. + +3. **Publish the GitHub release draft** — release-drafter auto-creates a draft release from merged PR titles and labels. Go to [GitHub Releases](https://github.com/microsoft/Agent365-devTools/releases), review the draft, set the correct version tag (e.g., `v1.2.0`), and click **Publish release**. + +4. **NuGet publish runs automatically** — the `release.yml` workflow triggers on `release: published` and pushes the package to NuGet.org using the `NUGET_API_KEY` repository secret. + +**Test locally before releasing:** +```bash +cd src +dotnet build dirs.proj -c Release +dotnet pack dirs.proj -c Release --output ../NuGetPackages + +dotnet tool uninstall -g Microsoft.Agents.A365.DevTools.Cli +dotnet tool install -g Microsoft.Agents.A365.DevTools.Cli \ + --add-source ../NuGetPackages \ + --prerelease +a365 --version +``` --- @@ -951,6 +949,7 @@ Then run: `source ~/.bashrc` (or `source ~/.zshrc`) - [ ] No breaking changes (or documented) - [ ] Error handling implemented - [ ] Logging added +- [ ] CHANGELOG.md updated in `[Unreleased]` (required for user-facing changes: features, bug fixes, behavioral changes) --- diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 501d89b8..ac5435ea 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -31,6 +31,7 @@ git MIT true + See https://github.com/microsoft/Agent365-devTools/blob/main/CHANGELOG.md for release notes. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs index 6c194667..62cf2a83 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs @@ -414,8 +414,9 @@ private static async Task EnsureMcpInheritablePermissionsAsync( var resourceAppId = ConfigConstants.GetAgent365ToolsResourceAppId(config.Environment); - // Use custom client app auth for inheritable permissions - Azure CLI doesn't support this operation - var requiredPermissions = new[] { "AgentIdentityBlueprint.UpdateAuthProperties.All", "Application.ReadWrite.All" }; + // Use custom client app auth for inheritable permissions - Azure CLI doesn't support this operation. + // Use RequiredPermissionGrantScopes so all callers share the same PS token cache key. + var requiredPermissions = AuthenticationConstants.RequiredPermissionGrantScopes; var (ok, alreadyExists, err) = await blueprintService.SetInheritablePermissionsAsync( config.TenantId, config.AgentBlueprintId, resourceAppId, scopes, requiredScopes: requiredPermissions, ct); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index a62f71a6..46044141 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -154,14 +154,20 @@ public static Command CreateCommand( "--update-endpoint", description: "Delete the existing messaging endpoint and register a new one with the specified URL"); + var skipRequirementsOption = new Option( + "--skip-requirements", + description: "Skip requirements validation check\n" + + "Use with caution: setup may fail if prerequisites are not met"); + command.AddOption(configOption); command.AddOption(verboseOption); command.AddOption(dryRunOption); command.AddOption(skipEndpointRegistrationOption); command.AddOption(endpointOnlyOption); command.AddOption(updateEndpointOption); + command.AddOption(skipRequirementsOption); - command.SetHandler(async (config, verbose, dryRun, skipEndpointRegistration, endpointOnly, updateEndpoint) => + command.SetHandler(async (config, verbose, dryRun, skipEndpointRegistration, endpointOnly, updateEndpoint, skipRequirements) => { // Generate correlation ID at workflow entry point var correlationId = HttpClientFactory.GenerateCorrelationId(); @@ -215,6 +221,37 @@ await UpdateEndpointAsync( return; } + // Run all requirements checks: system checks (PowerShell modules, Frontier Preview) + // and config checks (Location, ClientApp — includes isFallbackPublicClient auto-fix + // required for device code auth on macOS/Linux/WSL). + // Skip when dryRun is true: ClientAppRequirementCheck can mutate the app registration + // (e.g., set isFallbackPublicClient), which violates dry-run semantics. + if (!skipRequirements && !dryRun) + { + try + { + var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetRequirementChecks(clientAppValidator), + setupConfig, + logger, + category: null, + CancellationToken.None); + + if (!requirementsResult) + { + logger.LogError("Setup cannot proceed due to the failed requirement checks above. Please fix the issues above and then try again."); + logger.LogError("Use the resolution guidance provided for each failed check."); + ExceptionHandler.ExitWithCleanup(1); + } + } + catch (Exception reqEx) + { + logger.LogError(reqEx, "Requirements check failed with an unexpected error: {Message}", reqEx.Message); + logger.LogError("If you want to bypass requirement validation, rerun this command with the --skip-requirements flag."); + ExceptionHandler.ExitWithCleanup(1); + } + } + if (dryRun) { logger.LogInformation("DRY RUN: Create Agent Blueprint"); @@ -281,7 +318,7 @@ await CreateBlueprintImplementationAsync( correlationId: correlationId ); - }, configOption, verboseOption, dryRunOption, skipEndpointRegistrationOption, endpointOnlyOption, updateEndpointOption); + }, configOption, verboseOption, dryRunOption, skipEndpointRegistrationOption, endpointOnlyOption, updateEndpointOption, skipRequirementsOption); return command; } @@ -773,7 +810,9 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (string.IsNullOrWhiteSpace(existingServicePrincipalId)) { logger.LogDebug("Looking up service principal for blueprint..."); - var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync(tenantId, existingAppId, ct); + var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync( + tenantId, existingAppId, ct, + scopes: AuthenticationConstants.RequiredPermissionGrantScopes); if (spLookup.Found) { @@ -1339,6 +1378,20 @@ private static List GetApplicationScopes(Models.Agent365Config setupConf var applicationScopes = GetApplicationScopes(setupConfig, logger); bool consentAlreadyExists = false; + // Resolve blueprint SP object ID once — reused by both pre-check and polling. + // servicePrincipalId comes from generated config (persisted on previous runs). + // If absent, look it up using MSAL scopes that include Application.Read.All. + // Without Application.Read.All the az CLI token causes Graph to return empty results silently. + var blueprintSpId = servicePrincipalId; + if (string.IsNullOrWhiteSpace(blueprintSpId)) + { + logger.LogDebug("Looking up service principal for blueprint..."); + var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync( + tenantId, appId, ct, + scopes: AuthenticationConstants.RequiredPermissionGrantScopes); + blueprintSpId = spLookup.ObjectId; + } + // Only check for existing consent if blueprint already existed // New blueprints cannot have consent yet, so skip the verification if (alreadyExisted) @@ -1346,22 +1399,14 @@ private static List GetApplicationScopes(Models.Agent365Config setupConf logger.LogInformation("Verifying admin consent for application"); logger.LogDebug(" - Application scopes: {Scopes}", string.Join(", ", applicationScopes)); - // Check if consent already exists with required scopes - var blueprintSpId = servicePrincipalId; - if (string.IsNullOrWhiteSpace(blueprintSpId)) - { - logger.LogDebug("Looking up service principal for blueprint to check consent..."); - var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync(tenantId, appId, ct); - blueprintSpId = spLookup.ObjectId; - } - if (!string.IsNullOrWhiteSpace(blueprintSpId)) { - // Get Microsoft Graph service principal ID + // Get Microsoft Graph service principal ID (needs Application.Read.All) var graphSpId = await graphApiService.LookupServicePrincipalByAppIdAsync( tenantId, AuthenticationConstants.MicrosoftGraphResourceAppId, - ct); + ct, + AuthenticationConstants.RequiredPermissionGrantScopes); if (!string.IsNullOrWhiteSpace(graphSpId)) { @@ -1373,7 +1418,8 @@ private static List GetApplicationScopes(Models.Agent365Config setupConf graphSpId, applicationScopes, logger, - ct); + ct, + scopes: AuthenticationConstants.RequiredPermissionGrantScopes); } } @@ -1428,9 +1474,21 @@ await SetupHelpers.EnsureResourcePermissionsAsync( logger.LogInformation("Requesting admin consent for application"); logger.LogInformation(" - Application scopes: {Scopes}", string.Join(", ", applicationScopes)); logger.LogInformation("Opening browser for Graph API admin consent..."); - TryOpenBrowser(consentUrlGraph); + logger.LogInformation("If the browser does not open automatically, navigate to this URL to grant consent: {ConsentUrl}", consentUrlGraph); + BrowserHelper.TryOpenUrl(consentUrlGraph, logger); - var consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct); + bool consentSuccess; + if (!string.IsNullOrWhiteSpace(blueprintSpId)) + { + consentSuccess = await AdminConsentHelper.PollAdminConsentAsync( + graphApiService, logger, tenantId, blueprintSpId, + "Graph API Scopes", 180, 5, ct); + } + else + { + logger.LogDebug("Could not resolve blueprint service principal. Falling back to az rest polling."); + consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct); + } bool graphInheritablePermissionsConfigured = false; string? graphInheritablePermissionsError = null; @@ -1541,23 +1599,6 @@ private async static Task GetAuthenticatedGraphClientAsync(I } } - private static void TryOpenBrowser(string url) - { - try - { - using var p = new System.Diagnostics.Process(); - p.StartInfo = new System.Diagnostics.ProcessStartInfo - { - FileName = url, - UseShellExecute = true - }; - p.Start(); - } - catch - { - // non-fatal - } - } /// /// Creates client secret for Agent Blueprint (Phase 2.5) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs index 882eca85..15539f92 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; @@ -63,7 +64,7 @@ public static Command CreateCommand( if (string.IsNullOrWhiteSpace(setupConfig.AgentBlueprintId)) { logger.LogError("Blueprint ID not found. Run 'a365 setup blueprint' first."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } // Configure GraphApiService with custom client app ID if available @@ -72,6 +73,20 @@ public static Command CreateCommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations). + // Skipped in dry-run: PowerShellModulesRequirementCheck can auto-install modules, + // which would be a side effect in a mode that is supposed to be non-mutating. + if (!dryRun) + { + var systemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); + if (!systemChecksOk) + { + logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); + ExceptionHandler.ExitWithCleanup(1); + } + } + if (dryRun) { logger.LogInformation("DRY RUN: Configure CopilotStudio Permissions"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 5eaebdd8..f1870472 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -76,7 +76,7 @@ private static Command CreateMcpSubcommand( if (string.IsNullOrWhiteSpace(setupConfig.AgentBlueprintId)) { logger.LogError("Blueprint ID not found. Run 'a365 setup blueprint' first."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } // Configure GraphApiService with custom client app ID if available @@ -85,6 +85,20 @@ private static Command CreateMcpSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations). + // Skipped in dry-run: PowerShellModulesRequirementCheck can auto-install modules, + // which would be a side effect in a mode that is supposed to be non-mutating. + if (!dryRun) + { + var mcpSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); + if (!mcpSystemChecksOk) + { + logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); + ExceptionHandler.ExitWithCleanup(1); + } + } + if (dryRun) { // Read scopes from ToolingManifest.json @@ -154,7 +168,7 @@ private static Command CreateBotSubcommand( if (string.IsNullOrWhiteSpace(setupConfig.AgentBlueprintId)) { logger.LogError("Blueprint ID not found. Run 'a365 setup blueprint' first."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } // Configure GraphApiService with custom client app ID if available @@ -163,6 +177,20 @@ private static Command CreateBotSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations). + // Skipped in dry-run: PowerShellModulesRequirementCheck can auto-install modules, + // which would be a side effect in a mode that is supposed to be non-mutating. + if (!dryRun) + { + var botSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); + if (!botSystemChecksOk) + { + logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); + ExceptionHandler.ExitWithCleanup(1); + } + } + if (dryRun) { logger.LogInformation("DRY RUN: Configure Bot API Permissions"); @@ -228,7 +256,7 @@ private static Command CreateCustomSubcommand( if (string.IsNullOrWhiteSpace(setupConfig.AgentBlueprintId)) { logger.LogError("Blueprint ID not found. Run 'a365 setup blueprint' first."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } // Configure GraphApiService with custom client app ID if available @@ -237,6 +265,20 @@ private static Command CreateCustomSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations). + // Skipped in dry-run: PowerShellModulesRequirementCheck can auto-install modules, + // which would be a side effect in a mode that is supposed to be non-mutating. + if (!dryRun) + { + var customSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); + if (!customSystemChecksOk) + { + logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); + ExceptionHandler.ExitWithCleanup(1); + } + } + if (dryRun) { logger.LogInformation("DRY RUN: Configure Custom Blueprint Permissions"); @@ -466,7 +508,10 @@ private static async Task RemoveStaleCustomPermissionsAsync( AuthenticationConstants.MicrosoftGraphResourceAppId, }; - var requiredPermissions = new[] { "AgentIdentityBlueprint.UpdateAuthProperties.All", "Application.ReadWrite.All" }; + // Must match RequiredPermissionGrantScopes exactly so the PowerShell token acquired + // for inheritable permissions is reused (same cache key) rather than triggering + // a second Connect-MgGraph prompt. + var requiredPermissions = AuthenticationConstants.RequiredPermissionGrantScopes; List<(string ResourceAppId, List Scopes)> currentPermissions; try @@ -614,7 +659,7 @@ await RemoveStaleCustomPermissionsAsync( if (setupConfig.CustomBlueprintPermissions == null || setupConfig.CustomBlueprintPermissions.Count == 0) { - logger.LogInformation("No custom blueprint permissions configured."); + logger.LogInformation("No custom blueprint permissions specified in config. Skipping."); await configService.SaveStateAsync(setupConfig); return true; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 48dcdf75..9ea0af0b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -244,10 +244,20 @@ public static async Task EnsureResourcePermissionsAsync( { throw new SetupValidationException( "Failed to authenticate to Microsoft Graph with delegated permissions. " + - "Please sign in when prompted and ensure your account has the required roles and permission scopes."); + "Check the errors above for the specific cause. Common causes: " + + "missing PowerShell module (run 'a365 setup requirements' to install), " + + "insufficient permissions, or sign-in was cancelled."); } - var blueprintSpObjectId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, config.AgentBlueprintId, ct, permissionGrantScopes); + // Retry: Azure AD service principal propagation can lag 10-30s after blueprint creation. + var retryHelperSp = new RetryHelper(logger); + var blueprintSpObjectId = await retryHelperSp.ExecuteWithRetryAsync( + operation: (innerCt) => graph.LookupServicePrincipalByAppIdAsync(config.TenantId, config.AgentBlueprintId, innerCt, permissionGrantScopes), + shouldRetry: result => string.IsNullOrWhiteSpace(result), + maxRetries: 5, + baseDelaySeconds: 5, + cancellationToken: ct); + if (string.IsNullOrWhiteSpace(blueprintSpObjectId)) { throw new SetupValidationException($"Blueprint Service Principal not found for appId {config.AgentBlueprintId}. " + @@ -304,11 +314,11 @@ public static async Task EnsureResourcePermissionsAsync( logger.LogInformation(" - Configuring inheritable permissions: blueprint {Blueprint} to resourceAppId {ResourceAppId} scopes [{Scopes}]", config.AgentBlueprintId, resourceAppId, string.Join(' ', scopes)); - // Use custom client app auth for inheritable permissions - Azure CLI doesn't support this operation - var requiredPermissions = new[] { "AgentIdentityBlueprint.UpdateAuthProperties.All", "Application.ReadWrite.All" }; - + // Use custom client app auth for inheritable permissions - Azure CLI doesn't support this operation. + // Reuse permissionGrantScopes (which already includes AgentIdentityBlueprint.UpdateAuthProperties.All) + // so all Graph PowerShell calls in this method share a single Connect-MgGraph session/cache entry. var (ok, alreadyExists, err) = await blueprintService.SetInheritablePermissionsAsync( - config.TenantId, config.AgentBlueprintId, resourceAppId, scopes, requiredScopes: requiredPermissions, ct); + config.TenantId, config.AgentBlueprintId, resourceAppId, scopes, requiredScopes: permissionGrantScopes, ct); if (!ok && !alreadyExists) { @@ -338,7 +348,7 @@ public static async Task EnsureResourcePermissionsAsync( operation: async (ct) => { var (exists, verifiedScopes, verifyError) = await blueprintService.VerifyInheritablePermissionsAsync( - config.TenantId, config.AgentBlueprintId, resourceAppId, ct, requiredPermissions); + config.TenantId, config.AgentBlueprintId, resourceAppId, ct, permissionGrantScopes); return (exists, verifiedScopes, verifyError); }, shouldRetry: (result) => diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs index 4211d66d..dfea9ecc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs @@ -104,14 +104,17 @@ public static string[] GetRequiredRedirectUris(string clientAppId) }; /// - /// Required scopes for oauth2 permission grants to service principals. - /// These scopes enable the service principals to operate correctly with the necessary permissions. - /// All scopes require admin consent. + /// Required scopes for all PowerShell-based Microsoft Graph operations (OAuth2 grants, + /// service principal lookups, and inheritable permissions). + /// Using a single unified set ensures Connect-MgGraph authenticates once and the resulting + /// token is reused from the in-process cache for all downstream Graph operations. + /// All scopes require admin consent and are included in RequiredClientAppPermissions. /// public static readonly string[] RequiredPermissionGrantScopes = new[] { "Application.ReadWrite.All", - "DelegatedPermissionGrant.ReadWrite.All" + "DelegatedPermissionGrant.ReadWrite.All", + "AgentIdentityBlueprint.UpdateAuthProperties.All" }; /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs new file mode 100644 index 00000000..d15f7906 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.IO; + +namespace Microsoft.Agents.A365.DevTools.Cli.Exceptions; + +/// +/// Exception thrown when the a365.config.json configuration file cannot be found. +/// This is a USER ERROR - the file is missing or the command was run from the wrong directory. +/// Derives from FileNotFoundException so existing callers that catch FileNotFoundException +/// continue to work without changes. +/// +public class ConfigFileNotFoundException : FileNotFoundException +{ + public ConfigFileNotFoundException(string configFilePath) + : base( + message: $"Configuration file not found: {configFilePath}. " + + "Make sure you are running this command from your agent project directory. " + + "If you have not created a configuration file yet, run: a365 config init", + fileName: configFilePath) + { + } + + public int ExitCode => 2; // Configuration error +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs new file mode 100644 index 00000000..e3167dd4 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Diagnostics; +using System.Runtime.InteropServices; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Helpers; + +/// +/// Helper methods for cross-platform browser and URL operations. +/// +public static class BrowserHelper +{ + /// + /// Opens a URL in the system's default browser in a cross-platform way. + /// Non-fatal: if the browser cannot be launched, logs a warning via + /// when provided, or writes to when logger is null. + /// The fallback URL is always emitted so the user can open it manually. + /// + /// The URL to open. + /// Optional logger for diagnostic messages. + public static void TryOpenUrl(string url, ILogger? logger = null) + { + try + { + ProcessStartInfo psi; + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + psi = new ProcessStartInfo { FileName = url, UseShellExecute = true }; + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + psi = new ProcessStartInfo { FileName = "open" }; + psi.ArgumentList.Add(url); + } + else + { + psi = new ProcessStartInfo { FileName = "xdg-open" }; + psi.ArgumentList.Add(url); + } + using var process = new Process { StartInfo = psi }; + process.Start(); + } + catch (Exception ex) + { + if (logger != null) + { + logger.LogWarning(ex, "Failed to open browser automatically for URL {Url}", url); + logger.LogInformation("Please manually open: {Url}", url); + } + else + { + Console.Error.WriteLine($"Failed to open browser automatically: {ex.Message}"); + Console.Error.WriteLine($"Please manually open: {url}"); + } + } + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs index 452b142a..060dcba9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs @@ -2,10 +2,10 @@ // Licensed under the MIT License. using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Extensions.Logging; -using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text.Json; using System.Text.Json.Nodes; @@ -1046,7 +1046,7 @@ private async Task RequestAdminConsentAsync( _logger.LogInformation("URL: {Url}", consentUrl); // Open browser - TryOpenBrowser(consentUrl); + BrowserHelper.TryOpenUrl(consentUrl, _logger); _logger.LogInformation(""); _logger.LogInformation("Waiting for admin consent (timeout: {Timeout} seconds)...", timeoutSeconds); @@ -1131,24 +1131,6 @@ private async Task RequestAdminConsentAsync( return false; } - private void TryOpenBrowser(string url) - { - try - { - using var process = new System.Diagnostics.Process(); - process.StartInfo = new System.Diagnostics.ProcessStartInfo - { - FileName = url, - UseShellExecute = true - }; - process.Start(); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to open browser automatically"); - _logger.LogInformation("Please manually open: {Url}", url); - } - } /// /// Verify that a service principal exists in Azure AD for the given app ID. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AdminConsentHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AdminConsentHelper.cs deleted file mode 100644 index e69de29b..00000000 diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs index 76ce488b..6d8ea44d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs @@ -324,7 +324,7 @@ private async Task CacheTokenAsync(string resourceUrl, TokenInfo token) cache.Tokens[resourceUrl] = token; var updatedJson = JsonSerializer.Serialize(cache, new JsonSerializerOptions { WriteIndented = true }); await File.WriteAllTextAsync(_tokenCachePath, updatedJson); - _logger.LogInformation("Authentication token cached for {ResourceUrl} at: {Path}", resourceUrl, _tokenCachePath); + _logger.LogDebug("Authentication token cached for {ResourceUrl} at: {Path}", resourceUrl, _tokenCachePath); } /// @@ -535,14 +535,14 @@ protected virtual TokenCredential CreateDeviceCodeCredential(string clientId, st }, DeviceCodeCallback = (code, cancellation) => { - Console.WriteLine(); - Console.WriteLine("=========================================================================="); - Console.WriteLine($"To sign in, use a web browser to open the page:"); - Console.WriteLine($" {code.VerificationUri}"); - Console.WriteLine(); - Console.WriteLine($"And enter the code: {code.UserCode}"); - Console.WriteLine("=========================================================================="); - Console.WriteLine(); + _logger.LogInformation(""); + _logger.LogInformation("=========================================================================="); + _logger.LogInformation("To sign in, use a web browser to open the page:"); + _logger.LogInformation(" {VerificationUri}", code.VerificationUri); + _logger.LogInformation(""); + _logger.LogInformation("And enter the code: {UserCode}", code.UserCode); + _logger.LogInformation("=========================================================================="); + _logger.LogInformation(""); return Task.CompletedTask; } }); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs index 8d40d2ea..fe9bf45d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs @@ -244,30 +244,18 @@ public async Task GetServicePrincipalByObjectIdAsy public async Task GetServicePrincipalByAppIdAsync( string tenantId, string appId, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default, + IEnumerable? scopes = null) { try { _logger.LogDebug("Looking up service principal by appId: {AppId}", appId); - var filter = $"appId eq '{appId}'"; - var doc = await _graphApiService.GraphGetAsync( - tenantId, - $"/v1.0/servicePrincipals?$filter={Uri.EscapeDataString(filter)}", - cancellationToken); - - if (doc == null) - { - _logger.LogDebug("No service principal found with appId: {AppId}", appId); - return new ServicePrincipalLookupResult - { - Found = false, - LookupMethod = "appId" - }; - } + // Pass scopes so the caller can opt-in to a MSAL token with Application.Read.All. + // Without Application.Read.All the az CLI token causes Graph to return an empty array silently. + var objectId = await _graphApiService.LookupServicePrincipalByAppIdAsync(tenantId, appId, cancellationToken, scopes); - var root = doc.RootElement; - if (!root.TryGetProperty("value", out var valueElement) || valueElement.GetArrayLength() == 0) + if (objectId == null) { _logger.LogDebug("No service principal found with appId: {AppId}", appId); return new ServicePrincipalLookupResult @@ -277,19 +265,14 @@ public async Task GetServicePrincipalByAppIdAsync( }; } - var firstMatch = valueElement[0]; - var objectId = firstMatch.GetProperty("id").GetString(); - var displayName = firstMatch.GetProperty("displayName").GetString(); - - _logger.LogDebug("Found service principal: {DisplayName} (ObjectId: {ObjectId}, AppId: {AppId})", - displayName, objectId, appId); + _logger.LogDebug("Found service principal (ObjectId: {ObjectId}, AppId: {AppId})", objectId, appId); + // Note: DisplayName is not queried in this lookup path ($select=id only) — callers must not rely on it being populated. return new ServicePrincipalLookupResult { Found = true, ObjectId = objectId, AppId = appId, - DisplayName = displayName, LookupMethod = "appId", RequiresPersistence = true }; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs index e62de940..bd724030 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs @@ -113,6 +113,9 @@ public async Task EnsureValidClientAppAsync( // Step 6: Verify and fix redirect URIs await EnsureRedirectUrisAsync(clientAppId, graphToken, ct); + // Step 7: Verify and fix public client flows (required for device code fallback on non-Windows) + await EnsurePublicClientFlowsEnabledAsync(clientAppId, graphToken, ct); + _logger.LogDebug("Client app validation successful for {ClientAppId}", clientAppId); } catch (ClientAppValidationException) @@ -237,6 +240,82 @@ public async Task EnsureRedirectUrisAsync( } } + /// + /// Ensures the app registration has "Allow public client flows" enabled. + /// This setting is required for MSAL device code authentication fallback on non-Windows + /// platforms where interactive browser auth is unavailable (macOS headless, Linux, WSL). + /// Automatically enables it if disabled (self-healing). + /// + private async Task EnsurePublicClientFlowsEnabledAsync( + string clientAppId, + string graphToken, + CancellationToken ct = default) + { + try + { + _logger.LogDebug("Checking 'Allow public client flows' for client app {ClientAppId}", clientAppId); + + var appCheckResult = await _executor.ExecuteAsync( + "az", + $"rest --method GET --url \"{GraphApiBaseUrl}/applications?$filter=appId eq '{CommandStringHelper.EscapePowerShellString(clientAppId)}'&$select=id,isFallbackPublicClient\" --headers \"Authorization=Bearer {CommandStringHelper.EscapePowerShellString(graphToken)}\"", + cancellationToken: ct); + + if (!appCheckResult.Success) + { + _logger.LogWarning("Could not check 'Allow public client flows': {Error}", appCheckResult.StandardError); + return; + } + + var sanitizedOutput = JsonDeserializationHelper.CleanAzureCliJsonOutput(appCheckResult.StandardOutput); + var response = JsonNode.Parse(sanitizedOutput); + var apps = response?["value"]?.AsArray(); + + if (apps == null || apps.Count == 0) + { + _logger.LogWarning("Client app not found when checking 'Allow public client flows'"); + return; + } + + var app = apps[0]!.AsObject(); + var objectId = app["id"]?.GetValue(); + + if (string.IsNullOrWhiteSpace(objectId)) + { + _logger.LogWarning("Could not get application object ID when checking 'Allow public client flows'"); + return; + } + + var isFallbackPublicClient = app["isFallbackPublicClient"]?.GetValue() ?? false; + if (isFallbackPublicClient) + { + _logger.LogDebug("'Allow public client flows' is already enabled"); + return; + } + + _logger.LogInformation("Enabling 'Allow public client flows' on app registration (required for device code authentication fallback on macOS/Linux)."); + _logger.LogInformation("Run 'a365 setup requirements' at any time to re-verify and auto-fix this setting."); + + var patchBody = "{\"isFallbackPublicClient\":true}"; + var escapedBody = patchBody.Replace("\"", "\"\""); + var patchResult = await _executor.ExecuteAsync( + "az", + $"rest --method PATCH --url \"{GraphApiBaseUrl}/applications/{CommandStringHelper.EscapePowerShellString(objectId)}\" --headers \"Content-Type=application/json\" \"Authorization=Bearer {CommandStringHelper.EscapePowerShellString(graphToken)}\" --body \"{escapedBody}\"", + cancellationToken: ct); + + if (!patchResult.Success) + { + _logger.LogWarning("Failed to enable 'Allow public client flows': {Error}", patchResult.StandardError); + return; + } + + _logger.LogInformation("Successfully enabled 'Allow public client flows' on app registration."); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error ensuring 'Allow public client flows' is enabled (non-fatal)"); + } + } + #region Private Helper Methods private async Task AcquireGraphTokenAsync(CancellationToken ct) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs index 129df504..ba47a82c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs @@ -114,6 +114,7 @@ public virtual async Task ExecuteWithStreamingAsync( string? workingDirectory = null, string outputPrefix = "", bool interactive = false, + Func? outputTransform = null, CancellationToken cancellationToken = default) { _logger.LogDebug("Executing with streaming: {Command} {Arguments} (Interactive={Interactive})", command, arguments, interactive); @@ -156,7 +157,12 @@ public virtual async Task ExecuteWithStreamingAsync( // Don't print JWT tokens to console (security) if (!IsJwtToken(args.Data)) { - Console.WriteLine($"{outputPrefix}{args.Data}"); + var display = outputTransform?.Invoke(args.Data) ?? args.Data; + if (display != null) + { + // outputPrefix is prepended only to the first line; callers must not return multi-line strings from outputTransform. + Console.WriteLine($"{outputPrefix}{display}"); + } } else { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs index 332e0003..f3ed912d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; namespace Microsoft.Agents.A365.DevTools.Cli.Services; @@ -241,7 +242,7 @@ public async Task LoadAsync( if (!File.Exists(resolvedConfigPath)) { _logger?.LogError("Static configuration file not found: {ConfigPath}", resolvedConfigPath); - throw new FileNotFoundException($"{ErrorMessages.ConfigFileNotFound} (Path: {resolvedConfigPath})"); + throw new ConfigFileNotFoundException(resolvedConfigPath); } // Load static configuration (required) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index edc1f84a..de3cbdad 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -288,7 +288,7 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo if (!resp.IsSuccessStatusCode) { var errorBody = await resp.Content.ReadAsStringAsync(ct); - _logger.LogError("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, errorBody); + _logger.LogDebug("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, errorBody); return null; } var json = await resp.Content.ReadAsStringAsync(ct); @@ -408,7 +408,13 @@ public async Task GraphDeleteAsync( public virtual async Task LookupServicePrincipalByAppIdAsync( string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) { - var doc = await GraphGetAsync(tenantId, $"/v1.0/servicePrincipals?$filter=appId eq '{appId}'&$select=id", ct, scopes); + // $filter=appId eq is "Default+Advanced" per Graph docs -� no ConsistencyLevel header required. + // The token must have Application.Read.All; pass scopes to ensure MSAL token is used when needed. + using var doc = await GraphGetAsync( + tenantId, + $"/v1.0/servicePrincipals?$filter=appId eq '{appId}'&$select=id", + ct, + scopes); if (doc == null) return null; if (!doc.RootElement.TryGetProperty("value", out var value) || value.GetArrayLength() == 0) return null; return value[0].GetProperty("id").GetString(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs index 7b1824cf..5c06052a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs @@ -5,6 +5,7 @@ using System.Text.Json; using System.Threading; using System.Threading.Tasks; +using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Extensions.Logging; namespace Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; @@ -30,11 +31,25 @@ public static async Task PollAdminConsentAsync( { var start = DateTime.UtcNow; string? spId = null; + int lastProgressReportSeconds = 0; + + logger.LogInformation( + "Waiting for admin consent to be granted. Open the URL above in a browser and complete the consent flow. The CLI will continue automatically (timeout: {TimeoutSeconds}s).", + timeoutSeconds); try { while ((DateTime.UtcNow - start).TotalSeconds < timeoutSeconds && !ct.IsCancellationRequested) { + var elapsedSeconds = (int)(DateTime.UtcNow - start).TotalSeconds; + if (elapsedSeconds > 0 && elapsedSeconds - lastProgressReportSeconds >= 60) + { + lastProgressReportSeconds = elapsedSeconds; + logger.LogInformation( + "Still waiting for admin consent... ({ElapsedSeconds}s / {TimeoutSeconds}s).", + elapsedSeconds, timeoutSeconds); + } + if (spId == null) { var spResult = await executor.ExecuteAsync("az", @@ -83,6 +98,9 @@ public static async Task PollAdminConsentAsync( await Task.Delay(TimeSpan.FromSeconds(intervalSeconds), ct); } + logger.LogWarning( + "Admin consent was not detected within {TimeoutSeconds}s. Continuing — you can re-run this command after granting consent.", + timeoutSeconds); return false; } catch (OperationCanceledException) @@ -93,6 +111,81 @@ public static async Task PollAdminConsentAsync( } } + /// + /// Polls Microsoft Graph directly (via MSAL token) to detect an oauth2 permission grant. + /// Preferred over the az-cli-based overload for cross-platform compatibility. + /// Caller must supply the blueprint service principal object ID directly to avoid + /// a servicePrincipals $filter query that requires ConsistencyLevel: eventual. + /// + public static async Task PollAdminConsentAsync( + Services.GraphApiService graphApiService, + ILogger logger, + string tenantId, + string clientSpId, + string scopeDescriptor, + int timeoutSeconds, + int intervalSeconds, + CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(clientSpId)) + { + logger.LogDebug("Blueprint service principal ID not available, falling back to az rest polling."); + return false; + } + + var start = DateTime.UtcNow; + int lastProgressReportSeconds = 0; + + logger.LogInformation( + "Waiting for admin consent to be granted. Open the URL above in a browser and complete the consent flow. The CLI will continue automatically (timeout: {TimeoutSeconds}s).", + timeoutSeconds); + + try + { + while ((DateTime.UtcNow - start).TotalSeconds < timeoutSeconds && !ct.IsCancellationRequested) + { + var elapsedSeconds = (int)(DateTime.UtcNow - start).TotalSeconds; + if (elapsedSeconds > 0 && elapsedSeconds - lastProgressReportSeconds >= 60) + { + lastProgressReportSeconds = elapsedSeconds; + logger.LogInformation( + "Still waiting for admin consent... ({ElapsedSeconds}s / {TimeoutSeconds}s).", + elapsedSeconds, timeoutSeconds); + } + + // Mirror original az-rest polling behavior: check for any grant for clientId. + // No resourceId filter or scope check — consent just needs to exist. + var grantDoc = await graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpId}'", + ct, + AuthenticationConstants.RequiredPermissionGrantScopes); + + if (grantDoc != null && + grantDoc.RootElement.TryGetProperty("value", out var arr) && + arr.GetArrayLength() > 0) + { + logger.LogInformation("Consent granted ({ScopeDescriptor}).", scopeDescriptor); + return true; + } + + logger.LogDebug("No consent grants found for blueprint SP {ClientSpId} yet.", clientSpId); + + await Task.Delay(TimeSpan.FromSeconds(intervalSeconds), ct); + } + + logger.LogWarning( + "Admin consent was not detected within {TimeoutSeconds}s. Continuing — you can re-run this command after granting consent.", + timeoutSeconds); + return false; + } + catch (OperationCanceledException) + { + logger.LogDebug("Polling for admin consent was cancelled or timed out for SP {ClientSpId} ({Scope}).", clientSpId, scopeDescriptor); + return false; + } + } + /// /// Checks if admin consent already exists for specified scopes between client and resource service principals. /// Returns true if ALL required scopes are present in existing oauth2PermissionGrants. @@ -104,6 +197,8 @@ public static async Task PollAdminConsentAsync( /// List of required scope names (case-insensitive) /// Logger for diagnostics /// Cancellation token + /// OAuth2 scopes for Graph token acquisition. Should include DelegatedPermissionGrant.ReadWrite.All + /// to read oauth2PermissionGrants. When null, falls back to Azure CLI token which may lack required permissions. /// True if all required scopes are already granted, false otherwise public static async Task CheckConsentExistsAsync( Services.GraphApiService graphApiService, @@ -112,7 +207,8 @@ public static async Task CheckConsentExistsAsync( string resourceSpId, System.Collections.Generic.IEnumerable requiredScopes, ILogger logger, - CancellationToken ct) + CancellationToken ct, + System.Collections.Generic.IEnumerable? scopes = null) { if (string.IsNullOrWhiteSpace(clientSpId) || string.IsNullOrWhiteSpace(resourceSpId)) { @@ -123,11 +219,13 @@ public static async Task CheckConsentExistsAsync( try { - // Query existing grants + // Query existing grants — pass scopes so EnsureGraphHeadersAsync uses the MSAL token provider + // (which has DelegatedPermissionGrant.ReadWrite.All) instead of falling back to the Azure CLI token. var grantDoc = await graphApiService.GraphGetAsync( tenantId, $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpId}' and resourceId eq '{resourceSpId}'", - ct); + ct, + scopes); if (grantDoc == null || !grantDoc.RootElement.TryGetProperty("value", out var grants) || grants.GetArrayLength() == 0) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs index 38c9d62b..b283f175 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using Azure.Core; -using Azure.Identity; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Extensions.Logging; @@ -26,6 +25,7 @@ public sealed class InteractiveGraphAuthService { private readonly ILogger _logger; private readonly string _clientAppId; + private readonly Func? _credentialFactory; private GraphServiceClient? _cachedClient; private string? _cachedTenantId; @@ -40,7 +40,8 @@ public sealed class InteractiveGraphAuthService public InteractiveGraphAuthService( ILogger logger, - string clientAppId) + string clientAppId, + Func? credentialFactory = null) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -59,13 +60,18 @@ public InteractiveGraphAuthService( } _clientAppId = clientAppId; + _credentialFactory = credentialFactory; } /// /// Gets an authenticated GraphServiceClient using interactive browser authentication. /// Caches the client instance to avoid repeated authentication prompts. + /// + /// NOTE: GraphServiceClient acquires tokens lazily (on first API call). To surface + /// authentication failures early and ensure the "success" log is accurate, this method + /// eagerly acquires a token before constructing the client. /// - public Task GetAuthenticatedGraphClientAsync( + public async Task GetAuthenticatedGraphClientAsync( string tenantId, CancellationToken cancellationToken = default) { @@ -73,7 +79,7 @@ public Task GetAuthenticatedGraphClientAsync( if (_cachedClient != null && _cachedTenantId == tenantId) { _logger.LogDebug("Reusing cached Graph client for tenant {TenantId}", tenantId); - return Task.FromResult(_cachedClient); + return _cachedClient; } _logger.LogInformation("Attempting to authenticate to Microsoft Graph interactively..."); @@ -83,46 +89,35 @@ public Task GetAuthenticatedGraphClientAsync( _logger.LogInformation("Please sign in with an account that has Global Administrator or similar privileges."); _logger.LogInformation(""); - // Use browser authentication (MsalBrowserCredential handles WAM on Windows and browser on other platforms) + _logger.LogInformation("Authenticating to Microsoft Graph..."); + _logger.LogInformation("IMPORTANT: You must grant consent for all required permissions."); + _logger.LogInformation("Required permissions are defined in AuthenticationConstants.RequiredClientAppPermissions."); + _logger.LogInformation($"See {ConfigConstants.Agent365CliDocumentationUrl} for the complete list."); + _logger.LogInformation(""); + + // Eagerly acquire a token so authentication failures are detected here rather than + // surfacing later from inside GraphServiceClient's lazy token acquisition. + // Resolve credential inside try/catch so factory exceptions are wrapped consistently. + var tokenContext = new TokenRequestContext(RequiredScopes); + TokenCredential? credential = null; try { - // Pass null for redirectUri to let MsalBrowserCredential decide based on platform - var browserCredential = new MsalBrowserCredential( - _clientAppId, - tenantId, - redirectUri: null, // Let MsalBrowserCredential use WAM on Windows - _logger); - - _logger.LogInformation("Authenticating to Microsoft Graph..."); - _logger.LogInformation("IMPORTANT: You must grant consent for all required permissions."); - _logger.LogInformation("Required permissions are defined in AuthenticationConstants.RequiredClientAppPermissions."); - _logger.LogInformation($"See {ConfigConstants.Agent365CliDocumentationUrl} for the complete list."); - _logger.LogInformation(""); - - // Create GraphServiceClient with the credential - var graphClient = new GraphServiceClient(browserCredential, RequiredScopes); - - _logger.LogInformation("Successfully authenticated to Microsoft Graph!"); - _logger.LogInformation(""); - - // Cache the client for reuse - _cachedClient = graphClient; - _cachedTenantId = tenantId; - - return Task.FromResult(graphClient); + // Resolve credential: use injected factory (for tests) or default MsalBrowserCredential + credential = _credentialFactory?.Invoke(_clientAppId, tenantId) + ?? new MsalBrowserCredential(_clientAppId, tenantId, redirectUri: null, _logger); + + await credential.GetTokenAsync(tokenContext, cancellationToken); } - catch (MsalAuthenticationFailedException ex) when (ex.Message.Contains("invalid_grant")) + catch (MsalAuthenticationFailedException ex) when (ex.Message.Contains("invalid_grant", StringComparison.Ordinal)) { - // Permissions issue ThrowInsufficientPermissionsException(ex); throw; // Unreachable but required for compiler } catch (MsalAuthenticationFailedException ex) when ( - ex.Message.Contains("localhost") || - ex.Message.Contains("connection") || - ex.Message.Contains("redirect_uri")) + ex.Message.Contains("localhost", StringComparison.Ordinal) || + ex.Message.Contains("connection", StringComparison.Ordinal) || + ex.Message.Contains("redirect_uri", StringComparison.Ordinal)) { - // Infrastructure/connectivity issue _logger.LogError("Browser authentication failed due to connectivity issue: {Message}", ex.Message); throw new GraphApiException( "Browser authentication", @@ -145,6 +140,18 @@ public Task GetAuthenticatedGraphClientAsync( $"Authentication failed: {ex.Message}", isPermissionIssue: false); } + + // Token acquired successfully — log and construct the client. + // MsalBrowserCredential caches the MSAL account, so subsequent GetTokenAsync calls + // from GraphServiceClient will hit the silent cache without re-prompting. + _logger.LogInformation("Successfully authenticated to Microsoft Graph!"); + _logger.LogInformation(""); + + var graphClient = new GraphServiceClient(credential!, RequiredScopes); + _cachedClient = graphClient; + _cachedTenantId = tenantId; + + return graphClient; } private void ThrowInsufficientPermissionsException(Exception innerException) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index 40af220f..fb04aee9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -5,10 +5,12 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Runtime.InteropServices; using System.Text; using System.Text.Json; using System.Threading; using System.Threading.Tasks; +using Azure.Core; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Extensions.Logging; @@ -117,14 +119,29 @@ public MicrosoftGraphTokenProvider( return cached.AccessToken; } - _logger.LogInformation( - "Acquiring Microsoft Graph delegated access token via PowerShell (Device Code: {UseDeviceCode})", - useDeviceCode); + _logger.LogInformation("Acquiring Microsoft Graph delegated access token via PowerShell..."); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + _logger.LogInformation("A browser window will open for authentication. Complete sign-in, then return here — the CLI will continue automatically."); + } + else + { + _logger.LogInformation("A device code prompt will appear below. Open the URL in any browser, enter the code, complete sign-in, then return here — the CLI will continue automatically."); + } var script = BuildPowerShellScript(tenantId, validatedScopes, useDeviceCode, clientAppId); var result = await ExecuteWithFallbackAsync(script, ct); var token = ProcessResult(result); + // If PS Connect-MgGraph fails for any reason (no TTY on Linux, NullRef in DeviceCodeCredential, + // module issues, etc.), fall back to MSAL. On Windows this uses WAM; on Linux/macOS it uses + // device code. The acquired token is stored in _tokenCache below so subsequent calls + // (inheritable permissions, custom permissions) hit the cache without re-prompting. + if (string.IsNullOrWhiteSpace(token)) + { + token = await AcquireGraphTokenViaMsalAsync(tenantId, validatedScopes, clientAppId, ct); + } + if (string.IsNullOrWhiteSpace(token)) { return null; @@ -204,23 +221,20 @@ private static string BuildPowerShellScript(string tenantId, string[] scopes, bo ? $" -ClientId '{CommandStringHelper.EscapePowerShellString(clientAppId)}'" : ""; - // Workaround for older Microsoft.Graph versions that don't have Get-MgAccessToken - // We make a dummy Graph request and extract the token from the Authorization header + // Extract the access token from the Authorization header of a live Graph request. + // $ctx.AccessToken is NOT used because Microsoft.Graph.Authentication v2+ returns an + // opaque (non-JWT) value that is rejected when used as a Bearer token in Graph API calls. + // The Authorization header on an actual request always contains the real JWT Bearer token. return $"Import-Module Microsoft.Graph.Authentication -ErrorAction Stop; " + $"Connect-MgGraph -TenantId '{escapedTenantId}'{clientIdParam} -Scopes {scopesArray} {authMethod} -NoWelcome -ErrorAction Stop; " + $"$ctx = Get-MgContext; " + $"if ($null -eq $ctx) {{ throw 'Failed to establish Graph context' }}; " + - // Try to get token directly if available (newer versions) - $"if ($ctx.PSObject.Properties.Name -contains 'AccessToken' -and -not [string]::IsNullOrWhiteSpace($ctx.AccessToken)) {{ " + - $" $ctx.AccessToken " + - $"}} else {{ " + - // Fallback: Extract token from a test Graph request (older versions) - $" $response = Invoke-MgGraphRequest -Method GET -Uri 'https://graph.microsoft.com/v1.0/$metadata' -OutputType HttpResponseMessage -ErrorAction Stop; " + - $" $token = $response.RequestMessage.Headers.Authorization.Parameter; " + - $" if ([string]::IsNullOrWhiteSpace($token)) {{ throw 'Failed to extract access token from request' }}; " + - $" $token " + - $"}}"; + $"$response = Invoke-MgGraphRequest -Method GET -Uri 'https://graph.microsoft.com/v1.0/$metadata' -OutputType HttpResponseMessage -ErrorAction Stop; " + + $"$token = $response.RequestMessage.Headers.Authorization.Parameter; " + + $"$response.Dispose(); " + + $"if ([string]::IsNullOrWhiteSpace($token)) {{ throw 'Failed to extract access token from Graph request headers' }}; " + + $"$token"; } private static string BuildScopesArray(string[] scopes) @@ -234,18 +248,112 @@ private async Task ExecuteWithFallbackAsync( CancellationToken ct) { // Try PowerShell Core first (cross-platform) - var result = await ExecutePowerShellAsync("pwsh", script, ct); + var shell = "pwsh"; + var result = await ExecutePowerShellAsync(shell, script, ct); // Fallback to Windows PowerShell if pwsh is not available if (!result.Success && IsPowerShellNotFoundError(result)) { _logger.LogDebug("PowerShell Core not found, falling back to Windows PowerShell"); - result = await ExecutePowerShellAsync("powershell", script, ct); + shell = "powershell"; + result = await ExecutePowerShellAsync(shell, script, ct); + } + + // If the failure is due to a missing or broken module, attempt auto-install and retry once. + // This handles cases where Get-Module -ListAvailable reports the module as present but + // Import-Module fails at runtime (e.g., corrupt install, partial uninstall, path mismatch). + if (!result.Success && IsPowerShellModuleMissingError(result)) + { + if (await TryAutoInstallRequiredModulesAsync(shell, ct)) + { + _logger.LogInformation("Auto-installed missing PowerShell module(s). Retrying..."); + result = await ExecutePowerShellAsync(shell, script, ct); + } } return result; } + /// + /// Acquires a Microsoft Graph access token via MSAL as a fallback when PowerShell + /// Connect-MgGraph fails for any reason. On Windows uses WAM; on Linux/macOS uses device code. + /// Uses MsalBrowserCredential which shares the static in-process token cache, so a token + /// acquired here is reused silently on subsequent calls within the same CLI invocation. + /// + private async Task AcquireGraphTokenViaMsalAsync( + string tenantId, + string[] scopes, + string? clientAppId, + CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(clientAppId)) + { + _logger.LogDebug("No client app ID available for MSAL Graph fallback."); + return null; + } + + try + { + // MSAL requires fully-qualified scope URIs; PS Connect-MgGraph handles this internally. + var fullScopes = scopes + .Select(s => s.Contains("://", StringComparison.Ordinal) ? s : $"https://graph.microsoft.com/{s}") + .ToArray(); + + _logger.LogDebug("Acquiring Graph token via MSAL for scopes: {Scopes}", string.Join(", ", fullScopes)); + + var msalCredential = new MsalBrowserCredential(clientAppId, tenantId, logger: _logger); + var tokenResult = await msalCredential.GetTokenAsync(new TokenRequestContext(fullScopes), ct); + + if (string.IsNullOrWhiteSpace(tokenResult.Token)) + return null; + + _logger.LogInformation("Microsoft Graph access token acquired via MSAL fallback."); + return tokenResult.Token; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "MSAL Graph token fallback failed: {Message}", ex.Message); + return null; + } + } + + /// + /// Attempts to auto-install the PowerShell modules required for Graph token acquisition. + /// Returns true if installation succeeded, false otherwise. + /// + private async Task TryAutoInstallRequiredModulesAsync(string shell, CancellationToken ct) + { + _logger.LogInformation("Detected missing or broken PowerShell module. Attempting auto-install..."); + try + { + var installScript = + "Install-Module -Name 'Microsoft.Graph.Authentication' -Repository 'PSGallery' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop; " + + "Install-Module -Name 'Microsoft.Graph.Applications' -Repository 'PSGallery' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop"; + var result = await ExecutePowerShellAsync(shell, installScript, ct); + if (result.Success) + { + _logger.LogInformation("PowerShell modules auto-installed successfully."); + return true; + } + _logger.LogWarning("Auto-install of PowerShell modules failed: {Error}", result.StandardError); + return false; + } + catch (Exception ex) + { + _logger.LogWarning("Auto-install of PowerShell modules threw an exception: {Error}", ex.Message); + return false; + } + } + + private static bool IsPowerShellModuleMissingError(CommandResult result) + { + if (string.IsNullOrWhiteSpace(result.StandardError)) return false; + var error = result.StandardError; + return error.Contains("module", StringComparison.OrdinalIgnoreCase) && + (error.Contains("was not loaded", StringComparison.OrdinalIgnoreCase) || + error.Contains("not found in any module", StringComparison.OrdinalIgnoreCase)); + } + private async Task ExecutePowerShellAsync( string shell, string script, @@ -259,9 +367,46 @@ private async Task ExecutePowerShellAsync( workingDirectory: null, outputPrefix: "", interactive: true, + outputTransform: FormatDeviceCodeLine, cancellationToken: ct); } + /// + /// Intercepts the PS Connect-MgGraph device code line and reformats it to match the MSAL box format. + /// Input: "To sign in, use a web browser to open the page {url} and enter the code {code} to authenticate." + /// Output: the MSAL-style === box with URL and code on separate lines. + /// Returns null to suppress the original line; returns the line unchanged for all other output. + /// + private static string? FormatDeviceCodeLine(string line) + { + const string marker = "To sign in, use a web browser to open the page "; + const string codeMarker = " and enter the code "; + const string suffix = " to authenticate."; + + if (!line.Contains(marker, StringComparison.OrdinalIgnoreCase)) + return line; + + try + { + var pageStart = line.IndexOf(marker, StringComparison.OrdinalIgnoreCase) + marker.Length; + var codeStart = line.IndexOf(codeMarker, pageStart, StringComparison.OrdinalIgnoreCase); + if (codeStart < 0) return line; + + var url = line[pageStart..codeStart].Trim(); + var codeEnd = line.IndexOf(suffix, codeStart + codeMarker.Length, StringComparison.OrdinalIgnoreCase); + var code = codeEnd >= 0 + ? line[(codeStart + codeMarker.Length)..codeEnd].Trim() + : line[(codeStart + codeMarker.Length)..].Trim(); + + var sep = new string('=', 74); + return $"{sep}\nTo sign in, use a web browser to open the page:\n {url}\nAnd enter the code: {code}\n{sep}"; + } + catch + { + return line; + } + } + private static string BuildPowerShellArguments(string shell, string script) { var baseArgs = shell == "pwsh" @@ -280,10 +425,25 @@ private static string BuildPowerShellArguments(string shell, string script) _logger.LogError( "Failed to acquire Microsoft Graph access token. Error: {Error}", result.StandardError); + + if (IsPowerShellModuleMissingError(result)) + { + _logger.LogError( + "Required PowerShell module could not be loaded (auto-install was attempted but failed). " + + "Run 'a365 setup requirements' to manually install missing modules."); + } + return null; } - var token = result.StandardOutput?.Trim(); + // The script ends with `$token`, which outputs the JWT as the last line. + // Connect-MgGraph may also write informational messages (e.g. device code prompt) + // to stdout in non-interactive environments. Extract only the last non-empty line + // so those messages do not contaminate the token. + var token = result.StandardOutput? + .Split('\n', StringSplitOptions.RemoveEmptyEntries) + .Select(l => l.Trim()) + .LastOrDefault(l => !string.IsNullOrWhiteSpace(l)); if (string.IsNullOrWhiteSpace(token)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 1903877c..913fee89 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -22,11 +22,12 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// Uses Microsoft.Identity.Client.Extensions.Msal to persist tokens across all CLI instances. /// This dramatically reduces authentication prompts during multi-step operations like 'a365 setup all'. /// -/// Cache Location: %LocalApplicationData%\Agent365\msal-token-cache (Windows) -/// Security: Tokens are encrypted at rest using platform-appropriate mechanisms: -/// - Windows: DPAPI (Data Protection API) - tokens encrypted with user credentials -/// - macOS: Keychain - tokens stored in secure keychain -/// - Linux: Persistent caching is disabled (no platform encryption available); tokens remain in-memory only +/// Cache Location: [LocalApplicationData]/Agent365/msal-token-cache (Windows/macOS) +/// Security: Tokens are stored using platform-appropriate mechanisms: +/// - Windows: DPAPI (Data Protection API) - tokens encrypted with user credentials, persisted to disk +/// - macOS: Keychain - tokens stored in secure keychain, persisted to disk +/// - Linux: Shared in-memory cache (static, in-process) - tokens never written to disk, shared +/// across all MsalBrowserCredential instances in the same CLI process to avoid repeated prompts /// /// See: https://learn.microsoft.com/en-us/entra/msal/dotnet/acquiring-tokens/desktop-mobile/wam /// Enhancement: Improves the WAM authentication experience by reducing repeated login prompts. @@ -43,6 +44,13 @@ public sealed class MsalBrowserCredential : TokenCredential // This is the key to reducing multiple WAM prompts during setup operations. private static MsalCacheHelper? _cacheHelper; private static readonly object _cacheHelperLock = new(); + + // Linux-only: shared in-memory token cache, serialized as MSAL V3 format. + // Shared across all MsalBrowserCredential instances in the same CLI process so that + // a second auth call (e.g., client secret creation) can reuse the token from the first + // interactive sign-in without triggering another device code prompt. + private static byte[] _linuxInMemoryCacheBytes = Array.Empty(); + private static readonly object _linuxCacheLock = new(); private static readonly string CacheFileName = "msal-token-cache"; private static readonly string CacheDirectory = Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @@ -146,25 +154,27 @@ public MsalBrowserCredential( } /// - /// Registers a persistent cross-process token cache with the MSAL application. - /// The cache is shared across all instances of MsalBrowserCredential within this CLI process - /// and persists to disk for reuse across CLI invocations. + /// Registers a shared token cache with the MSAL application. + /// The cache is shared across all MsalBrowserCredential instances within this CLI process. /// - /// Security: Uses platform-appropriate encryption (DPAPI on Windows, Keychain on macOS). - /// On Linux, persistent caching is skipped to avoid storing tokens in plaintext on disk. + /// Security: Uses platform-appropriate storage: + /// - Windows: DPAPI-encrypted file, persisted across CLI invocations + /// - macOS: Keychain-backed file, persisted across CLI invocations + /// - Linux: In-memory only (shared in-process via static bytes), not persisted to disk /// private static void RegisterPersistentCache(IPublicClientApplication app, ILogger? logger) { try { - // Skip persistent caching on Linux to avoid storing tokens in plaintext on disk. - // Linux lacks a platform-provided encryption mechanism (libsecret/Keyring requires - // additional setup that cannot be guaranteed). Users on Linux will see repeated - // authentication prompts but their tokens remain safely in-memory only. + // Linux: no secure file storage available, but share tokens across all instances + // within this CLI process using a static in-memory serialized cache. + // This eliminates repeated device code prompts during multi-step operations + // (e.g., blueprint creation followed by client secret creation in 'setup blueprint'). + // Tokens are never written to disk on Linux. if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - logger?.LogDebug("Skipping persistent token cache on Linux - no platform encryption available. Tokens will be in-memory only."); + RegisterSharedInMemoryCache(app, logger); return; } @@ -226,6 +236,36 @@ private static void RegisterPersistentCache(IPublicClientApplication app, ILogge } } + /// + /// Registers a shared in-memory token cache for Linux platforms. + /// Tokens are not persisted to disk, but are shared across all MsalBrowserCredential + /// instances within the current CLI process, eliminating repeated auth prompts within + /// a single command invocation (e.g., blueprint creation + client secret creation). + /// + private static void RegisterSharedInMemoryCache(IPublicClientApplication app, ILogger? logger) + { + app.UserTokenCache.SetBeforeAccess(args => + { + lock (_linuxCacheLock) + { + args.TokenCache.DeserializeMsalV3(_linuxInMemoryCacheBytes); + } + }); + + app.UserTokenCache.SetAfterAccess(args => + { + if (args.HasStateChanged) + { + lock (_linuxCacheLock) + { + _linuxInMemoryCacheBytes = args.TokenCache.SerializeMsalV3(); + } + } + }); + + logger?.LogDebug("Registered shared in-memory token cache for Linux (in-process only, not persisted to disk)."); + } + /// public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) { @@ -315,8 +355,15 @@ public override async ValueTask GetTokenAsync( } catch (PlatformNotSupportedException ex) { + // macOS: MSAL throws PlatformNotSupportedException when no browser is available _logger?.LogWarning("Browser authentication is not supported on this platform: {Message}", ex.Message); - throw new MsalAuthenticationFailedException($"Browser authentication is not supported on this platform ({ex.Message})", ex); + return await AcquireTokenWithDeviceCodeFallbackAsync(scopes, cancellationToken); + } + catch (MsalClientException ex) when (ex.ErrorCode == "linux_xdg_open_failed") + { + // Linux/WSL: MSAL throws MsalClientException when xdg-open and friends are unavailable + _logger?.LogWarning("Browser cannot be opened on this platform: {Message}", ex.Message); + return await AcquireTokenWithDeviceCodeFallbackAsync(scopes, cancellationToken); } catch (MsalException ex) { @@ -324,6 +371,86 @@ public override async ValueTask GetTokenAsync( throw new MsalAuthenticationFailedException($"Failed to acquire token: {ex.Message}", ex); } } + + private async Task AcquireTokenWithDeviceCodeFallbackAsync( + string[] scopes, + CancellationToken cancellationToken) + { + // Before showing a device code, try to get a cached token. + // On Linux, the shared in-process cache may already hold a token from an earlier + // authentication step in the same CLI invocation (e.g., blueprint creation), + // which can be reused silently without prompting the user again. + var accountsList = (await _publicClientApp.GetAccountsAsync()).ToList(); + // Filter by tenant to avoid silently authenticating as the wrong identity when multiple accounts are cached. + // If multiple accounts share the same tenant (rare), FirstOrDefault picks the first match; this is acceptable + // since MSAL will re-prompt if the silent acquisition fails for the wrong account. + var cachedAccount = accountsList.Count switch + { + 0 => null, + 1 => accountsList[0], + _ => accountsList.FirstOrDefault(a => + string.Equals(a.HomeAccountId?.TenantId, _tenantId, StringComparison.OrdinalIgnoreCase)) + }; + if (cachedAccount != null) + { + try + { + _logger?.LogDebug("Attempting silent token acquisition before device code..."); + var silentResult = await _publicClientApp + .AcquireTokenSilent(scopes, cachedAccount) + .ExecuteAsync(cancellationToken); + _logger?.LogDebug("Acquired token silently, skipping device code prompt."); + return new AccessToken(silentResult.AccessToken, silentResult.ExpiresOn); + } + catch (MsalUiRequiredException) + { + _logger?.LogDebug("Silent acquisition failed, proceeding with device code."); + } + } + + _logger?.LogInformation("Falling back to device code authentication..."); + _logger?.LogInformation("Please sign in with your Microsoft account"); + + try + { + var deviceCodeResult = await _publicClientApp + .AcquireTokenWithDeviceCode(scopes, deviceCode => + { + _logger?.LogInformation(""); + _logger?.LogInformation("=========================================================================="); + _logger?.LogInformation("To sign in, use a web browser to open the page:"); + _logger?.LogInformation(" {VerificationUrl}", deviceCode.VerificationUrl); + _logger?.LogInformation(""); + _logger?.LogInformation("And enter the code: {UserCode}", deviceCode.UserCode); + _logger?.LogInformation("=========================================================================="); + _logger?.LogInformation(""); + return Task.CompletedTask; + }) + .ExecuteAsync(cancellationToken); + + _logger?.LogDebug("Successfully acquired token via device code authentication."); + return new AccessToken(deviceCodeResult.AccessToken, deviceCodeResult.ExpiresOn); + } + catch (MsalException msalEx) when ( + msalEx.Message.Contains("AADSTS7000218", StringComparison.Ordinal) || + (msalEx is MsalServiceException svcEx && svcEx.ErrorCode == "invalid_client" && + msalEx.Message.Contains("client_assertion", StringComparison.Ordinal))) + { + // Do NOT pass msalEx as logger argument — avoids printing the full stack trace. + // This error means "Allow public client flows" is disabled on the app registration. + _logger?.LogError("Device code authentication failed: 'Allow public client flows' is not enabled on the app registration."); + _logger?.LogError("Run 'a365 setup requirements' to detect and auto-fix this automatically."); + _logger?.LogError("Or fix manually: Azure Portal > App registrations > Authentication > Settings > Enable 'Allow public client flows' > Save."); + throw new MsalAuthenticationFailedException( + "Device code authentication requires 'Allow public client flows' to be enabled. Run 'a365 setup requirements' to auto-fix, or enable it manually in Azure Portal > App registrations > Authentication.", + msalEx); + } + catch (MsalException msalEx) + { + _logger?.LogError(msalEx, "Device code authentication failed: {Message}", msalEx.Message); + throw new MsalAuthenticationFailedException($"Device code authentication failed: {msalEx.Message}", msalEx); + } + } } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs index caf02564..85871072 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs @@ -87,7 +87,7 @@ private async Task CheckImplementationAsync(Agent365Conf } } - // Return results + // All modules present - done if (missingModules.Count == 0) { return RequirementCheckResult.Success( @@ -95,15 +95,60 @@ private async Task CheckImplementationAsync(Agent365Conf ); } - var missingModuleNames = string.Join(", ", missingModules.Select(m => m.Name)); - var installCommands = GenerateInstallationInstructions(missingModules); + // Attempt auto-install for missing modules + logger.LogInformation("Attempting to auto-install missing PowerShell modules..."); + var autoInstalled = new List(); + var stillMissing = new List(); + + foreach (var module in missingModules) + { + logger.LogInformation("Installing {ModuleName}...", module.Name); + var installSuccess = await InstallModuleAsync(module.Name, logger, cancellationToken); + + if (installSuccess) + { + var verified = await CheckModuleInstalledAsync(module.Name, logger, cancellationToken); + if (verified) + { + autoInstalled.Add(module); + logger.LogInformation("Successfully installed {ModuleName}", module.Name); + } + else + { + stillMissing.Add(module); + logger.LogWarning("Install succeeded but {ModuleName} not found in module path after install", module.Name); + } + } + else + { + stillMissing.Add(module); + } + } + + if (stillMissing.Count == 0) + { + var autoInstalledNames = string.Join(", ", autoInstalled.Select(m => m.Name)); + var alreadyInstalled = installedModules.Count > 0 + ? $" Previously installed: {string.Join(", ", installedModules.Select(m => m.Name))}." + : string.Empty; + return RequirementCheckResult.Success( + details: $"Auto-installed missing modules: {autoInstalledNames}.{alreadyInstalled}" + ); + } + + // Some modules could not be auto-installed + var stillMissingNames = string.Join(", ", stillMissing.Select(m => m.Name)); + var installCommands = GenerateInstallationInstructions(stillMissing); + var autoInstalledNote = autoInstalled.Count > 0 + ? $"Auto-installed: {string.Join(", ", autoInstalled.Select(m => m.Name))}. " + : string.Empty; return RequirementCheckResult.Failure( - errorMessage: $"Missing required PowerShell modules: {missingModuleNames}", - resolutionGuidance: installCommands, - details: $"These modules are required for Microsoft Graph operations, app registration, and Azure authentication. " + - $"Missing: {missingModuleNames}. " + - $"Installed: {string.Join(", ", installedModules.Select(m => m.Name))}" + errorMessage: $"Missing required PowerShell modules (auto-install failed): {stillMissingNames}", + resolutionGuidance: $"{autoInstalledNote}{installCommands}", + details: $"These modules are required for Microsoft Graph operations. " + + $"Auto-install was attempted but failed for: {stillMissingNames}. " + + $"Installed: {string.Join(", ", installedModules.Concat(autoInstalled).Select(m => m.Name))}" ); } @@ -179,6 +224,29 @@ private async Task CheckModuleInstalledAsync(string moduleName, ILogger lo } } + /// + /// Attempts to install a PowerShell module using Install-Module with CurrentUser scope. + /// Uses -Force and -AllowClobber to handle conflicts without requiring elevation. + /// + private async Task InstallModuleAsync(string moduleName, ILogger logger, CancellationToken cancellationToken) + { + try + { + var command = $"Install-Module -Name '{moduleName}' -Repository 'PSGallery' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop"; + var result = await ExecutePowerShellCommandAsync("pwsh", command, logger, cancellationToken); + if (!result.success) + { + logger.LogDebug("Auto-install failed for {ModuleName}: {Output}", moduleName, result.output); + } + return result.success; + } + catch (Exception ex) + { + logger.LogDebug("Auto-install threw exception for {ModuleName}: {Error}", moduleName, ex.Message); + return false; + } + } + /// /// Execute a PowerShell command and return the result /// @@ -218,7 +286,7 @@ private async Task CheckModuleInstalledAsync(string moduleName, ILogger lo } logger.LogDebug("PowerShell command failed: {Error}", error); - return (false, null); + return (false, error); } catch (Exception ex) { @@ -239,9 +307,9 @@ private static string GenerateInstallationInstructions(List miss "Method 1: Install all required modules at once" }; - // PowerShell 7+ command - var moduleNames = string.Join(",", missingModules.Select(m => $"'{m.Name}'")); - instructions.Add($" pwsh -Command \"Install-Module -Name '{moduleNames}' -Scope CurrentUser -Force\""); + // PowerShell 7+ command — pass quoted module names directly as an array literal (no outer quotes) + var moduleNames = string.Join(", ", missingModules.Select(m => $"'{m.Name}'")); + instructions.Add($" pwsh -Command \"Install-Module -Name {moduleNames} -Scope CurrentUser -Force\""); instructions.Add(""); // Individual module instructions diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs index f45913e3..49a40c17 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -151,6 +151,26 @@ public void CreateCommand_ShouldHaveDryRunOption() dryRunOption!.Aliases.Should().Contain("--dry-run"); } + [Fact] + public void CreateCommand_ShouldHaveSkipRequirementsOption() + { + // Act + var command = BlueprintSubcommand.CreateCommand( + _mockLogger, + _mockConfigService, + _mockExecutor, + _mockAzureValidator, + _mockWebAppCreator, + _mockPlatformDetector, + _mockBotConfigurator, + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); + + // Assert + var skipRequirementsOption = command.Options.FirstOrDefault(o => o.Name == "skip-requirements"); + skipRequirementsOption.Should().NotBeNull(); + skipRequirementsOption!.Aliases.Should().Contain("--skip-requirements"); + } + [Fact] public async Task DryRun_ShouldLoadConfigAndNotExecute() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs index 4fa222c3..70843db5 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs @@ -3,6 +3,7 @@ using System.Text.Json; using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Xunit; @@ -43,7 +44,7 @@ public async Task LoadAsync_ThrowsFileNotFoundException_WhenConfigFileDoesNotExi var configPath = Path.Combine(_testDirectory, "nonexistent.json"); // Act & Assert - await Assert.ThrowsAsync( + await Assert.ThrowsAsync( () => _service.LoadAsync(configPath)); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs index efd9953c..84ccb243 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs @@ -165,24 +165,14 @@ public async Task GetServicePrincipalByAppIdAsync_WhenSPExists_ReturnsFoundWithD { // Arrange var spObjectId = "22222222-2222-2222-2222-222222222222"; - var jsonResponse = $@"{{ - ""value"": [ - {{ - ""id"": ""{spObjectId}"", - ""appId"": ""{TestAppId}"", - ""displayName"": ""Test SP"" - }} - ] - }}"; - var jsonDoc = JsonDocument.Parse(jsonResponse); - // The filter will be URL-escaped: "appId eq '...'" becomes "appId%20eq%20%27...%27" - _graphApiService.GraphGetAsync( + // LookupServicePrincipalByAppIdAsync handles ConsistencyLevel header internally + _graphApiService.LookupServicePrincipalByAppIdAsync( TestTenantId, - Arg.Is(s => s.Contains($"appId%20eq%20%27{TestAppId}%27")), + TestAppId, Arg.Any(), - null) - .Returns(jsonDoc); + Arg.Any?>()) + .Returns(spObjectId); // Act var result = await _service.GetServicePrincipalByAppIdAsync(TestTenantId, TestAppId); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs index 7db998b8..20a4f51c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs @@ -508,6 +508,94 @@ private void SetupGraphPermissionResolution(string token) #endregion + #region EnsurePublicClientFlowsEnabledAsync Tests + + [Fact] + public async Task EnsureValidClientAppAsync_WhenPublicClientFlowsAlreadyEnabled_DoesNotPatchPublicClientFlows() + { + // Arrange + var token = "fake-token-123"; + SetupTokenAcquisition(token); + SetupAppExistsWithAllPermissions(ValidClientAppId, "Test App"); + SetupAdminConsentGranted(ValidClientAppId); + SetupPublicClientFlowsCheck(enabled: true); + + // Act + await _validator.EnsureValidClientAppAsync(ValidClientAppId, ValidTenantId); + + // Assert - PATCH for isFallbackPublicClient should NOT be called + await _executor.DidNotReceive().ExecuteAsync( + Arg.Is(s => s == "az"), + Arg.Is(s => s.Contains("rest --method PATCH") && s.Contains("isFallbackPublicClient")), + cancellationToken: Arg.Any()); + } + + [Fact] + public async Task EnsureValidClientAppAsync_WhenPublicClientFlowsDisabled_PatchesPublicClientFlows() + { + // Arrange + var token = "fake-token-123"; + SetupTokenAcquisition(token); + SetupAppExistsWithAllPermissions(ValidClientAppId, "Test App"); + SetupAdminConsentGranted(ValidClientAppId); + SetupPublicClientFlowsCheck(enabled: false); + + _executor.ExecuteAsync( + Arg.Is(s => s == "az"), + Arg.Is(s => s.Contains("rest --method PATCH") && s.Contains("isFallbackPublicClient")), + cancellationToken: Arg.Any()) + .Returns(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty }); + + // Act - should not throw (non-fatal) + await _validator.EnsureValidClientAppAsync(ValidClientAppId, ValidTenantId); + + // Assert - PATCH for isFallbackPublicClient should be called once + await _executor.Received(1).ExecuteAsync( + Arg.Is(s => s == "az"), + Arg.Is(s => s.Contains("rest --method PATCH") && s.Contains("isFallbackPublicClient")), + cancellationToken: Arg.Any()); + } + + [Fact] + public async Task EnsureValidClientAppAsync_WhenPublicClientFlowsPatchFails_DoesNotThrow() + { + // Arrange + var token = "fake-token-123"; + SetupTokenAcquisition(token); + SetupAppExistsWithAllPermissions(ValidClientAppId, "Test App"); + SetupAdminConsentGranted(ValidClientAppId); + SetupPublicClientFlowsCheck(enabled: false); + + _executor.ExecuteAsync( + Arg.Is(s => s == "az"), + Arg.Is(s => s.Contains("rest --method PATCH") && s.Contains("isFallbackPublicClient")), + cancellationToken: Arg.Any()) + .Returns(new CommandResult { ExitCode = 1, StandardOutput = string.Empty, StandardError = "Forbidden" }); + + // Act - should not throw (non-fatal operation) + await _validator.EnsureValidClientAppAsync(ValidClientAppId, ValidTenantId); + } + + private void SetupPublicClientFlowsCheck(bool enabled) + { + var appJson = $$""" + { + "value": [{ + "id": "object-id-123", + "isFallbackPublicClient": {{(enabled ? "true" : "false")}} + }] + } + """; + + _executor.ExecuteAsync( + Arg.Is(s => s == "az"), + Arg.Is(s => s.Contains("isFallbackPublicClient")), + cancellationToken: Arg.Any()) + .Returns(new CommandResult { ExitCode = 0, StandardOutput = appJson, StandardError = string.Empty }); + } + + #endregion + #region EnsureRedirectUrisAsync Tests [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs index bcd914eb..c9be1f8d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs @@ -112,18 +112,9 @@ public async Task GraphPostWithResponseAsync_Returns_Failure_With_Body() public async Task LookupServicePrincipalAsync_DoesNotIncludeConsistencyLevelHeader() { // This test verifies that the ConsistencyLevel header is NOT sent during service principal lookup. - // The ConsistencyLevel: eventual header is only required for advanced Graph queries and causes - // HTTP 400 "One or more headers are invalid" errors for simple $filter queries. + // Per Graph docs, servicePrincipal $filter=appId eq is "Default+Advanced" — it works without + // ConsistencyLevel. Adding it caused HTTP 400 "One or more headers are invalid" in some scenarios. // Regression test for issue discovered on 2025-12-19. - // - // NOTE: This test covers BOTH bug locations: - // 1. ExecutePublishGraphStepsAsync (line 211) - where header was incorrectly set after token acquisition - // 2. EnsureGraphHeadersAsync (lines 745-746) - where header was incorrectly set before all Graph API calls - // - // The bug in ExecutePublishGraphStepsAsync was "defensive" - it set the header on the HttpClient, but - // EnsureGraphHeadersAsync would have overwritten it anyway. By testing EnsureGraphHeadersAsync (which is - // called by ALL Graph API operations), we ensure the header is never sent regardless of whether - // ExecutePublishGraphStepsAsync tries to set it. // Arrange HttpRequestMessage? capturedRequest = null; @@ -190,7 +181,7 @@ public async Task LookupServicePrincipalAsync_DoesNotIncludeConsistencyLevelHead // Verify the ConsistencyLevel header is NOT present on the service principal lookup request capturedRequest.Headers.Contains("ConsistencyLevel").Should().BeFalse( "ConsistencyLevel header should NOT be present for simple service principal lookup queries. " + - "This header is only needed for advanced Graph query capabilities and causes HTTP 400 errors otherwise."); + "Per Graph docs, appId eq is Default+Advanced and does not require this header."); } [Theory] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/InteractiveGraphAuthServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/InteractiveGraphAuthServiceTests.cs index 37fa6c3a..6c8a1dce 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/InteractiveGraphAuthServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/InteractiveGraphAuthServiceTests.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Azure.Core; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; using NSubstitute; @@ -184,4 +187,149 @@ public void MsalBrowserCredential_ManualTest_ShouldUseWAMOnWindows() } #endregion + + #region GetAuthenticatedGraphClientAsync Tests + + // These helpers mirror the pattern from AuthenticationServiceTests to enable injecting + // fakes via the credentialFactory constructor parameter without touching the file system + // or launching any real authentication UI. + + private sealed class ThrowingTokenCredential : TokenCredential + { + private readonly Exception _exception; + public ThrowingTokenCredential(Exception exception) => _exception = exception; + public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) => throw _exception; + public override ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) => throw _exception; + } + + private sealed class StubTokenCredential : TokenCredential + { + private readonly AccessToken _token; + public StubTokenCredential(string token, DateTimeOffset expiresOn) => _token = new AccessToken(token, expiresOn); + public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) => _token; + public override ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) => new(_token); + } + + private const string ValidGuid = "12345678-1234-1234-1234-123456789abc"; + private const string ValidTenantId = "87654321-4321-4321-4321-cba987654321"; + + /// + /// Verifies that a credential failure surfaced during eager token acquisition + /// throws rather than silently returning a broken client. + /// + /// Pre-fix: GetAuthenticatedGraphClientAsync returned "success" without ever calling + /// GetTokenAsync because GraphServiceClient acquires tokens lazily. This masked broken + /// credentials at construction time (Gap 2 from the macOS auth regression). + /// + /// Post-fix: Eager token acquisition detects the failure early and surfaces a clear error. + /// + [Fact] + public async Task GetAuthenticatedGraphClientAsync_WhenCredentialFails_ThrowsGraphApiException() + { + // Arrange — credential that always fails (simulates a broken/unsupported auth flow) + var failingCredential = new ThrowingTokenCredential( + new MsalAuthenticationFailedException("Authentication failed")); + + var logger = Substitute.For>(); + var sut = new InteractiveGraphAuthService(logger, ValidGuid, + credentialFactory: (_, _) => failingCredential); + + // Act + var act = async () => await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + + // Assert — GraphApiException surfaced at construction time, not later during API calls + await act.Should().ThrowAsync(); + } + + /// + /// Verifies that when the credential succeeds, a GraphServiceClient is returned + /// and the "Successfully authenticated" log is only emitted after actual token acquisition. + /// + [Fact] + public async Task GetAuthenticatedGraphClientAsync_WhenCredentialSucceeds_ReturnsClient() + { + // Arrange + var workingCredential = new StubTokenCredential("token-value", DateTimeOffset.UtcNow.AddHours(1)); + var logger = Substitute.For>(); + var sut = new InteractiveGraphAuthService(logger, ValidGuid, + credentialFactory: (_, _) => workingCredential); + + // Act + var client = await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + + // Assert + client.Should().NotBeNull(); + } + + /// + /// Verifies that the service returns the same cached GraphServiceClient for the same tenant + /// on repeated calls, avoiding redundant authentication prompts. + /// + [Fact] + public async Task GetAuthenticatedGraphClientAsync_ForSameTenant_ReturnsCachedClient() + { + // Arrange + var workingCredential = new StubTokenCredential("token-value", DateTimeOffset.UtcNow.AddHours(1)); + var logger = Substitute.For>(); + int callCount = 0; + var sut = new InteractiveGraphAuthService(logger, ValidGuid, + credentialFactory: (_, _) => { callCount++; return workingCredential; }); + + // Act — call twice for the same tenant + var client1 = await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + var client2 = await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + + // Assert — factory called only once; same instance returned on the second call + callCount.Should().Be(1); + client1.Should().BeSameAs(client2); + } + + /// + /// Verifies that the service authenticates again when a different tenant is requested, + /// rather than returning a cached client scoped to the wrong tenant. + /// + [Fact] + public async Task GetAuthenticatedGraphClientAsync_ForDifferentTenant_AuthenticatesSeparately() + { + // Arrange + var workingCredential = new StubTokenCredential("token-value", DateTimeOffset.UtcNow.AddHours(1)); + var logger = Substitute.For>(); + int callCount = 0; + var sut = new InteractiveGraphAuthService(logger, ValidGuid, + credentialFactory: (_, _) => { callCount++; return workingCredential; }); + + const string otherTenant = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + + // Act + var client1 = await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + var client2 = await sut.GetAuthenticatedGraphClientAsync(otherTenant); + + // Assert — factory called twice (once per tenant) + callCount.Should().Be(2); + client1.Should().NotBeSameAs(client2); + } + + /// + /// Verifies that an access_denied error from the auth provider surfaces as + /// GraphApiException rather than a raw MsalServiceException. + /// + [Fact] + public async Task GetAuthenticatedGraphClientAsync_WhenAccessDenied_ThrowsGraphApiException() + { + // Arrange — simulate user cancelling or denying the auth prompt + var accessDenied = new Microsoft.Identity.Client.MsalServiceException("access_denied", "User cancelled"); + var failingCredential = new ThrowingTokenCredential(accessDenied); + + var logger = Substitute.For>(); + var sut = new InteractiveGraphAuthService(logger, ValidGuid, + credentialFactory: (_, _) => failingCredential); + + // Act + var act = async () => await sut.GetAuthenticatedGraphClientAsync(ValidTenantId); + + // Assert + await act.Should().ThrowAsync(); + } + + #endregion } \ No newline at end of file diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs index 6952a08a..17200c86 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs @@ -35,6 +35,7 @@ public async Task GetMgGraphAccessTokenAsync_WithValidClientAppId_IncludesClient Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -51,6 +52,7 @@ await _executor.Received(1).ExecuteWithStreamingAsync( Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()); } @@ -68,6 +70,7 @@ public async Task GetMgGraphAccessTokenAsync_WithoutClientAppId_OmitsClientIdPar Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -84,6 +87,7 @@ await _executor.Received(1).ExecuteWithStreamingAsync( Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()); } @@ -158,6 +162,7 @@ public async Task GetMgGraphAccessTokenAsync_WhenExecutionFails_ReturnsNull() Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) .Returns(new CommandResult { ExitCode = 1, StandardOutput = string.Empty, StandardError = "PowerShell error" }); @@ -184,6 +189,7 @@ public async Task GetMgGraphAccessTokenAsync_WithValidToken_ReturnsToken() Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -229,6 +235,7 @@ public async Task GetMgGraphAccessTokenAsync_EscapesSingleQuotesInClientAppId() Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty });