From cbdfdba0937a6edccae4a0eaec062a1f885f0d21 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 11:37:33 -0800 Subject: [PATCH 01/18] fix: device code fallback for macOS browser auth and cross-platform URL opening - MsalBrowserCredential: catch PlatformNotSupportedException and fall back to device code flow instead of re-throwing, so macOS/Linux headless environments always get a working auth path - InteractiveGraphAuthService: eagerly acquire token before constructing GraphServiceClient to surface auth failures at construction time rather than silently returning a broken client (Gap 2 from PR #290) - InteractiveGraphAuthService: inject optional credentialFactory for testability - AuthenticationService: replace Console.WriteLine with structured logger calls in device code callback - Add BrowserHelper with cross-platform TryOpenUrl (Windows/macOS/Linux) - Remove duplicated TryOpenBrowser methods from BlueprintSubcommand and A365CreateInstanceRunner; both now call BrowserHelper.TryOpenUrl directly - Add 5 unit tests covering the eager-auth gap and credential caching behavior --- .../SetupSubcommands/BlueprintSubcommand.cs | 19 +-- .../Helpers/BrowserHelper.cs | 47 ++++++ .../Services/A365CreateInstanceRunner.cs | 22 +-- .../Services/AuthenticationService.cs | 16 +- .../Services/InteractiveGraphAuthService.cs | 66 ++++---- .../Services/MsalBrowserCredential.cs | 29 +++- .../InteractiveGraphAuthServiceTests.cs | 148 ++++++++++++++++++ 7 files changed, 270 insertions(+), 77 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs 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..3136eb99 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -1428,7 +1428,7 @@ 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); + BrowserHelper.TryOpenUrl(consentUrlGraph); var consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct); @@ -1541,23 +1541,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/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs new file mode 100644 index 00000000..993bf6f9 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -0,0 +1,47 @@ +// 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: logs a warning and prints the URL if the browser cannot be launched. + /// + /// 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", Arguments = url }; + } + else + { + psi = new ProcessStartInfo { FileName = "xdg-open", Arguments = url }; + } + using var process = new Process { StartInfo = psi }; + process.Start(); + } + catch (Exception ex) + { + logger?.LogWarning(ex, "Failed to open browser automatically"); + logger?.LogInformation("Please manually open: {Url}", 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/AuthenticationService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs index 76ce488b..d05021f0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs @@ -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/InteractiveGraphAuthService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs index 38c9d62b..55ffd986 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs @@ -26,6 +26,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 +41,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 +61,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 +80,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,37 +90,25 @@ 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) + // Resolve credential: use injected factory (for tests) or default MsalBrowserCredential + var credential = _credentialFactory?.Invoke(_clientAppId, tenantId) + ?? new MsalBrowserCredential(_clientAppId, tenantId, redirectUri: null, _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(""); + + // Eagerly acquire a token so authentication failures are detected here rather than + // surfacing later from inside GraphServiceClient's lazy token acquisition. + var tokenContext = new TokenRequestContext(RequiredScopes); 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); + await credential.GetTokenAsync(tokenContext, cancellationToken); } catch (MsalAuthenticationFailedException ex) when (ex.Message.Contains("invalid_grant")) { - // Permissions issue ThrowInsufficientPermissionsException(ex); throw; // Unreachable but required for compiler } @@ -122,7 +117,6 @@ public Task GetAuthenticatedGraphClientAsync( ex.Message.Contains("connection") || ex.Message.Contains("redirect_uri")) { - // Infrastructure/connectivity issue _logger.LogError("Browser authentication failed due to connectivity issue: {Message}", ex.Message); throw new GraphApiException( "Browser authentication", @@ -145,6 +139,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/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 1903877c..e1790666 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -316,7 +316,34 @@ public override async ValueTask GetTokenAsync( catch (PlatformNotSupportedException ex) { _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); + _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) + { + _logger?.LogError(msalEx, "Device code authentication failed: {Message}", msalEx.Message); + throw new MsalAuthenticationFailedException($"Device code authentication failed: {msalEx.Message}", msalEx); + } } catch (MsalException ex) { 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 From 6c8fe1da8aefa894a2d86d1557f7645845f2d3aa Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 12:00:10 -0800 Subject: [PATCH 02/18] fix: address PR review comments and handle Linux xdg_open_failed error - MsalBrowserCredential: also catch MsalClientException(linux_xdg_open_failed) for device code fallback on Linux/WSL (confirmed via WSL Ubuntu 24.04 repro); extract shared AcquireTokenWithDeviceCodeFallbackAsync to avoid duplication - BrowserHelper: fall back to Console.Error when logger is null so the manual URL is never silently lost regardless of caller - BlueprintSubcommand: log consent URL before TryOpenUrl and pass logger so users on headless platforms always see the URL to complete admin consent Co-Authored-By: Claude Sonnet 4.6 --- .../SetupSubcommands/BlueprintSubcommand.cs | 3 +- .../Helpers/BrowserHelper.cs | 12 +++- .../Services/MsalBrowserCredential.cs | 70 +++++++++++-------- 3 files changed, 54 insertions(+), 31 deletions(-) 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 3136eb99..dd2ad9bb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -1428,7 +1428,8 @@ 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..."); - BrowserHelper.TryOpenUrl(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); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs index 993bf6f9..444f23cc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -40,8 +40,16 @@ public static void TryOpenUrl(string url, ILogger? logger = null) } catch (Exception ex) { - logger?.LogWarning(ex, "Failed to open browser automatically"); - logger?.LogInformation("Please manually open: {Url}", url); + if (logger != null) + { + logger.LogWarning(ex, "Failed to open browser automatically"); + 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/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index e1790666..fdda82cf 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -315,35 +315,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); - _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) - { - _logger?.LogError(msalEx, "Device code authentication failed: {Message}", msalEx.Message); - throw new MsalAuthenticationFailedException($"Device code authentication failed: {msalEx.Message}", msalEx); - } + 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) { @@ -351,6 +331,40 @@ public override async ValueTask GetTokenAsync( throw new MsalAuthenticationFailedException($"Failed to acquire token: {ex.Message}", ex); } } + + private async Task AcquireTokenWithDeviceCodeFallbackAsync( + string[] scopes, + CancellationToken cancellationToken) + { + _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) + { + _logger?.LogError(msalEx, "Device code authentication failed: {Message}", msalEx.Message); + throw new MsalAuthenticationFailedException($"Device code authentication failed: {msalEx.Message}", msalEx); + } + } } /// From 4da9b55e781bd224704f07a03fd13f2de5d5ac83 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 12:36:11 -0800 Subject: [PATCH 03/18] fix: auto-fix public client flows, add requirements check to blueprint, clean AADSTS7000218 error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ClientAppValidator: add EnsurePublicClientFlowsEnabledAsync (Step 7) that auto-detects and enables 'Allow public client flows' on the app registration via az rest GET/PATCH — required for MSAL device code fallback on macOS/Linux; non-fatal if PATCH fails - BlueprintSubcommand: run config requirements checks (LocationCheck + ClientAppCheck) before blueprint logic, with --skip-requirements opt-out; matches AllSubcommand pattern - MsalBrowserCredential: catch AADSTS7000218/invalid_client before general MsalException to emit clean actionable error message pointing to 'a365 setup requirements' instead of stack trace - Tests: add EnsurePublicClientFlowsEnabledAsync tests (enabled/disabled/patch-fails) and CreateCommand_ShouldHaveSkipRequirementsOption test Co-Authored-By: Claude Sonnet 4.6 --- .../SetupSubcommands/BlueprintSubcommand.cs | 38 +++++++- .../Services/ClientAppValidator.cs | 79 +++++++++++++++++ .../Services/MsalBrowserCredential.cs | 14 +++ .../Commands/BlueprintSubcommandTests.cs | 20 +++++ .../Services/ClientAppValidatorTests.cs | 88 +++++++++++++++++++ 5 files changed, 237 insertions(+), 2 deletions(-) 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 dd2ad9bb..d5e2820b 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,34 @@ await UpdateEndpointAsync( return; } + // Run config-dependent requirements checks (includes ClientAppRequirementCheck which + // auto-fixes isFallbackPublicClient required for device code auth on macOS/Linux/WSL). + if (!skipRequirements) + { + try + { + var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( + RequirementsSubcommand.GetConfigRequirementChecks(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 +315,7 @@ await CreateBlueprintImplementationAsync( correlationId: correlationId ); - }, configOption, verboseOption, dryRunOption, skipEndpointRegistrationOption, endpointOnlyOption, updateEndpointOption); + }, configOption, verboseOption, dryRunOption, skipEndpointRegistrationOption, endpointOnlyOption, updateEndpointOption, skipRequirementsOption); return command; } 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/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index fdda82cf..6a54486e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -359,6 +359,20 @@ private async Task AcquireTokenWithDeviceCodeFallbackAsync( _logger?.LogDebug("Successfully acquired token via device code authentication."); return new AccessToken(deviceCodeResult.AccessToken, deviceCodeResult.ExpiresOn); } + catch (MsalException msalEx) when ( + msalEx.Message.Contains("AADSTS7000218") || + (msalEx is MsalServiceException svcEx && svcEx.ErrorCode == "invalid_client" && + msalEx.Message.Contains("client_assertion"))) + { + // 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); 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 46af84e2..caf355c7 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/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] From 32723761828853957743c52c14557266a0aabdd2 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 13:07:47 -0800 Subject: [PATCH 04/18] fix: eliminate double auth on Linux, improve PS module error handling - MsalBrowserCredential: add shared in-memory MSAL token cache for Linux so all MsalBrowserCredential instances within one CLI invocation share the same token cache, eliminating repeated device code prompts during multi-step operations (e.g. blueprint creation + client secret creation) - BrowserHelper: remove exception object from LogWarning so xdg-open failures no longer emit a full stack trace in the output - MicrosoftGraphTokenProvider: detect missing PowerShell module error and log actionable guidance to run 'a365 setup requirements' - SetupHelpers: replace misleading "please sign in when prompted" message with one that mentions missing PS module as a common cause - PowerShellModulesRequirementCheck: auto-install missing modules via Install-Module -Scope CurrentUser -Force -AllowClobber when detected; returns Success if auto-install succeeds, Failure with manual steps if not Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/SetupSubcommands/SetupHelpers.cs | 4 +- .../Helpers/BrowserHelper.cs | 2 +- .../Internal/MicrosoftGraphTokenProvider.cs | 10 +++ .../Services/MsalBrowserCredential.cs | 70 ++++++++++++---- .../PowerShellModulesRequirementCheck.cs | 84 +++++++++++++++++-- 5 files changed, 145 insertions(+), 25 deletions(-) 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..91c1564a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -244,7 +244,9 @@ 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); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs index 444f23cc..fc85d1b2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -42,7 +42,7 @@ public static void TryOpenUrl(string url, ILogger? logger = null) { if (logger != null) { - logger.LogWarning(ex, "Failed to open browser automatically"); + logger.LogWarning("Failed to open browser automatically: {Message}", ex.Message); logger.LogInformation("Please manually open: {Url}", url); } else 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..e6f9abf1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -280,6 +280,16 @@ private static string BuildPowerShellArguments(string shell, string script) _logger.LogError( "Failed to acquire Microsoft Graph access token. Error: {Error}", result.StandardError); + + var error = result.StandardError ?? string.Empty; + if (error.Contains("module", StringComparison.OrdinalIgnoreCase) && + (error.Contains("was not loaded", StringComparison.OrdinalIgnoreCase) || + error.Contains("not found in any module", StringComparison.OrdinalIgnoreCase))) + { + _logger.LogError( + "Required PowerShell module is not installed. Run 'a365 setup requirements' to install missing modules."); + } + return null; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 6a54486e..3d865f66 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) { 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..c0df2b86 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}' -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 /// From 2eae60e3eba0388f3097e5a8dea549c2e4ce47dc Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 14:13:42 -0800 Subject: [PATCH 05/18] fix: add system requirement checks to all setup commands that use Graph PS auth setup blueprint was running only config checks (Location + ClientApp), missing PowerShellModulesRequirementCheck entirely. setup permissions mcp/bot/custom and setup permissions copilotstudio had no requirement checks at all. All four now gate on GetSystemRequirementChecks() before executing Graph operations, so missing Microsoft.Graph.Authentication is caught and auto-installed upfront rather than failing mid-run. Closes partial work toward #106. --- .../SetupSubcommands/BlueprintSubcommand.cs | 7 ++--- .../CopilotStudioSubcommand.cs | 9 +++++++ .../SetupSubcommands/PermissionsSubcommand.cs | 27 +++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) 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 d5e2820b..cf80b100 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -221,14 +221,15 @@ await UpdateEndpointAsync( return; } - // Run config-dependent requirements checks (includes ClientAppRequirementCheck which - // auto-fixes isFallbackPublicClient required for device code auth on macOS/Linux/WSL). + // 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). if (!skipRequirements) { try { var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetConfigRequirementChecks(clientAppValidator), + RequirementsSubcommand.GetRequirementChecks(clientAppValidator), setupConfig, logger, category: null, 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..9c2ff0a2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs @@ -72,6 +72,15 @@ public static Command CreateCommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations) + 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."); + Environment.Exit(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..88772bbc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -85,6 +85,15 @@ private static Command CreateMcpSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations) + 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."); + Environment.Exit(1); + } + if (dryRun) { // Read scopes from ToolingManifest.json @@ -163,6 +172,15 @@ private static Command CreateBotSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations) + 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."); + Environment.Exit(1); + } + if (dryRun) { logger.LogInformation("DRY RUN: Configure Bot API Permissions"); @@ -237,6 +255,15 @@ private static Command CreateCustomSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } + // Verify system requirements (PowerShell modules are required for Graph operations) + 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."); + Environment.Exit(1); + } + if (dryRun) { logger.LogInformation("DRY RUN: Configure Custom Blueprint Permissions"); From 35bd6a043edecc47066bd7d284877be1c4ea6fd4 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 14:19:17 -0800 Subject: [PATCH 06/18] Skip requirements on dry run; clarify browser fallback Requirements checks are now skipped during dry runs to prevent unintended mutations. Updated `TryOpenUrl` documentation to clarify fallback behavior and logging when browser launch fails. --- .../Commands/SetupSubcommands/BlueprintSubcommand.cs | 4 +++- .../Helpers/BrowserHelper.cs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) 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 cf80b100..5b919a8d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -224,7 +224,9 @@ await UpdateEndpointAsync( // 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). - if (!skipRequirements) + // Skip when dryRun is true: ClientAppRequirementCheck can mutate the app registration + // (e.g., set isFallbackPublicClient), which violates dry-run semantics. + if (!skipRequirements && !dryRun) { try { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs index fc85d1b2..3f9c29cb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -14,7 +14,9 @@ public static class BrowserHelper { /// /// Opens a URL in the system's default browser in a cross-platform way. - /// Non-fatal: logs a warning and prints the URL if the browser cannot be launched. + /// 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. From f10f63648fce1fb6561c177d73c40473dd8c89f3 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 14:29:34 -0800 Subject: [PATCH 07/18] fix: skip requirements on dry run, self-correct missing PS modules in Graph token acquisition - Skip system requirement checks (including auto-install) when --dry-run is set in PermissionsSubcommand (mcp/bot/custom) and CopilotStudioSubcommand to preserve non-mutating dry-run semantics - MicrosoftGraphTokenProvider now detects a missing/broken Microsoft.Graph module at runtime, auto-installs both required Graph modules, and retries token acquisition once before failing; error message clarifies auto-install was attempted if retry also fails - PowerShellModulesRequirementCheck.ExecutePowerShellCommandAsync returns stderr instead of null on failure so auto-install debug logs are actionable Co-Authored-By: Claude Sonnet 4.6 --- .../CopilotStudioSubcommand.cs | 17 ++++-- .../SetupSubcommands/PermissionsSubcommand.cs | 51 ++++++++++------ .../Internal/MicrosoftGraphTokenProvider.cs | 58 ++++++++++++++++++- .../PowerShellModulesRequirementCheck.cs | 2 +- 4 files changed, 100 insertions(+), 28 deletions(-) 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 9c2ff0a2..7545c936 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs @@ -72,13 +72,18 @@ public static Command CreateCommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } - // Verify system requirements (PowerShell modules are required for Graph operations) - var systemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!systemChecksOk) + // 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) { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + 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."); + Environment.Exit(1); + } } if (dryRun) 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 88772bbc..326503b6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -85,13 +85,18 @@ private static Command CreateMcpSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } - // Verify system requirements (PowerShell modules are required for Graph operations) - var mcpSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!mcpSystemChecksOk) + // 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) { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + 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."); + Environment.Exit(1); + } } if (dryRun) @@ -172,13 +177,18 @@ private static Command CreateBotSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } - // Verify system requirements (PowerShell modules are required for Graph operations) - var botSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!botSystemChecksOk) + // 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) { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + 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."); + Environment.Exit(1); + } } if (dryRun) @@ -255,13 +265,18 @@ private static Command CreateCustomSubcommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } - // Verify system requirements (PowerShell modules are required for Graph operations) - var customSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!customSystemChecksOk) + // 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) { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + 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."); + Environment.Exit(1); + } } if (dryRun) 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 e6f9abf1..14816d9f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -234,18 +234,69 @@ 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; } + /// + /// 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' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop; " + + "Install-Module -Name 'Microsoft.Graph.Applications' -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, @@ -287,7 +338,8 @@ private static string BuildPowerShellArguments(string shell, string script) error.Contains("not found in any module", StringComparison.OrdinalIgnoreCase))) { _logger.LogError( - "Required PowerShell module is not installed. Run 'a365 setup requirements' to install missing modules."); + "Required PowerShell module could not be loaded (auto-install was attempted but failed). " + + "Run 'a365 setup requirements' to manually install missing modules."); } return null; 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 c0df2b86..5c819be1 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 @@ -286,7 +286,7 @@ private async Task InstallModuleAsync(string moduleName, ILogger logger, C } logger.LogDebug("PowerShell command failed: {Error}", error); - return (false, null); + return (false, error); } catch (Exception ex) { From 1550c0ee3b2167d1b1d52f5b57369a40b00e9af7 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 14:54:29 -0800 Subject: [PATCH 08/18] fix: extract real JWT from Graph request headers; unify PS auth scopes to reduce login prompts Microsoft.Graph.Authentication v2+ returns an opaque (non-JWT) token from $ctx.AccessToken that is rejected when used as a Bearer token. Switch to always extracting the token from the Authorization header of a live Invoke-MgGraphRequest call, which always contains the real JWT. Add AgentIdentityBlueprint.UpdateAuthProperties.All to RequiredPermissionGrantScopes so all PowerShell-based Graph operations (OAuth2 grants, service principal lookups, and inheritable permissions) use the same scope set. This ensures Connect-MgGraph authenticates once and the access token is reused from the in-process cache for all downstream calls, reducing device code prompts from 4 to 3 on first run (1 PS + 2 MSAL for different resources). Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/SetupSubcommands/SetupHelpers.cs | 10 +++++----- .../Constants/AuthenticationConstants.cs | 11 ++++++---- .../Internal/MicrosoftGraphTokenProvider.cs | 20 ++++++++----------- 3 files changed, 20 insertions(+), 21 deletions(-) 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 91c1564a..436c21d1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -306,11 +306,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) { @@ -340,7 +340,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/Services/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index 14816d9f..863526f5 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -204,23 +204,19 @@ 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; " + + $"if ([string]::IsNullOrWhiteSpace($token)) {{ throw 'Failed to extract access token from Graph request headers' }}; " + + $"$token"; } private static string BuildScopesArray(string[] scopes) From bae2765f6063260820b68d7e3bc89e75dcc53d7b Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 15:03:35 -0800 Subject: [PATCH 09/18] fix: extract last stdout line as JWT token; add clean error for missing config - MicrosoftGraphTokenProvider: extract last non-empty stdout line as token. Connect-MgGraph in headless Linux/WSL falls back to device code and writes the 'To sign in...' prompt to stdout, contaminating the full StandardOutput string. Trimming the whole output returns a non-JWT string; taking only the last line (where $token is always written) fixes the JWT validation warning and the resulting SETUP_VALIDATION_FAILED for inheritable permissions. - ConfigFileNotFoundException: new Agent365Exception subclass for missing a365.config.json. ConfigService.LoadAsync now throws this instead of a raw FileNotFoundException, so the global handler displays a clean actionable error ('run a365 config init') instead of a full stack trace. Co-Authored-By: Claude Sonnet 4.6 --- .../Exceptions/ConfigFileNotFoundException.cs | 29 +++++++++++++++++++ .../Services/ConfigService.cs | 3 +- .../Internal/MicrosoftGraphTokenProvider.cs | 9 +++++- 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs 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..45caee24 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +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. +/// +public class ConfigFileNotFoundException : Agent365Exception +{ + public ConfigFileNotFoundException(string configFilePath) + : base( + errorCode: "CONFIG_FILE_NOT_FOUND", + issueDescription: $"Configuration file not found: {configFilePath}", + mitigationSteps: new List + { + "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" + }, + context: new Dictionary + { + ["ConfigFile"] = configFilePath + }) + { + } + + public override int ExitCode => 2; // Configuration error +} 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/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index 863526f5..3a08ed83 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -341,7 +341,14 @@ private static string BuildPowerShellArguments(string shell, string script) 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)) { From 5b58ab7c2fb0687387bc3ba123626cd457f4e21d Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 15:20:34 -0800 Subject: [PATCH 10/18] fix: unify PS token cache key in RemoveStale and DeployCommand; update test Both RemoveStaleCustomPermissionsAsync (PermissionsSubcommand) and DeployCommand used a hardcoded 2-scope array missing DelegatedPermissionGrant.ReadWrite.All. This produced a different sorted cache key than RequiredPermissionGrantScopes, causing a second Connect-MgGraph device-code prompt during 'setup blueprint' even though a token was already cached from the inheritable permissions step. Switch both sites to AuthenticationConstants.RequiredPermissionGrantScopes so all Graph token acquisitions share one cache entry per CLI invocation. Update Agent365ConfigServiceTests to expect ConfigFileNotFoundException instead of FileNotFoundException, matching the new exception type thrown by ConfigService. Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/DeployCommand.cs | 5 +++-- .../Commands/SetupSubcommands/PermissionsSubcommand.cs | 5 ++++- .../Services/Agent365ConfigServiceTests.cs | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-) 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/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 326503b6..32aad36d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -508,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 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)); } From ed2ace6b2f72e75ec5c8acf751c89ec4b815ce6c Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 16:20:31 -0800 Subject: [PATCH 11/18] fix: improve auth prompt UX and clarify custom permissions message MicrosoftGraphTokenProvider: replace misleading "Device Code: False" log with platform-specific guidance so users know what to expect: - Windows: "A browser window will open for authentication..." - Linux/macOS: "A device code prompt will appear below..." This avoids confusion when Connect-MgGraph auto-switches to device code in headless environments even though useDeviceCode=false was requested. PermissionsSubcommand: change "No custom blueprint permissions configured" to "No custom blueprint permissions specified in config. Skipping." to make clear this is about config content, not a system state or error condition. Co-Authored-By: Claude Sonnet 4.6 --- .../SetupSubcommands/PermissionsSubcommand.cs | 2 +- .../Internal/MicrosoftGraphTokenProvider.cs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) 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 32aad36d..821ae9ec 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -659,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/Services/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index 3a08ed83..b3ad1e6e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -5,6 +5,7 @@ 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; @@ -117,9 +118,15 @@ 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); From 4fd11c7b005e850a49e76ff4e0294c617315f9da Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 4 Mar 2026 16:30:52 -0800 Subject: [PATCH 12/18] fix: retry blueprint SP lookup on Azure AD propagation delay LookupServicePrincipalByAppIdAsync can return null for 10-30s after blueprint creation due to Azure AD eventual consistency. Use the existing RetryHelper (already used for inheritable permissions verification) to retry up to 5 times with 5s base delay / exponential backoff before throwing the 'service principal may not have propagated' error. Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/SetupSubcommands/SetupHelpers.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 436c21d1..5bb3da6f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -249,7 +249,15 @@ public static async Task EnsureResourcePermissionsAsync( "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}. " + From ba0e867031e3e1a80f7f4ce19cb83cdcc6426095 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 08:46:12 -0800 Subject: [PATCH 13/18] fix: fix admin consent polling via MSAL token with Application.Read.All Root cause: SP lookup and consent grant queries used az CLI token (scopes=null) which lacks Application.Read.All and DelegatedPermissionGrant.ReadWrite.All. Graph silently returned HTTP 200 with empty array, leaving blueprintSpId=null and causing polling to time out after 180s even after consent was granted. Changes: - BlueprintSubcommand: pass RequiredPermissionGrantScopes to SP lookups and CheckConsentExistsAsync; resolve blueprintSpId once at start of consent flow; use new MSAL PollAdminConsentAsync when SP ID is available - AdminConsentHelper: add MSAL PollAdminConsentAsync overload using GraphApiService; add scopes param to CheckConsentExistsAsync; add progress/timeout log messages - BlueprintLookupService: add optional scopes param to GetServicePrincipalByAppIdAsync - GraphApiService: add debug logging for failed GET; fix JsonDocument disposal - CommandExecutor: add optional outputTransform to ExecuteWithStreamingAsync - MicrosoftGraphTokenProvider: reformat PS device code output to MSAL box style - AuthenticationService: reduce token cache log noise (LogInfo -> LogDebug) Co-Authored-By: Claude Sonnet 4.6 --- .../SetupSubcommands/BlueprintSubcommand.cs | 48 +++++--- .../Services/AdminConsentHelper.cs | 0 .../Services/AuthenticationService.cs | 2 +- .../Services/BlueprintLookupService.cs | 32 ++---- .../Services/CommandExecutor.cs | 7 +- .../Services/GraphApiService.cs | 14 ++- .../Services/Helpers/AdminConsentHelper.cs | 104 +++++++++++++++++- .../Internal/MicrosoftGraphTokenProvider.cs | 37 +++++++ .../Services/BlueprintLookupServiceTests.cs | 20 +--- .../Services/GraphApiServiceTests.cs | 15 +-- .../MicrosoftGraphTokenProviderTests.cs | 7 ++ 11 files changed, 213 insertions(+), 73 deletions(-) delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/AdminConsentHelper.cs 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 5b919a8d..46044141 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -810,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) { @@ -1376,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) @@ -1383,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)) { @@ -1410,7 +1418,8 @@ private static List GetApplicationScopes(Models.Agent365Config setupConf graphSpId, applicationScopes, logger, - ct); + ct, + scopes: AuthenticationConstants.RequiredPermissionGrantScopes); } } @@ -1468,7 +1477,18 @@ await SetupHelpers.EnsureResourcePermissionsAsync( 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; 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 d05021f0..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); } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs index 8d40d2ea..d3e2d417 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,13 @@ 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); 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/CommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs index 129df504..7ee606c6 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,11 @@ 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) + { + Console.WriteLine($"{outputPrefix}{display}"); + } } else { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index 036adb1f..2c911bce 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -287,7 +287,11 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo // Ensure HttpResponseMessage is properly disposed using var resp = await _httpClient.GetAsync(url, ct); - if (!resp.IsSuccessStatusCode) return null; + if (!resp.IsSuccessStatusCode) + { + _logger.LogDebug("Graph GET {Url} failed: {StatusCode} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + return null; + } var json = await resp.Content.ReadAsStringAsync(ct); return JsonDocument.Parse(json); @@ -401,7 +405,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/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index b3ad1e6e..5cf0240d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -313,9 +313,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" 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/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/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 }); From c1e2486a5c20a660a3671072af75b93f05adcebb Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 10:46:09 -0800 Subject: [PATCH 14/18] fix: fall back to MSAL when PS Connect-MgGraph fails on any platform When PowerShell Connect-MgGraph fails (NullRef in DeviceCodeCredential on Linux, no TTY, module issues, or any other reason), fall back to MsalBrowserCredential to acquire the Graph token. On Windows this uses WAM; on Linux/macOS it uses device code with silent-first logic. The token is stored in MicrosoftGraphTokenProvider's in-process cache so subsequent calls (inheritable permissions, custom permissions) within the same CLI invocation reuse it without re-prompting. Also adds silent token acquisition attempt before device code in MsalBrowserCredential.AcquireTokenWithDeviceCodeFallbackAsync to reduce unnecessary device code prompts when a cached token exists. Co-Authored-By: Claude Sonnet 4.6 --- .../Internal/MicrosoftGraphTokenProvider.cs | 53 +++++++++++++++++++ .../Services/MsalBrowserCredential.cs | 23 ++++++++ 2 files changed, 76 insertions(+) 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 5cf0240d..cfb52494 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -10,6 +10,7 @@ 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; @@ -132,6 +133,15 @@ public MicrosoftGraphTokenProvider( 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; @@ -263,6 +273,49 @@ private async Task ExecuteWithFallbackAsync( 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. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 3d865f66..9f4c1f64 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -376,6 +376,29 @@ 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 accounts = await _publicClientApp.GetAccountsAsync(); + var cachedAccount = accounts.FirstOrDefault(); + 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"); From ac390bf37e7a2a0fd6ab0e2ab17b7d5d2acc6983 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 11:38:51 -0800 Subject: [PATCH 15/18] fix: address PR review comments - disposal, logging, exit handling, doc fixes - ConfigFileNotFoundException: derive from FileNotFoundException so existing catch sites (CleanupCommand, DeployCommand, etc.) continue to work - InteractiveGraphAuthService: remove unused Azure.Identity using; move credential factory resolution inside try/catch for consistent error wrapping - BrowserHelper: include exception object in LogWarning for structured logging - PowerShellModulesRequirementCheck: fix GenerateInstallationInstructions to produce valid PS syntax for multi-module Install-Module calls - PermissionsSubcommand / CopilotStudioSubcommand: replace Environment.Exit(1) with ExceptionHandler.ExitWithCleanup(1) to flush output before exit - MicrosoftGraphTokenProvider: dispose HttpResponseMessage after token extraction - MsalBrowserCredential: use platform-neutral path notation in cache doc comment Co-Authored-By: Claude Sonnet 4.6 --- .../CopilotStudioSubcommand.cs | 5 +++-- .../SetupSubcommands/PermissionsSubcommand.cs | 12 +++++------ .../Exceptions/ConfigFileNotFoundException.cs | 21 +++++++------------ .../Helpers/BrowserHelper.cs | 2 +- .../Services/InteractiveGraphAuthService.cs | 13 ++++++------ .../Internal/MicrosoftGraphTokenProvider.cs | 1 + .../Services/MsalBrowserCredential.cs | 2 +- .../PowerShellModulesRequirementCheck.cs | 6 +++--- 8 files changed, 30 insertions(+), 32 deletions(-) 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 7545c936..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 @@ -82,7 +83,7 @@ public static Command CreateCommand( if (!systemChecksOk) { logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } } 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 821ae9ec..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 @@ -95,7 +95,7 @@ private static Command CreateMcpSubcommand( if (!mcpSystemChecksOk) { logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } } @@ -168,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 @@ -187,7 +187,7 @@ private static Command CreateBotSubcommand( if (!botSystemChecksOk) { logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } } @@ -256,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 @@ -275,7 +275,7 @@ private static Command CreateCustomSubcommand( if (!customSystemChecksOk) { logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - Environment.Exit(1); + ExceptionHandler.ExitWithCleanup(1); } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs index 45caee24..27b40a77 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs @@ -6,24 +6,19 @@ 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 : Agent365Exception +public class ConfigFileNotFoundException : FileNotFoundException { public ConfigFileNotFoundException(string configFilePath) : base( - errorCode: "CONFIG_FILE_NOT_FOUND", - issueDescription: $"Configuration file not found: {configFilePath}", - mitigationSteps: new List - { - "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" - }, - context: new Dictionary - { - ["ConfigFile"] = configFilePath - }) + 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 override int ExitCode => 2; // Configuration error + 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 index 3f9c29cb..3dc7688c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -44,7 +44,7 @@ public static void TryOpenUrl(string url, ILogger? logger = null) { if (logger != null) { - logger.LogWarning("Failed to open browser automatically: {Message}", ex.Message); + logger.LogWarning(ex, "Failed to open browser automatically for URL {Url}", url); logger.LogInformation("Please manually open: {Url}", url); } else diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs index 55ffd986..3ac81787 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; @@ -90,10 +89,6 @@ public async Task GetAuthenticatedGraphClientAsync( _logger.LogInformation("Please sign in with an account that has Global Administrator or similar privileges."); _logger.LogInformation(""); - // Resolve credential: use injected factory (for tests) or default MsalBrowserCredential - var credential = _credentialFactory?.Invoke(_clientAppId, tenantId) - ?? new MsalBrowserCredential(_clientAppId, tenantId, redirectUri: null, _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."); @@ -102,9 +97,15 @@ public async Task GetAuthenticatedGraphClientAsync( // 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 { + // 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")) @@ -146,7 +147,7 @@ public async Task GetAuthenticatedGraphClientAsync( _logger.LogInformation("Successfully authenticated to Microsoft Graph!"); _logger.LogInformation(""); - var graphClient = new GraphServiceClient(credential, RequiredScopes); + var graphClient = new GraphServiceClient(credential!, RequiredScopes); _cachedClient = graphClient; _cachedTenantId = tenantId; 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 cfb52494..dd2c50ab 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -232,6 +232,7 @@ private static string BuildPowerShellScript(string tenantId, string[] scopes, bo $"if ($null -eq $ctx) {{ throw 'Failed to establish Graph context' }}; " + $"$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"; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 9f4c1f64..d9e61862 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -22,7 +22,7 @@ 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/macOS) +/// 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 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 5c819be1..6960b7ef 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 @@ -307,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 From 1867f3acd975c6770e9e891089b95e94a7174261 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 11:58:16 -0800 Subject: [PATCH 16/18] fix: address second round of PR review comments - ConfigFileNotFoundException: add explicit using System.IO to avoid implicit global usings dependency - SetupHelpers: remove unnecessary ! null-forgiving operator on awaited task (it only asserts the Task itself is non-null, not the result) - BrowserHelper: use ArgumentList instead of Arguments for open/xdg-open to avoid argument parsing issues with special characters in URLs - MsalBrowserCredential: filter cached MSAL account by tenant ID before silent auth to avoid authenticating as wrong identity when multiple accounts are cached; fall through to device code if no tenant match - MicrosoftGraphTokenProvider: reuse IsPowerShellModuleMissingError in ProcessResult to eliminate duplicated string-matching logic Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/SetupSubcommands/SetupHelpers.cs | 2 +- .../Exceptions/ConfigFileNotFoundException.cs | 2 ++ .../Helpers/BrowserHelper.cs | 6 ++++-- .../Services/Internal/MicrosoftGraphTokenProvider.cs | 5 +---- .../Services/MsalBrowserCredential.cs | 11 +++++++++-- 5 files changed, 17 insertions(+), 9 deletions(-) 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 5bb3da6f..9ea0af0b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -252,7 +252,7 @@ public static async Task EnsureResourcePermissionsAsync( // 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)!, + operation: (innerCt) => graph.LookupServicePrincipalByAppIdAsync(config.TenantId, config.AgentBlueprintId, innerCt, permissionGrantScopes), shouldRetry: result => string.IsNullOrWhiteSpace(result), maxRetries: 5, baseDelaySeconds: 5, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs index 27b40a77..d15f7906 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.IO; + namespace Microsoft.Agents.A365.DevTools.Cli.Exceptions; /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs index 3dc7688c..e3167dd4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/BrowserHelper.cs @@ -31,11 +31,13 @@ public static void TryOpenUrl(string url, ILogger? logger = null) } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - psi = new ProcessStartInfo { FileName = "open", Arguments = url }; + psi = new ProcessStartInfo { FileName = "open" }; + psi.ArgumentList.Add(url); } else { - psi = new ProcessStartInfo { FileName = "xdg-open", Arguments = url }; + psi = new ProcessStartInfo { FileName = "xdg-open" }; + psi.ArgumentList.Add(url); } using var process = new Process { StartInfo = psi }; process.Start(); 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 dd2c50ab..821f278c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -426,10 +426,7 @@ private static string BuildPowerShellArguments(string shell, string script) "Failed to acquire Microsoft Graph access token. Error: {Error}", result.StandardError); - var error = result.StandardError ?? string.Empty; - if (error.Contains("module", StringComparison.OrdinalIgnoreCase) && - (error.Contains("was not loaded", StringComparison.OrdinalIgnoreCase) || - error.Contains("not found in any module", StringComparison.OrdinalIgnoreCase))) + if (IsPowerShellModuleMissingError(result)) { _logger.LogError( "Required PowerShell module could not be loaded (auto-install was attempted but failed). " + diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index d9e61862..c98404b8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -380,8 +380,15 @@ private async Task AcquireTokenWithDeviceCodeFallbackAsync( // 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 accounts = await _publicClientApp.GetAccountsAsync(); - var cachedAccount = accounts.FirstOrDefault(); + var accountsList = (await _publicClientApp.GetAccountsAsync()).ToList(); + // Filter by tenant to avoid silently authenticating as the wrong identity when multiple accounts are cached. + 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 From 64b8c2d5357b7d07859100157dfe1e70fe65fc44 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 12:17:53 -0800 Subject: [PATCH 17/18] chore: add CHANGELOG, NuGet release notes, and review process updates - CHANGELOG.md: Keep a Changelog format covering [Unreleased], 1.1.0, 1.0.0 - Directory.Build.props: PackageReleaseNotes points to CHANGELOG.md (fixes NuGet warning) - src/DEVELOPER.md: update Release Process to describe workflow; add CHANGELOG to PR checklist - .github/copilot-instructions.md: add CHANGELOG reminder to Code Review Mindset - .claude/agents/pr-code-reviewer.md: add CHANGELOG check in Step 2 - .claude/agents/code-reviewer.md: add CHANGELOG check to Self-Verification - .claude/agents/code-review-manager.md: add CHANGELOG to Project Context standards Co-Authored-By: Claude Sonnet 4.6 --- .claude/agents/code-review-manager.md | 1 + .claude/agents/code-reviewer.md | 1 + .claude/agents/pr-code-reviewer.md | 7 +++- .github/copilot-instructions.md | 1 + CHANGELOG.md | 53 +++++++++++++++++++++++++++ src/DEVELOPER.md | 45 +++++++++++------------ src/Directory.Build.props | 1 + 7 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 CHANGELOG.md 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. From cccd8100e70976f243cf02d47837a8c27d8ae05e Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Thu, 5 Mar 2026 12:28:41 -0800 Subject: [PATCH 18/18] fix: address PR review comments - StringComparison, PSGallery, encoding, docs - MsalBrowserCredential.cs: StringComparison.Ordinal on Contains calls; comment clarifying same-tenant account edge case - InteractiveGraphAuthService.cs: StringComparison.Ordinal on all Contains calls in exception filters - PowerShellModulesRequirementCheck.cs: pin Install-Module to -Repository PSGallery - MicrosoftGraphTokenProvider.cs: pin both Install-Module calls to -Repository PSGallery - GraphApiService.cs: fix U+FFFD encoding artifact in comment - CommandExecutor.cs: document outputPrefix applies only to first line (outputTransform must not return multi-line) - BlueprintLookupService.cs: comment that DisplayName is not populated in this lookup path Co-Authored-By: Claude Sonnet 4.6 --- .../Services/BlueprintLookupService.cs | 1 + .../Services/CommandExecutor.cs | 1 + .../Services/GraphApiService.cs | 2 +- .../Services/InteractiveGraphAuthService.cs | 8 ++++---- .../Services/Internal/MicrosoftGraphTokenProvider.cs | 4 ++-- .../Services/MsalBrowserCredential.cs | 6 ++++-- .../PowerShellModulesRequirementCheck.cs | 2 +- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs index d3e2d417..fe9bf45d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs @@ -267,6 +267,7 @@ public async Task GetServicePrincipalByAppIdAsync( _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, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs index 7ee606c6..ba47a82c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs @@ -160,6 +160,7 @@ public virtual async Task ExecuteWithStreamingAsync( 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}"); } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index 9041a239..de3cbdad 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -408,7 +408,7 @@ public async Task GraphDeleteAsync( public virtual async Task LookupServicePrincipalByAppIdAsync( string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) { - // $filter=appId eq is "Default+Advanced" per Graph docs � no ConsistencyLevel header required. + // $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, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs index 3ac81787..b283f175 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/InteractiveGraphAuthService.cs @@ -108,15 +108,15 @@ public async Task GetAuthenticatedGraphClientAsync( 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)) { 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)) { _logger.LogError("Browser authentication failed due to connectivity issue: {Message}", ex.Message); throw new GraphApiException( 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 821f278c..fb04aee9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -327,8 +327,8 @@ private async Task TryAutoInstallRequiredModulesAsync(string shell, Cancel try { var installScript = - "Install-Module -Name 'Microsoft.Graph.Authentication' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop; " + - "Install-Module -Name 'Microsoft.Graph.Applications' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop"; + "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) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index c98404b8..913fee89 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -382,6 +382,8 @@ private async Task AcquireTokenWithDeviceCodeFallbackAsync( // 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, @@ -430,9 +432,9 @@ private async Task AcquireTokenWithDeviceCodeFallbackAsync( return new AccessToken(deviceCodeResult.AccessToken, deviceCodeResult.ExpiresOn); } catch (MsalException msalEx) when ( - msalEx.Message.Contains("AADSTS7000218") || + msalEx.Message.Contains("AADSTS7000218", StringComparison.Ordinal) || (msalEx is MsalServiceException svcEx && svcEx.ErrorCode == "invalid_client" && - msalEx.Message.Contains("client_assertion"))) + 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. 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 6960b7ef..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 @@ -232,7 +232,7 @@ private async Task InstallModuleAsync(string moduleName, ILogger logger, C { try { - var command = $"Install-Module -Name '{moduleName}' -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop"; + 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) {