diff --git a/CHANGELOG.md b/CHANGELOG.md index a791e03a..8028f0e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `a365 publish` updates manifest IDs, creates `manifest.zip`, and prints concise upload instructions for Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). Interactive prompts only occur in interactive terminals; redirect stdin to suppress them in scripts. ### Fixed +- Intermittent `ConnectionResetError (10054)` failures on corporate networks with TLS inspection proxies (Zscaler, Netskope) — Graph and ARM API calls now use direct MSAL.NET token acquisition instead of `az account get-access-token` subprocesses, bypassing the Python HTTP stack that triggered proxy resets (#321) - `a365 cleanup` blueprint deletion now succeeds for Global Administrators even when the blueprint was created by a different user - `a365 setup all` no longer times out for non-admin users — the CLI immediately surfaces a consent URL to share with an administrator instead of waiting for a browser prompt - `a365 setup all` requests admin consent once for all resources instead of prompting once per resource 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 b7ce3663..adb8f062 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -386,6 +386,7 @@ public static async Task CreateBlueprintImplementationA new GraphApiService( cleanLoggerFactory.CreateLogger(), executor, + new AuthenticationService(cleanLoggerFactory.CreateLogger()), graphBaseUrl: setupConfig.GraphBaseUrl)); // Use DI-provided GraphApiService which already has MicrosoftGraphTokenProvider configured @@ -761,7 +762,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( existingServicePrincipalId = null; // SP missing for an existing app — attempt creation so downstream steps have a valid SP. logger.LogInformation("Service principal not found for existing blueprint — attempting to create it..."); - var spToken = await graphApiService.GetGraphAccessTokenAsync(tenantId, ct); + var spToken = await graphApiService.GetGraphAccessTokenAsync(tenantId, ct: ct); if (!string.IsNullOrWhiteSpace(spToken)) { using var spHttpClient = Services.Internal.HttpClientFactory.CreateAuthenticatedClient(spToken); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs index 73f18fbc..75e5baf9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs @@ -300,42 +300,10 @@ public static async Task ValidateAzureCliAuthenticationAsync( logger.LogDebug("Azure CLI already authenticated as {LoginHint}", loginHint); } - // Verify we have the management scope (token is cached at process level by AzCliHelper). - logger.LogDebug("Verifying access to Azure management resources..."); - var managementToken = await AzCliHelper.AcquireAzCliTokenAsync(ArmApiService.ArmResource, tenantId); - - if (string.IsNullOrWhiteSpace(managementToken)) - { - logger.LogWarning("Unable to acquire management scope token. Attempting re-authentication..."); - logger.LogInformation("A browser window will open for authentication."); - - var loginResult = await executor.ExecuteAsync("az", $"login --tenant {tenantId}", cancellationToken: cancellationToken); - - if (!loginResult.Success) - { - logger.LogError("Azure CLI login with management scope failed. Please run manually: az login --scope https://management.core.windows.net//.default"); - return false; - } - - logger.LogInformation("Azure CLI re-authentication successful!"); - AzCliHelper.InvalidateAzCliTokenCache(); - await Task.Delay(2000, cancellationToken); - - var retryToken = await AzCliHelper.AcquireAzCliTokenAsync(ArmApiService.ArmResource, tenantId); - if (string.IsNullOrWhiteSpace(retryToken)) - { - logger.LogWarning("Still unable to acquire management scope token after re-authentication."); - logger.LogWarning("Continuing anyway - you may encounter permission errors later."); - } - else - { - logger.LogDebug("Management scope token acquired successfully!"); - } - } - else - { - logger.LogDebug("Management scope verified successfully"); - } + // ARM token acquisition is handled lazily by ArmApiService via MSAL (WAM/browser/device-code). + // No eager token pre-fetch is needed here — MSAL will prompt for sign-in when the + // first ARM call is made if no valid cached token exists. + logger.LogDebug("Azure authentication verified. ARM token will be acquired on first resource call."); return true; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 09d9c276..7878f4ea 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -254,6 +254,7 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(sp => sp.GetRequiredService()); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs index aaa66491..448bb3eb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs @@ -483,7 +483,7 @@ string GetConfig(string name) => try { // Use Azure CLI token to get current user (this requires delegated context) - var delegatedToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct); + var delegatedToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct: ct); if (!string.IsNullOrWhiteSpace(delegatedToken)) { using var delegatedClient = HttpClientFactory.CreateAuthenticatedClient(delegatedToken, correlationId: correlationId); @@ -680,7 +680,7 @@ string GetConfig(string name) => _logger.LogInformation(" - Agent Identity ID: {Id}", agenticAppId); // Get Graph access token - var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct); + var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct: ct); if (string.IsNullOrWhiteSpace(graphToken)) { _logger.LogError("Failed to acquire Graph API access token"); @@ -940,7 +940,7 @@ private async Task AssignLicensesAsync( _logger.LogInformation("Assigning licenses to user {UserId} using Graph API (CorrelationId: {CorrelationId})", userId, correlationId); // Get Graph access token - var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, cancellationToken); + var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct: cancellationToken); if (string.IsNullOrWhiteSpace(graphToken)) { _logger.LogError("Failed to acquire Graph API access token for license assignment"); @@ -1151,7 +1151,7 @@ private async Task VerifyServicePrincipalExistsAsync( try { // Use Graph API to check if service principal exists - var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct); + var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct: ct); if (string.IsNullOrWhiteSpace(graphToken)) { _logger.LogWarning("Failed to acquire Graph token for service principal verification"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs index 2fec7fc4..c60430b3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs @@ -15,8 +15,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// Service for Azure Resource Manager (ARM) existence checks via direct HTTP. /// Replaces subprocess-based 'az group exists', 'az appservice plan show', and /// 'az webapp show' calls — each drops from ~15-20s to ~0.5s. -/// Token acquisition is handled by AzCliHelper (process-level cache shared with -/// other services using the management endpoint). +/// Token acquisition uses MSAL via AuthenticationService (WAM on Windows, +/// browser/device-code on macOS/Linux) — no az CLI subprocess involved. /// public class ArmApiService : IDisposable { @@ -27,17 +27,21 @@ public class ArmApiService : IDisposable private readonly ILogger _logger; private readonly HttpClient _httpClient; + private readonly IAuthenticationService _authService; + private readonly RetryHelper _retryHelper; - // Allow injecting a custom HttpMessageHandler for unit testing. - public ArmApiService(ILogger logger, HttpMessageHandler? handler = null) + // Allow injecting a custom HttpMessageHandler and RetryHelper for unit testing. + public ArmApiService(ILogger logger, IAuthenticationService authService, HttpMessageHandler? handler = null, RetryHelper? retryHelper = null) { _logger = logger; + _authService = authService; _httpClient = handler != null ? new HttpClient(handler) : HttpClientFactory.CreateAuthenticatedClient(); + _retryHelper = retryHelper ?? new RetryHelper(_logger); } // Parameterless constructor to ease test mocking/substitution frameworks. public ArmApiService() - : this(NullLogger.Instance, null) + : this(NullLogger.Instance, new AuthenticationService(NullLogger.Instance), null) { } @@ -45,7 +49,7 @@ public ArmApiService() private async Task EnsureArmHeadersAsync(string tenantId, CancellationToken ct) { - var token = await AzCliHelper.AcquireAzCliTokenAsync(ArmResource, tenantId); + var token = await _authService.GetAccessTokenAsync(ArmResource, tenantId); if (string.IsNullOrWhiteSpace(token)) { _logger.LogWarning("Unable to acquire ARM access token for tenant {TenantId}", tenantId); @@ -74,7 +78,8 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke try { - using var response = await _httpClient.GetAsync(url, ct); + using var response = await _retryHelper.ExecuteWithRetryAsync( + ct => _httpClient.GetAsync(url, ct), cancellationToken: ct); _logger.LogDebug("ARM resource group check: {StatusCode}", response.StatusCode); if (response.StatusCode == HttpStatusCode.OK) return true; if (response.StatusCode == HttpStatusCode.NotFound) return false; @@ -82,7 +87,10 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke } catch (Exception ex) { - _logger.LogDebug(ex, "ARM resource group check failed — will fall back to az CLI"); + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "ARM resource group check failed — will fall back to az CLI"); return null; } } @@ -106,7 +114,8 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke try { - using var response = await _httpClient.GetAsync(url, ct); + using var response = await _retryHelper.ExecuteWithRetryAsync( + ct => _httpClient.GetAsync(url, ct), cancellationToken: ct); _logger.LogDebug("ARM app service plan check: {StatusCode}", response.StatusCode); if (response.StatusCode == HttpStatusCode.OK) return true; if (response.StatusCode == HttpStatusCode.NotFound) return false; @@ -114,7 +123,10 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke } catch (Exception ex) { - _logger.LogDebug(ex, "ARM app service plan check failed — will fall back to az CLI"); + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "ARM app service plan check failed — will fall back to az CLI"); return null; } } @@ -138,7 +150,8 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke try { - using var response = await _httpClient.GetAsync(url, ct); + using var response = await _retryHelper.ExecuteWithRetryAsync( + ct => _httpClient.GetAsync(url, ct), cancellationToken: ct); _logger.LogDebug("ARM web app check: {StatusCode}", response.StatusCode); if (response.StatusCode == HttpStatusCode.OK) return true; if (response.StatusCode == HttpStatusCode.NotFound) return false; @@ -146,7 +159,10 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke } catch (Exception ex) { - _logger.LogDebug(ex, "ARM web app check failed — will fall back to az CLI"); + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "ARM web app check failed — will fall back to az CLI"); return null; } } @@ -186,7 +202,8 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke try { - using var response = await _httpClient.GetAsync(url, ct); + using var response = await _retryHelper.ExecuteWithRetryAsync( + ct => _httpClient.GetAsync(url, ct), cancellationToken: ct); if (!response.IsSuccessStatusCode) { _logger.LogDebug("ARM role assignment check returned {StatusCode}", response.StatusCode); @@ -218,8 +235,12 @@ private async Task EnsureArmHeadersAsync(string tenantId, CancellationToke } catch (Exception ex) { - _logger.LogDebug(ex, "ARM role assignment check failed — will fall back to az CLI"); + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "ARM role assignment check failed — will fall back to az CLI"); return null; } } + } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs index edb47613..14cf72ce 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs @@ -12,6 +12,25 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; +/// +/// Abstraction over MSAL token acquisition used by ArmApiService and GraphApiService. +/// Enables test substitution without triggering real interactive authentication. +/// Co-located with AuthenticationService per the related-interfaces convention. +/// +public interface IAuthenticationService +{ + Task GetAccessTokenAsync( + string resourceUrl, + string? tenantId = null, + bool forceRefresh = false, + string? clientId = null, + IEnumerable? scopes = null, + bool useInteractiveBrowser = true, + string? userId = null); + + Task ResolveLoginHintFromCacheAsync(); +} + /// /// Service for handling authentication to Agent 365 Tools and Microsoft Graph API. /// @@ -37,7 +56,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// - Subsequent commands: 0 prompts (uses cached tokens) /// - Token refresh: Automatic when within 5 minutes of expiration /// -public class AuthenticationService +public class AuthenticationService : IAuthenticationService { private readonly ILogger _logger; private readonly string _tokenCachePath; @@ -105,14 +124,14 @@ public async Task GetAccessTokenAsync( } else { - _logger.LogInformation("Using cached authentication token for {ResourceUrl} (tenant: {TenantId})", + _logger.LogDebug("Using cached authentication token for {ResourceUrl} (tenant: {TenantId})", resourceUrl, tenantId); return cachedToken.AccessToken; } } else { - _logger.LogInformation("Using cached authentication token for {ResourceUrl}", resourceUrl); + _logger.LogDebug("Using cached authentication token for {ResourceUrl}", resourceUrl); return cachedToken.AccessToken; } } @@ -168,8 +187,8 @@ private async Task AuthenticateInteractivelyAsync( // This creates the format required by Azure AD for the TokenRequestContext: {resourceAppId}/{scope} // Example: "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1/McpServers.Mail.All" scopes = explicitScopes.Select(s => $"{resourceUrl}/{s}").ToArray(); - _logger.LogInformation("Using explicit scopes for authentication: {Scopes}", string.Join(", ", explicitScopes)); - _logger.LogInformation("Formatted as: {FormattedScopes}", string.Join(", ", scopes)); + _logger.LogDebug("Using explicit scopes for authentication: {Scopes}", string.Join(", ", explicitScopes)); + _logger.LogDebug("Formatted as: {FormattedScopes}", string.Join(", ", scopes)); } else { @@ -206,13 +225,13 @@ private async Task AuthenticateInteractivelyAsync( scope = resourceUrl.EndsWith("/.default", StringComparison.OrdinalIgnoreCase) ? resourceUrl : $"{resourceUrl}/.default"; - _logger.LogInformation("Using custom resource for authentication: {Resource}", resourceUrl); + _logger.LogDebug("Using custom resource for authentication: {Resource}", resourceUrl); } scopes = [scope]; - _logger.LogInformation($"Token scope: {scope}"); + _logger.LogDebug("Token scope: {Scope}", scope); } - _logger.LogInformation("Authenticating for tenant: {TenantId}", effectiveTenantId); + _logger.LogDebug("Authenticating for tenant: {TenantId}", effectiveTenantId); // Use provided client ID or default to PowerShell client ID effectiveClientId = string.IsNullOrWhiteSpace(clientId) @@ -559,6 +578,57 @@ protected virtual TokenCredential CreateDeviceCodeCredential(string clientId, st }); } + /// + /// Resolves the login hint (UPN) from the persistent token cache by decoding a cached JWT. + /// Used to pre-select the correct account for WAM/MSAL when az CLI is not available. + /// Returns null if no cached token exists or the UPN claim cannot be read. + /// + public async Task ResolveLoginHintFromCacheAsync() + { + if (!File.Exists(_tokenCachePath)) + return null; + try + { + var json = await File.ReadAllTextAsync(_tokenCachePath); + var cache = JsonSerializer.Deserialize(json); + if (cache?.Tokens == null) return null; + foreach (var entry in cache.Tokens.Values) + { + var upn = TryExtractUpnFromJwt(entry.AccessToken); + if (!string.IsNullOrWhiteSpace(upn)) + return upn; + } + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to resolve login hint from token cache"); + } + return null; + } + + private static string? TryExtractUpnFromJwt(string? jwt) + { + if (string.IsNullOrWhiteSpace(jwt)) return null; + try + { + var parts = jwt.Split('.'); + if (parts.Length < 2) return null; + var payload = parts[1]; + // JWT uses Base64Url encoding: replace URL-safe chars before standard Base64 decode. + payload = payload.Replace('-', '+').Replace('_', '/'); + // Restore Base64 padding stripped by JWT encoding. + payload = payload.PadRight(payload.Length + (4 - payload.Length % 4) % 4, '='); + var bytes = Convert.FromBase64String(payload); + using var doc = JsonDocument.Parse(bytes); + if (doc.RootElement.TryGetProperty("upn", out var upn) && !string.IsNullOrWhiteSpace(upn.GetString())) + return upn.GetString(); + if (doc.RootElement.TryGetProperty("preferred_username", out var pref) && !string.IsNullOrWhiteSpace(pref.GetString())) + return pref.GetString(); + } + catch { } // Static helper — no logger access. Caller logs via ResolveLoginHintFromCacheAsync. + return null; + } + /// /// Clears cached authentication token(s) /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs index d389c333..6ebbd5d9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs @@ -61,26 +61,9 @@ public async Task CreateEndpointWithAgentBlueprintAs try { - // Get subscription info for tenant ID - var subscriptionResult = await _executor.ExecuteAsync("az", "account show", captureOutput: true); - if (subscriptionResult == null) - { - _logger.LogError("Failed to execute account show command - null result"); - return EndpointRegistrationResult.Failed; - } - - if (!subscriptionResult.Success) - { - _logger.LogError("Failed to get subscription information for endpoint creation"); - return EndpointRegistrationResult.Failed; - } - - var cleanedOutput = JsonDeserializationHelper.CleanAzureCliJsonOutput(subscriptionResult.StandardOutput); - var subscriptionInfo = JsonSerializer.Deserialize(cleanedOutput); - var tenantId = subscriptionInfo.GetProperty("tenantId").GetString(); - var currentUser = subscriptionInfo.TryGetProperty("user", out var userProp) && - userProp.TryGetProperty("name", out var nameProp) - ? nameProp.GetString() : null; + // Load config first to get tenant ID — avoids az CLI subprocess for account info. + var config = await _configService.LoadAsync(); + var tenantId = config.TenantId; if (string.IsNullOrEmpty(tenantId)) { @@ -88,12 +71,17 @@ public async Task CreateEndpointWithAgentBlueprintAs return EndpointRegistrationResult.Failed; } + // Resolve login hint for account pre-selection in WAM/MSAL. + // Try az CLI first (if the user has run 'az login'); fall back to the + // UPN embedded in a previously cached MSAL token — non-fatal if unavailable. + var currentUser = await AzCliHelper.ResolveLoginHintAsync() + ?? await _authService.ResolveLoginHintFromCacheAsync(); + // Create new endpoint with agent blueprint identity _logger.LogInformation("Creating new endpoint with Agent Blueprint Identity..."); try { - var config = await _configService.LoadAsync(); var createEndpointUrl = EndpointHelper.GetCreateEndpointUrl(config.Environment); _logger.LogInformation("Calling create endpoint directly..."); @@ -254,27 +242,8 @@ public async Task DeleteEndpointWithAgentBlueprintAsync( try { - // Get subscription info for tenant ID - var subscriptionResult = await _executor.ExecuteAsync("az", "account show", captureOutput: true); - if (subscriptionResult == null) - { - _logger.LogError("Failed to execute account show command - null result"); - return false; - } - - if (!subscriptionResult.Success) - { - _logger.LogError("Failed to get subscription information for endpoint deletion"); - return false; - } - - var cleanedOutput = JsonDeserializationHelper.CleanAzureCliJsonOutput(subscriptionResult.StandardOutput); - var subscriptionInfo = JsonSerializer.Deserialize(cleanedOutput); - var tenantId = subscriptionInfo.GetProperty("tenantId").GetString(); - var currentUser = subscriptionInfo.TryGetProperty("user", out var userProp) && - userProp.TryGetProperty("name", out var nameProp) - ? nameProp.GetString() : null; - _logger.LogDebug("ATG token request — current user from az account: {CurrentUser}", currentUser ?? "(null)"); + var config = await _configService.LoadAsync(); + var tenantId = config.TenantId; if (string.IsNullOrEmpty(tenantId)) { @@ -282,12 +251,15 @@ public async Task DeleteEndpointWithAgentBlueprintAsync( return false; } + var currentUser = await AzCliHelper.ResolveLoginHintAsync() + ?? await _authService.ResolveLoginHintFromCacheAsync(); + _logger.LogDebug("ATG token request — current user: {CurrentUser}", currentUser ?? "(null)"); + // Delete endpoint with agent blueprint identity _logger.LogInformation("Deleting endpoint with Agent Blueprint Identity..."); try { - var config = await _configService.LoadAsync(); var deleteEndpointUrl = EndpointHelper.GetDeleteEndpointUrl(config.Environment); _logger.LogInformation("Calling delete endpoint directly..."); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs index eb155eb4..c37bcfc2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs @@ -548,7 +548,7 @@ private async Task TryExtendConsentGrantScopesAsync( const string path = "/v1.0/applications?$filter=appId eq '{0}'&$select=id,appId,displayName,requiredResourceAccess"; var graphResponse = await _graphApiService.GraphGetWithResponseAsync(tenantId, - string.Format(path, clientAppId), ct); + string.Format(path, clientAppId), ct: ct); if (graphResponse == null || !graphResponse.IsSuccess) { @@ -561,10 +561,9 @@ private async Task TryExtendConsentGrantScopesAsync( return null; } - _logger.LogDebug("Graph app query returned 401 — invalidating token cache and retrying (possible CAE revocation)"); - AzCliHelper.InvalidateAzCliTokenCache(); + _logger.LogDebug("Graph app query returned 401 — retrying with fresh token (possible CAE revocation)"); graphResponse = await _graphApiService.GraphGetWithResponseAsync(tenantId, - string.Format(path, clientAppId), ct); + string.Format(path, clientAppId), forceRefresh: true, ct: ct); if (!graphResponse.IsSuccess) throw ClientAppValidationException.TokenRevoked(clientAppId); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/DelegatedConsentService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/DelegatedConsentService.cs index 1c7e7be9..74f1cc35 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/DelegatedConsentService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/DelegatedConsentService.cs @@ -69,7 +69,7 @@ public async Task EnsureBlueprintPermissionGrantAsync( // Get Graph access token with required scopes _logger.LogInformation("Acquiring Graph API access token..."); - var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, cancellationToken); + var graphToken = await _graphService.GetGraphAccessTokenAsync(tenantId, ct: cancellationToken); if (string.IsNullOrWhiteSpace(graphToken)) { _logger.LogError("Failed to acquire Graph API access token"); @@ -275,13 +275,9 @@ public async Task EnsureBlueprintPermissionGrantAsync( return null; } - // The new az login session invalidates any previously cached tokens. - AzCliHelper.InvalidateAzCliTokenCache(); - _logger.LogInformation(" Acquiring fresh Graph API token..."); - // Re-populate the process-level cache with the new session's token. - var token = await AzCliHelper.AcquireAzCliTokenAsync(GraphApiConstants.GetResource(GraphApiConstants.BaseUrl), tenantId); + var token = await _graphService.GetGraphAccessTokenAsync(tenantId); if (!string.IsNullOrWhiteSpace(token)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index c4dd688b..7ee33719 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -1,922 +1,886 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; -using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using System.Net; -using System.Net.Http.Headers; -using System.Text; -using System.Text.Json; -using System.Text.Json.Nodes; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services; - -/// -/// Service for managing Microsoft Graph API permissions and registrations -/// -public class GraphApiService -{ - private readonly ILogger _logger; - private readonly CommandExecutor _executor; - private readonly HttpClient _httpClient; - private readonly IMicrosoftGraphTokenProvider? _tokenProvider; - private string _graphBaseUrl; - - // Token caching is handled at the process level by AzCliHelper.AcquireAzCliTokenAsync. - // All GraphApiService instances (and other services) share a single token per - // (resource, tenantId) pair — no per-instance cache needed. - - // Login hint resolved once per GraphApiService instance from 'az account show'. - // Used to direct MSAL/WAM to the correct Azure CLI identity, preventing the Windows - // account (WAM default) or a stale cached MSAL account from being used instead. - private string? _loginHint; - private bool _loginHintResolved; - - // Resolver delegate for the login hint. Defaults to AzCliHelper.ResolveLoginHintAsync; - // injectable via constructor so unit tests can bypass the real 'az account show' process. - private readonly Func> _loginHintResolver; - - /// - /// Optional custom client app ID to use for authentication with Microsoft Graph PowerShell. - /// When set, this will be passed to Connect-MgGraph -ClientId parameter. - /// - public string? CustomClientAppId { get; set; } - - /// - /// Override the Microsoft Graph base URL for sovereign / government cloud tenants. - /// Defaults to (commercial cloud). - /// Set this after construction when the config is available (e.g. from Agent365Config.GraphBaseUrl). - /// - public string GraphBaseUrl - { - get => _graphBaseUrl; - set => _graphBaseUrl = string.IsNullOrWhiteSpace(value) ? GraphApiConstants.BaseUrl : value; - } - - // Lightweight wrapper to surface HTTP status, reason and body to callers - public record GraphResponse - { - public bool IsSuccess { get; init; } - public int StatusCode { get; init; } - public string ReasonPhrase { get; init; } = string.Empty; - public string Body { get; init; } = string.Empty; - public JsonDocument? Json { get; init; } - } - - // Allow injecting a custom HttpMessageHandler for unit testing. - // loginHintResolver: optional override for 'az account show' login-hint resolution. - // Pass () => Task.FromResult(null) in unit tests to skip the real az process. - public GraphApiService(ILogger logger, CommandExecutor executor, HttpMessageHandler? handler = null, IMicrosoftGraphTokenProvider? tokenProvider = null, Func>? loginHintResolver = null, string? graphBaseUrl = null) - { - _logger = logger; - _executor = executor; - _httpClient = handler != null ? new HttpClient(handler) : HttpClientFactory.CreateAuthenticatedClient(); - _tokenProvider = tokenProvider; - _loginHintResolver = loginHintResolver ?? AzCliHelper.ResolveLoginHintAsync; - _graphBaseUrl = string.IsNullOrWhiteSpace(graphBaseUrl) ? GraphApiConstants.BaseUrl : graphBaseUrl; - } - - // Parameterless constructor to ease test mocking/substitution frameworks which may - // require creating proxy instances without providing constructor arguments. - public GraphApiService() - : this(NullLogger.Instance, new CommandExecutor(NullLogger.Instance), null, null, null) - { - } - - // Two-argument convenience constructor used by tests and callers that supply - // a logger and an existing CommandExecutor (no custom handler). - public GraphApiService(ILogger logger, CommandExecutor executor) - : this(logger ?? NullLogger.Instance, executor ?? throw new ArgumentNullException(nameof(executor)), null, null, null) - { - } - - /// - /// Get access token for Microsoft Graph API using Azure CLI - /// - public virtual async Task GetGraphAccessTokenAsync(string tenantId, CancellationToken ct = default) - { - _logger.LogDebug("Acquiring Graph API access token for tenant {TenantId}", tenantId); - - try - { - var resource = GraphApiConstants.GetResource(_graphBaseUrl); - - // Check the process-level cache first. AzCliHelper caches tokens per (resource, tenantId) - // for the process lifetime — avoids spawning duplicate 'az account get-access-token' - // subprocesses when multiple services request the same token. - var cachedToken = await AzCliHelper.AcquireAzCliTokenAsync(resource, tenantId); - if (!string.IsNullOrWhiteSpace(cachedToken)) - { - _logger.LogDebug("Graph API access token acquired from cache"); - return cachedToken; - } - - // Cache miss or az CLI not authenticated — check login state via the injectable resolver. - // Using _loginHintResolver (not the static helper directly) keeps the test seam consistent. - var loginHint = await _loginHintResolver(); - if (loginHint == null) - { - _logger.LogInformation("Azure CLI not authenticated. Initiating login..."); - _logger.LogInformation("A browser window will open for authentication. Please check your taskbar or browser if you don't see it."); - var loginResult = await _executor.ExecuteAsync( - "az", - $"login --tenant {tenantId}", - cancellationToken: ct); - - if (!loginResult.Success) - { - _logger.LogError("Azure CLI login failed"); - return null; - } - - // Bust the caches so the fresh login identity and token are picked up. - AzCliHelper.InvalidateLoginHintCache(); - AzCliHelper.InvalidateAzCliTokenCache(); - } - - // Acquire the token — this goes through AzCliHelper so it is cached for all - // subsequent callers (including those that call AzCliHelper directly). - var tokenResult = await _executor.ExecuteAsync( - "az", - $"account get-access-token --resource {resource} --tenant {tenantId} --query accessToken -o tsv", - captureOutput: true, - cancellationToken: ct); - - if (tokenResult.Success && !string.IsNullOrWhiteSpace(tokenResult.StandardOutput)) - { - var token = tokenResult.StandardOutput.Trim(); - // Warm the shared cache so other services that call AzCliHelper.AcquireAzCliTokenAsync - // directly receive this token without spawning another subprocess. - AzCliHelper.WarmAzCliTokenCache(resource, tenantId, token); - _logger.LogDebug("Graph API access token acquired successfully"); - return token; - } - - // Check for CAE-related errors in the error output - var errorOutput = tokenResult.StandardError ?? ""; - if (errorOutput.Contains("AADSTS50173", StringComparison.OrdinalIgnoreCase) || - errorOutput.Contains("session", StringComparison.OrdinalIgnoreCase) || - errorOutput.Contains("expired", StringComparison.OrdinalIgnoreCase)) - { - _logger.LogWarning("Authentication session may have expired. Attempting fresh login..."); - - // Force logout and re-login - _logger.LogInformation("Logging out of Azure CLI..."); - await _executor.ExecuteAsync("az", "logout", suppressErrorLogging: true, cancellationToken: ct); - - _logger.LogInformation("Initiating fresh login..."); - var freshLoginResult = await _executor.ExecuteAsync( - "az", - $"login --tenant {tenantId}", - cancellationToken: ct); - - if (!freshLoginResult.Success) - { - _logger.LogError("Fresh login failed. Please manually run: az login --tenant {TenantId}", tenantId); - return null; - } - - // Bust caches after re-authentication so stale tokens are not returned. - AzCliHelper.InvalidateLoginHintCache(); - AzCliHelper.InvalidateAzCliTokenCache(); - - // Retry token acquisition - _logger.LogInformation("Retrying token acquisition..."); - var retryTokenResult = await _executor.ExecuteAsync( - "az", - $"account get-access-token --resource {resource} --tenant {tenantId} --query accessToken -o tsv", - captureOutput: true, - cancellationToken: ct); - - if (retryTokenResult.Success && !string.IsNullOrWhiteSpace(retryTokenResult.StandardOutput)) - { - var token = retryTokenResult.StandardOutput.Trim(); - AzCliHelper.WarmAzCliTokenCache(resource, tenantId, token); - _logger.LogInformation("Graph API access token acquired successfully after re-authentication"); - return token; - } - - _logger.LogError("Failed to acquire token after re-authentication: {Error}", retryTokenResult.StandardError); - return null; - } - - _logger.LogError("Failed to acquire Graph API access token: {Error}", tokenResult.StandardError); - return null; - } - catch (Exception ex) - { - _logger.LogError(ex, "Error acquiring Graph API access token"); - - // Check for CAE-related exceptions - if (ex.Message.Contains("TokenIssuedBeforeRevocationTimestamp", StringComparison.OrdinalIgnoreCase) || - ex.Message.Contains("InteractionRequired", StringComparison.OrdinalIgnoreCase)) - { - _logger.LogError(""); - _logger.LogError("=== AUTHENTICATION SESSION EXPIRED ==="); - _logger.LogError("Your authentication session is no longer valid."); - _logger.LogError(""); - _logger.LogError("TO RESOLVE:"); - _logger.LogError(" 1. Run: az logout"); - _logger.LogError(" 2. Run: az login --tenant {TenantId}", tenantId); - _logger.LogError(" 3. Retry your command"); - _logger.LogError(""); - } - - return null; - } - } - - - private async Task EnsureGraphHeadersAsync(string tenantId, CancellationToken ct = default, IEnumerable? scopes = null) - { - // Authentication Strategy: - // 1. If specific scopes required AND token provider configured: Use interactive browser auth (cached) - // 2. Otherwise: Use Azure CLI token (requires 'az login') - // This dual approach minimizes auth prompts while supporting special scope requirements - - string? token; - - if (scopes != null && _tokenProvider != null) - { - // Use token provider with delegated scopes (interactive browser auth with caching) - _logger.LogDebug("Acquiring Graph token with specific scopes via token provider: {Scopes}", string.Join(", ", scopes)); - var loginHint = await ResolveLoginHintAsync(); - token = await _tokenProvider.GetMgGraphAccessTokenAsync(tenantId, scopes, false, CustomClientAppId, ct, loginHint); - - if (string.IsNullOrWhiteSpace(token)) - { - _logger.LogError("Failed to acquire Graph token with scopes: {Scopes}", string.Join(", ", scopes)); - return false; - } - - _logger.LogDebug("Successfully acquired Graph token with specific scopes (cached or new)"); - } - else if (scopes != null && _tokenProvider == null) - { - // Scopes required but no token provider - this is a configuration issue - _logger.LogError("Token provider is not configured, but specific scopes are required: {Scopes}", string.Join(", ", scopes)); - return false; - } - else - { - // Use the process-level token cache in AzCliHelper — shared across all service - // instances so a token acquired in any phase is reused by subsequent phases. - token = await AzCliHelper.AcquireAzCliTokenAsync(GraphApiConstants.GetResource(_graphBaseUrl), tenantId); - - if (string.IsNullOrWhiteSpace(token)) - { - // Cache miss or az CLI not authenticated — run full auth + recovery flow. - _logger.LogDebug("Process-level token cache miss; running full auth flow for tenant {TenantId}", tenantId); - token = await GetGraphAccessTokenAsync(tenantId, ct); - - if (string.IsNullOrWhiteSpace(token)) - { - _logger.LogError("Failed to acquire Graph token via Azure CLI. Ensure 'az login' is completed."); - return false; - } - - // Warm the process-level cache so subsequent callers (including other - // GraphApiService instances and services) skip the auth flow entirely. - AzCliHelper.WarmAzCliTokenCache(GraphApiConstants.GetResource(_graphBaseUrl), tenantId, token); - } - } - - // Remove all newline characters and trim whitespace to prevent header validation errors - token = token.ReplaceLineEndings(string.Empty).Trim(); - - _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); - // NOTE: Do NOT add "ConsistencyLevel: eventual" header here. - // This header is only required for advanced Graph query capabilities ($count, $search, certain $filter operations). - // For simple queries like service principal lookups, this header is not needed and causes HTTP 400 errors. - // See: https://learn.microsoft.com/en-us/graph/aad-advanced-queries - - return true; - } - - /// - /// Returns the object ID of the currently signed-in user via GET /v1.0/me. - /// Replaces 'az ad signed-in-user show --query id -o tsv' (~30s) with a Graph HTTP call (~200ms). - /// Returns null if the call fails (caller should fall back to az CLI). - /// - public virtual async Task GetCurrentUserObjectIdAsync(string tenantId, CancellationToken ct = default) - { - using var doc = await GraphGetAsync(tenantId, "/v1.0/me?$select=id", ct); - if (doc == null) return null; - return doc.RootElement.TryGetProperty("id", out var idEl) ? idEl.GetString() : null; - } - - /// - /// Checks whether a service principal with the given object ID exists in the tenant. - /// Replaces 'az ad sp show --id {principalId}' (~30s) with a Graph HTTP call (~200ms). - /// Used for MSI propagation polling — returns true when the SP is visible in the tenant. - /// - public virtual async Task ServicePrincipalExistsAsync(string tenantId, string principalId, CancellationToken ct = default) - { - using var doc = await GraphGetAsync(tenantId, $"/v1.0/servicePrincipals/{principalId}?$select=id", ct); - return doc != null; - } - - /// - /// Executes a GET request to Microsoft Graph API. - /// Virtual to allow mocking in unit tests using Moq. - /// - public virtual async Task GraphGetAsync(string tenantId, string relativePath, CancellationToken ct = default, IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) return null; - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - using var resp = await _httpClient.GetAsync(url, ct); - if (!resp.IsSuccessStatusCode) - { - var errorBody = await resp.Content.ReadAsStringAsync(ct); - _logger.LogDebug("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, errorBody); - return null; - } - var json = await resp.Content.ReadAsStringAsync(ct); - - return JsonDocument.Parse(json); - } - - /// - /// GET from Graph and always return HTTP response details (status, body, parsed JSON). - /// Use this instead of GraphGetAsync when the caller needs to distinguish auth failures - /// (401) from transient server errors (503, 429, network exceptions). - /// - public virtual async Task GraphGetWithResponseAsync(string tenantId, string relativePath, CancellationToken ct = default, IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) - return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = "NoAuth", Body = "Failed to acquire token" }; - - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - - try - { - using var resp = await _httpClient.GetAsync(url, ct); - var body = await resp.Content.ReadAsStringAsync(ct); - - JsonDocument? json = null; - if (resp.IsSuccessStatusCode && !string.IsNullOrWhiteSpace(body)) - { - try { json = JsonDocument.Parse(body); } catch { /* ignore parse errors */ } - } - - if (!resp.IsSuccessStatusCode) - _logger.LogDebug("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); - - return new GraphResponse - { - IsSuccess = resp.IsSuccessStatusCode, - StatusCode = (int)resp.StatusCode, - ReasonPhrase = resp.ReasonPhrase ?? string.Empty, - Body = body ?? string.Empty, - Json = json - }; - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Graph GET {Url} threw an exception", url); - return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = ex.Message, Body = string.Empty }; - } - } - - public virtual async Task GraphPostAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) return null; - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - using var resp = await _httpClient.PostAsync(url, content, ct); - var body = await resp.Content.ReadAsStringAsync(ct); - if (!resp.IsSuccessStatusCode) - { - var errorMessage = TryExtractGraphErrorMessage(body); - if (errorMessage != null) - _logger.LogWarning("Graph POST {Url} failed: {ErrorMessage}", url, errorMessage); - else - _logger.LogWarning("Graph POST {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); - _logger.LogDebug("Graph POST response body: {Body}", body); - return null; - } - - return string.IsNullOrWhiteSpace(body) ? null : JsonDocument.Parse(body); - } - - /// - /// POST to Graph but always return HTTP response details (status, body, parsed JSON) - /// - public virtual async Task GraphPostWithResponseAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) - { - return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = "NoAuth", Body = "Failed to acquire token" }; - } - - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - - var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - using var resp = await _httpClient.PostAsync(url, content, ct); - var body = await resp.Content.ReadAsStringAsync(ct); - - JsonDocument? json = null; - if (!string.IsNullOrWhiteSpace(body)) - { - try { json = JsonDocument.Parse(body); } catch { /* ignore parse errors */ } - } - - return new GraphResponse - { - IsSuccess = resp.IsSuccessStatusCode, - StatusCode = (int)resp.StatusCode, - ReasonPhrase = resp.ReasonPhrase ?? string.Empty, - Body = body ?? string.Empty, - Json = json - }; - } - - /// - /// Executes a PATCH request to Microsoft Graph API. - /// Virtual to allow mocking in unit tests using Moq. - /// - public virtual async Task GraphPatchAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) return false; - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - using var request = new HttpRequestMessage(new HttpMethod("PATCH"), url) { Content = content }; - using var resp = await _httpClient.SendAsync(request, ct); - - // Many PATCH calls return 204 NoContent on success - if (!resp.IsSuccessStatusCode) - { - var body = await resp.Content.ReadAsStringAsync(ct); - var errorMessage = TryExtractGraphErrorMessage(body); - if (errorMessage != null) - _logger.LogError("Graph PATCH {Url} failed: {ErrorMessage}", url, errorMessage); - else - _logger.LogError("Graph PATCH {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); - _logger.LogDebug("Graph PATCH response body: {Body}", body); - } - - return resp.IsSuccessStatusCode; - } - - public async Task GraphDeleteAsync( - string tenantId, - string relativePath, - CancellationToken ct = default, - bool treatNotFoundAsSuccess = true, - IEnumerable? scopes = null) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) return false; - - var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); - - using var req = new HttpRequestMessage(HttpMethod.Delete, url); - using var resp = await _httpClient.SendAsync(req, ct); - - // 404 can be considered success for idempotent deletes - if (treatNotFoundAsSuccess && (int)resp.StatusCode == 404) return true; - - if (!resp.IsSuccessStatusCode) - { - var body = await resp.Content.ReadAsStringAsync(ct); - var errorMessage = TryExtractGraphErrorMessage(body); - if (errorMessage != null) - _logger.LogError("Graph DELETE {Url} failed: {ErrorMessage}", url, errorMessage); - else - _logger.LogError("Graph DELETE {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); - _logger.LogDebug("Graph DELETE response body: {Body}", body); - return false; - } - - return true; - } - - /// - /// Looks up a service principal by its application (client) ID. - /// Virtual to allow mocking in unit tests using Moq. - /// - 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. - // 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(); - } - - /// - /// Looks up the display name of a service principal by its application ID. - /// Returns null if the service principal is not found. - /// Virtual to allow substitution in unit tests using NSubstitute. - /// - public virtual async Task GetServicePrincipalDisplayNameAsync( - string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) - { - // Validate GUID format to prevent OData injection - if (!Guid.TryParse(appId, out var validGuid)) - { - _logger.LogWarning("Invalid appId format for service principal lookup: {AppId}", appId); - return null; - } - - // Use validated GUID in normalized format to prevent OData injection - using var doc = await GraphGetAsync(tenantId, $"/v1.0/servicePrincipals?$filter=appId eq '{validGuid:D}'&$select=displayName", ct, scopes); - if (doc == null) return null; - if (!doc.RootElement.TryGetProperty("value", out var value) || value.GetArrayLength() == 0) return null; - if (!value[0].TryGetProperty("displayName", out var displayName)) return null; - return displayName.GetString(); - } - - /// - /// Ensures a service principal exists for the given application ID. - /// Creates the service principal if it doesn't already exist. - /// Virtual to allow mocking in unit tests using Moq. - /// - public virtual async Task EnsureServicePrincipalForAppIdAsync( - string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) - { - // Try existing - var spId = await LookupServicePrincipalByAppIdAsync(tenantId, appId, ct, scopes); - if (!string.IsNullOrWhiteSpace(spId)) return spId!; - - // Create SP for this application - var created = await GraphPostAsync(tenantId, "/v1.0/servicePrincipals", new { appId }, ct, scopes); - if (created == null || !created.RootElement.TryGetProperty("id", out var idProp)) - throw new InvalidOperationException($"Failed to create servicePrincipal for appId {appId}"); - - return idProp.GetString()!; - } - - public async Task CreateOrUpdateOauth2PermissionGrantAsync( - string tenantId, - string clientSpObjectId, - string resourceSpObjectId, - IEnumerable scopes, - CancellationToken ct = default, - IEnumerable? permissionGrantScopes = null) - { - var desiredScopeString = string.Join(' ', scopes); - - // Read existing — extract string values immediately so JsonDocument can be disposed - string? existingId = null; - string existingScopes = ""; - - using (var listDoc = await GraphGetAsync( - tenantId, - $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpObjectId}' and resourceId eq '{resourceSpObjectId}'", - ct, - permissionGrantScopes)) - { - if (listDoc?.RootElement.TryGetProperty("value", out var arr) == true && arr.GetArrayLength() > 0) - { - var grant = arr[0]; - existingId = grant.TryGetProperty("id", out var idProp) ? idProp.GetString() : null; - existingScopes = grant.TryGetProperty("scope", out var scopeProp) ? scopeProp.GetString() ?? "" : ""; - } - } - - if (string.IsNullOrWhiteSpace(existingId)) - { - // AllPrincipals (tenant-wide) grants require Global Administrator. - // Only called from admin paths (setup admin or setup all run by GA). - var payload = new - { - clientId = clientSpObjectId, - consentType = "AllPrincipals", - resourceId = resourceSpObjectId, - scope = desiredScopeString - }; - - _logger.LogDebug("Graph POST /v1.0/oauth2PermissionGrants body: {Body}", JsonSerializer.Serialize(payload)); - - // A freshly-created service principal may not yet be visible to the - // oauth2PermissionGrants replica (Directory_ObjectNotFound). Retry with - // exponential back-off so the command is self-healing without user intervention. - const int maxRetries = 8; - const int baseDelaySeconds = 5; - for (int attempt = 0; attempt < maxRetries; attempt++) - { - var grantResponse = await GraphPostWithResponseAsync(tenantId, "/v1.0/oauth2PermissionGrants", payload, ct, permissionGrantScopes); - // Dispose the error JSON immediately — only IsSuccess and Body are needed below. - grantResponse.Json?.Dispose(); - - if (grantResponse.IsSuccess) - return true; - - if (!grantResponse.Body.Contains("Directory_ObjectNotFound", StringComparison.OrdinalIgnoreCase)) - return false; // non-transient error, do not retry - - if (attempt < maxRetries - 1) - { - var delaySecs = (int)Math.Min(baseDelaySeconds * Math.Pow(2, attempt), 60); - _logger.LogWarning( - "Service principal not yet replicated to grants endpoint — retrying in {Delay}s (attempt {Attempt}/{Max})...", - delaySecs, attempt + 1, maxRetries - 1); - await Task.Delay(TimeSpan.FromSeconds(delaySecs), ct); - } - } - - _logger.LogWarning( - "OAuth2 permission grant failed after {MaxRetries} retries — service principal may still be propagating. " + - "Re-run 'a365 setup admin' to retry.", - maxRetries); - return false; - } - - // Merge scopes if needed - var currentSet = new HashSet(existingScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.OrdinalIgnoreCase); - var desiredSet = new HashSet(desiredScopeString.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.OrdinalIgnoreCase); - - if (desiredSet.IsSubsetOf(currentSet)) return true; // already satisfied - - currentSet.UnionWith(desiredSet); - var merged = string.Join(' ', currentSet); - - return await GraphPatchAsync(tenantId, $"/v1.0/oauth2PermissionGrants/{existingId}", new { scope = merged }, ct, permissionGrantScopes); - } - - /// - /// Checks if the current user has sufficient privileges to create service principals. - /// Virtual to allow mocking in unit tests using Moq. - /// - /// The tenant ID - /// Cancellation token - /// True if user has required roles, false otherwise - public virtual async Task<(bool hasPrivileges, List roles)> CheckServicePrincipalCreationPrivilegesAsync( - string tenantId, - CancellationToken ct = default) - { - try - { - _logger.LogDebug("Checking user's directory roles for service principal creation privileges"); - - var token = await GetGraphAccessTokenAsync(tenantId, ct); - if (token == null) - { - _logger.LogWarning("Could not acquire Graph token to check privileges"); - return (false, new List()); - } - - // Trim token to remove any newline characters that may cause header validation errors - token = token.Trim(); - - using var request = new HttpRequestMessage(HttpMethod.Get, - $"{_graphBaseUrl}/v1.0/me/memberOf/microsoft.graph.directoryRole"); - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); - - using var response = await _httpClient.SendAsync(request, ct); - if (!response.IsSuccessStatusCode) - { - _logger.LogWarning("Could not retrieve user's directory roles: {Status}", response.StatusCode); - return (false, new List()); - } - - var json = await response.Content.ReadAsStringAsync(ct); - var doc = JsonDocument.Parse(json); - - var roles = new List(); - if (doc.RootElement.TryGetProperty("value", out var rolesArray)) - { - roles = rolesArray.EnumerateArray() - .Where(role => role.TryGetProperty("displayName", out var displayName)) - .Select(role => role.GetProperty("displayName").GetString()) - .Where(roleName => !string.IsNullOrEmpty(roleName)) - .ToList()!; - } - - _logger.LogDebug("User has {Count} directory roles", roles.Count); - - // Check for required roles - var requiredRoles = new[] - { - "Application Administrator", - "Cloud Application Administrator", - "Global Administrator" - }; - - var hasRequiredRole = roles.Any(r => requiredRoles.Contains(r, StringComparer.OrdinalIgnoreCase)); - - if (hasRequiredRole) - { - _logger.LogDebug("User has sufficient privileges for service principal creation"); - } - else - { - _logger.LogDebug("User does not have required roles for service principal creation. Roles: {Roles}", - string.Join(", ", roles)); - } - - return (hasRequiredRole, roles); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to check service principal creation privileges: {Message}", ex.Message); - return (false, new List()); - } - } - - /// - /// Checks if a user is an owner of an application (read-only validation). - /// Does not attempt to add the user as owner, only verifies ownership. - /// - /// The tenant ID - /// The application object ID (not the client/app ID) - /// The user's object ID to check. If null, uses the current authenticated user. - /// Cancellation token - /// OAuth2 scopes for elevated permissions (e.g., Application.ReadWrite.All, Directory.ReadWrite.All) - /// True if the user is an owner, false otherwise - public virtual async Task IsApplicationOwnerAsync( - string tenantId, - string applicationObjectId, - string? userObjectId = null, - CancellationToken ct = default, - IEnumerable? scopes = null) - { - try - { - // Get current user's object ID if not provided - if (string.IsNullOrWhiteSpace(userObjectId)) - { - if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) - { - _logger.LogWarning("Could not acquire Graph token to check application owner"); - return false; - } - - using var meRequest = new HttpRequestMessage(HttpMethod.Get, - $"{_graphBaseUrl}/v1.0/me?$select=id"); - meRequest.Headers.Authorization = _httpClient.DefaultRequestHeaders.Authorization; - - using var meResponse = await _httpClient.SendAsync(meRequest, ct); - if (!meResponse.IsSuccessStatusCode) - { - _logger.LogWarning("Could not retrieve current user's ID: {Status}", meResponse.StatusCode); - return false; - } - - var meJson = await meResponse.Content.ReadAsStringAsync(ct); - using var meDoc = JsonDocument.Parse(meJson); - - if (!meDoc.RootElement.TryGetProperty("id", out var idElement)) - { - _logger.LogWarning("Could not extract user ID from Graph response"); - return false; - } - - userObjectId = idElement.GetString(); - } - - if (string.IsNullOrWhiteSpace(userObjectId)) - { - _logger.LogWarning("User object ID is empty, cannot check owner"); - return false; - } - - // Check if user is an owner - _logger.LogDebug("Checking if user {UserId} is an owner of application {AppObjectId}", userObjectId, applicationObjectId); - - var ownersDoc = await GraphGetAsync(tenantId, $"/v1.0/applications/{applicationObjectId}/owners?$select=id", ct, scopes); - if (ownersDoc != null && ownersDoc.RootElement.TryGetProperty("value", out var ownersArray)) - { - var isOwner = ownersArray.EnumerateArray() - .Where(owner => owner.TryGetProperty("id", out var ownerId)) - .Any(owner => string.Equals(owner.GetProperty("id").GetString(), userObjectId, StringComparison.OrdinalIgnoreCase)); - - return isOwner; - } - - return false; - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error checking if user is owner of application: {Message}", ex.Message); - return false; - } - } - - /// - /// Checks whether the currently signed-in user holds the Global Administrator role, - /// which is required to grant tenant-wide admin consent interactively. - /// Uses only — works for both admin and non-admin users. - /// Returns (non-blocking) if the check cannot be completed. - /// - public virtual async Task IsCurrentUserAdminAsync( - string tenantId, - CancellationToken ct = default) - { - return await CheckDirectoryRoleAsync(tenantId, AuthenticationConstants.GlobalAdminRoleTemplateId, ct); - } - - /// - /// Checks whether the currently signed-in user holds the Agent ID Administrator role, - /// which is required to create or update inheritable permissions on agent blueprints. - /// Uses only — works for both admin and non-admin users. - /// Returns (non-blocking) if the check cannot be completed. - /// - public virtual async Task IsCurrentUserAgentIdAdminAsync( - string tenantId, - CancellationToken ct = default) - { - return await CheckDirectoryRoleAsync(tenantId, AuthenticationConstants.AgentIdAdminRoleTemplateId, ct); - } - - /// - /// Returns if the role is confirmed active, - /// if confirmed absent, or - /// if the check itself failed (e.g. network error, - /// throttling, auth failure) — in which case the caller should attempt the operation - /// anyway and let the API surface the real error. - /// Queries /me/transitiveMemberOf/microsoft.graph.directoryRole, which requires only - /// User.Read and succeeds for both admin and non-admin users. - /// Note: PIM-eligible-but-not-activated assignments are not considered active. - /// - private async Task CheckDirectoryRoleAsync(string tenantId, string roleTemplateId, CancellationToken ct) - { - try - { - // /me/transitiveMemberOf is a directory query — Directory.Read.All is required. - // User.Read is insufficient and would return Unknown for most users. - IEnumerable? scopes = _tokenProvider != null - ? [AuthenticationConstants.DirectoryReadAllScope] - : null; - - string? nextUrl = "/v1.0/me/transitiveMemberOf/microsoft.graph.directoryRole?$select=roleTemplateId"; - - while (nextUrl != null) - { - using var doc = await GraphGetAsync(tenantId, nextUrl, ct, scopes); - - if (doc == null) - return Models.RoleCheckResult.Unknown; - - if (!doc.RootElement.TryGetProperty("value", out var roles)) - { - _logger.LogWarning("Unexpected Graph response shape — 'value' property missing from transitiveMemberOf response."); - return Models.RoleCheckResult.Unknown; - } - - if (roles.EnumerateArray().Any(r => - r.TryGetProperty("roleTemplateId", out var id) && - string.Equals(id.GetString(), roleTemplateId, StringComparison.OrdinalIgnoreCase))) - return Models.RoleCheckResult.HasRole; - - nextUrl = doc.RootElement.TryGetProperty("@odata.nextLink", out var nextLink) - ? nextLink.GetString() - : null; - } - - return Models.RoleCheckResult.DoesNotHaveRole; - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Role check for {TemplateId} failed — will attempt operation anyway: {Message}", - roleTemplateId, ex.Message); - return Models.RoleCheckResult.Unknown; - } - } - - /// - /// Resolves the Azure CLI login hint once per instance from 'az account show'. - /// The hint is passed to MSAL so that WAM and silent auth target the correct - /// Azure CLI identity instead of the Windows default account. - /// Returns null if az account show fails or the user field is absent. - /// - private async Task ResolveLoginHintAsync() - { - if (_loginHintResolved) - return _loginHint; - - _loginHintResolved = true; - _loginHint = await _loginHintResolver(); - return _loginHint; - } - - /// - /// Attempts to extract a human-readable error message from a Graph API JSON error response body. - /// Returns null if the body cannot be parsed or does not contain an error message. - /// - private static string? TryExtractGraphErrorMessage(string body) - { - if (string.IsNullOrWhiteSpace(body)) return null; - try - { - using var doc = JsonDocument.Parse(body); - if (doc.RootElement.TryGetProperty("error", out var error) && - error.TryGetProperty("message", out var msg)) - { - return msg.GetString(); - } - } - catch { /* ignore parse errors */ } - return null; - } -} +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Service for managing Microsoft Graph API permissions and registrations +/// +public class GraphApiService +{ + private readonly ILogger _logger; + private readonly CommandExecutor _executor; + private readonly HttpClient _httpClient; + private readonly IMicrosoftGraphTokenProvider? _tokenProvider; + private readonly IAuthenticationService _authService; + private readonly RetryHelper _retryHelper; + private string _graphBaseUrl; + + // Login hint resolved once per GraphApiService instance. + // Used to direct MSAL/WAM to the correct identity, preventing the Windows default + // account (WAM) or a stale cached MSAL account from being used instead. + // Resolved from az CLI if available, otherwise from the AuthenticationService token cache. + private string? _loginHint; + private bool _loginHintResolved; + + // Resolver delegate for the login hint. + // Defaults to az CLI first, then AuthenticationService JWT cache as fallback. + // Injectable via constructor so unit tests can bypass the real az process. + private readonly Func> _loginHintResolver; + + /// + /// Optional custom client app ID to use for authentication with Microsoft Graph PowerShell. + /// When set, this will be passed to Connect-MgGraph -ClientId parameter. + /// + public string? CustomClientAppId { get; set; } + + /// + /// Override the Microsoft Graph base URL for sovereign / government cloud tenants. + /// Defaults to (commercial cloud). + /// Set this after construction when the config is available (e.g. from Agent365Config.GraphBaseUrl). + /// + public string GraphBaseUrl + { + get => _graphBaseUrl; + set => _graphBaseUrl = string.IsNullOrWhiteSpace(value) ? GraphApiConstants.BaseUrl : value; + } + + // Lightweight wrapper to surface HTTP status, reason and body to callers + public record GraphResponse + { + public bool IsSuccess { get; init; } + public int StatusCode { get; init; } + public string ReasonPhrase { get; init; } = string.Empty; + public string Body { get; init; } = string.Empty; + public JsonDocument? Json { get; init; } + } + + // Allow injecting a custom HttpMessageHandler for unit testing. + // loginHintResolver: optional override for login-hint resolution. + // Pass () => Task.FromResult(null) in unit tests to skip login-hint resolution. + public GraphApiService(ILogger logger, CommandExecutor executor, IAuthenticationService authService, HttpMessageHandler? handler = null, IMicrosoftGraphTokenProvider? tokenProvider = null, Func>? loginHintResolver = null, string? graphBaseUrl = null, RetryHelper? retryHelper = null) + { + _logger = logger; + _executor = executor; + _authService = authService; + _httpClient = handler != null ? new HttpClient(handler) : HttpClientFactory.CreateAuthenticatedClient(); + _tokenProvider = tokenProvider; + _retryHelper = retryHelper ?? new RetryHelper(_logger); + // Default: try az CLI first (if present), fall back to JWT cache in AuthenticationService. + _loginHintResolver = loginHintResolver ?? (() => ResolveLoginHintWithFallbackAsync(authService)); + _graphBaseUrl = string.IsNullOrWhiteSpace(graphBaseUrl) ? GraphApiConstants.BaseUrl : graphBaseUrl; + } + + // Parameterless constructor to ease test mocking/substitution frameworks which may + // require creating proxy instances without providing constructor arguments. + public GraphApiService() + : this(NullLogger.Instance, new CommandExecutor(NullLogger.Instance), new AuthenticationService(NullLogger.Instance), null, null, null) + { + } + + // Convenience constructors for tests. Castle DynamicProxy (used by NSubstitute) resolves + // constructors by exact argument count — it does not handle C# optional parameters. + // Both forms must exist as separate constructors so Castle can proxy either call site. + // + // 2-arg form: used by tests that do not need loginHintResolver control. + public GraphApiService(ILogger logger, CommandExecutor executor) + : this(logger ?? NullLogger.Instance, executor ?? throw new ArgumentNullException(nameof(executor)), new AuthenticationService(NullLogger.Instance), null, null, null) + { + } + + // 3-arg form: used by partial-mock tests that inject loginHintResolver to prevent + // a real az account show subprocess. Pass () => Task.FromResult(null). + public GraphApiService(ILogger logger, CommandExecutor executor, Func>? loginHintResolver) + : this(logger ?? NullLogger.Instance, executor ?? throw new ArgumentNullException(nameof(executor)), new AuthenticationService(NullLogger.Instance), null, null, loginHintResolver) + { + } + + private static async Task ResolveLoginHintWithFallbackAsync(IAuthenticationService authService) + { + // Try az CLI first — most reliable when the user has run 'az login'. + var hint = await AzCliHelper.ResolveLoginHintAsync(); + if (!string.IsNullOrWhiteSpace(hint)) + return hint; + // Fall back to the UPN embedded in a previously cached MSAL JWT. + return await authService.ResolveLoginHintFromCacheAsync(); + } + + /// + /// Acquires an access token for Microsoft Graph API via MSAL (WAM on Windows, + /// browser/device-code on macOS/Linux). Token is cached persistently by + /// AuthenticationService — no az CLI subprocess involved. + /// + public virtual async Task GetGraphAccessTokenAsync(string tenantId, bool forceRefresh = false, CancellationToken ct = default) + { + _logger.LogDebug("Acquiring Graph API access token for tenant {TenantId}", tenantId); + try + { + var resource = GraphApiConstants.GetResource(_graphBaseUrl); + var loginHint = await _loginHintResolver(); + var token = await _authService.GetAccessTokenAsync(resource, tenantId, forceRefresh: forceRefresh, userId: loginHint); + if (!string.IsNullOrWhiteSpace(token)) + { + _logger.LogDebug("Graph API access token acquired successfully"); + return token; + } + _logger.LogError("Failed to acquire Graph API access token for tenant {TenantId}", tenantId); + return null; + } + catch (Exception ex) + { + _logger.LogError(ex, "Error acquiring Graph API access token"); + return null; + } + } + + + private async Task EnsureGraphHeadersAsync(string tenantId, bool forceRefresh = false, IEnumerable? scopes = null, CancellationToken ct = default) + { + // Authentication Strategy: + // 1. If specific scopes required AND token provider configured: Use MSAL with delegated scopes (WAM/browser/device-code) + // 2. Otherwise: Use MSAL via AuthenticationService (WAM/browser/device-code, persistent cache) + // All paths go through MSAL — no az CLI subprocess involved. + + string? token; + + if (scopes != null && _tokenProvider != null) + { + // Use token provider with delegated scopes (interactive browser auth with caching) + _logger.LogDebug("Acquiring Graph token with specific scopes via token provider: {Scopes}", string.Join(", ", scopes)); + var loginHint = await ResolveLoginHintAsync(); + token = await _tokenProvider.GetMgGraphAccessTokenAsync(tenantId, scopes, false, CustomClientAppId, ct, loginHint, forceRefresh); + + if (string.IsNullOrWhiteSpace(token)) + { + _logger.LogError("Failed to acquire Graph token with scopes: {Scopes}", string.Join(", ", scopes)); + return false; + } + + _logger.LogDebug("Successfully acquired Graph token with specific scopes (cached or new)"); + } + else if (scopes != null && _tokenProvider == null) + { + // Scopes required but no token provider - this is a configuration issue + _logger.LogError("Token provider is not configured, but specific scopes are required: {Scopes}", string.Join(", ", scopes)); + return false; + } + else + { + // Default path: acquire via AuthenticationService (MSAL, persistent disk cache). + token = await GetGraphAccessTokenAsync(tenantId, forceRefresh: forceRefresh, ct: ct); + + if (string.IsNullOrWhiteSpace(token)) + { + _logger.LogError("Failed to acquire Graph token. Sign-in will be prompted on the next attempt."); + return false; + } + } + + // Remove all newline characters and trim whitespace to prevent header validation errors + token = token.ReplaceLineEndings(string.Empty).Trim(); + + _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + // NOTE: Do NOT add "ConsistencyLevel: eventual" header here. + // This header is only required for advanced Graph query capabilities ($count, $search, certain $filter operations). + // For simple queries like service principal lookups, this header is not needed and causes HTTP 400 errors. + // See: https://learn.microsoft.com/en-us/graph/aad-advanced-queries + + return true; + } + + /// + /// Returns the object ID of the currently signed-in user via GET /v1.0/me. + /// Replaces 'az ad signed-in-user show --query id -o tsv' (~30s) with a Graph HTTP call (~200ms). + /// Returns null if the call fails (caller should fall back to az CLI). + /// + public virtual async Task GetCurrentUserObjectIdAsync(string tenantId, CancellationToken ct = default) + { + using var doc = await GraphGetAsync(tenantId, "/v1.0/me?$select=id", ct); + if (doc == null) return null; + return doc.RootElement.TryGetProperty("id", out var idEl) ? idEl.GetString() : null; + } + + /// + /// Checks whether a service principal with the given object ID exists in the tenant. + /// Replaces 'az ad sp show --id {principalId}' (~30s) with a Graph HTTP call (~200ms). + /// Used for MSI propagation polling — returns true when the SP is visible in the tenant. + /// + public virtual async Task ServicePrincipalExistsAsync(string tenantId, string principalId, CancellationToken ct = default) + { + using var doc = await GraphGetAsync(tenantId, $"/v1.0/servicePrincipals/{principalId}?$select=id", ct); + return doc != null; + } + + /// + /// Executes a GET request to Microsoft Graph API. + /// Virtual to allow mocking in unit tests using Moq. + /// + public virtual async Task GraphGetAsync(string tenantId, string relativePath, CancellationToken ct = default, IEnumerable? scopes = null) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) return null; + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + try + { + using var resp = await _retryHelper.ExecuteWithRetryAsync( + ct => _httpClient.GetAsync(url, ct), cancellationToken: ct); + if (!resp.IsSuccessStatusCode) + { + var errorBody = await resp.Content.ReadAsStringAsync(ct); + _logger.LogDebug("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, errorBody); + return null; + } + var json = await resp.Content.ReadAsStringAsync(ct); + return JsonDocument.Parse(json); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph GET {Url} failed with exception", url); + return null; + } + } + + /// + /// GET from Graph and always return HTTP response details (status, body, parsed JSON). + /// Use this instead of GraphGetAsync when the caller needs to distinguish auth failures + /// (401) from transient server errors (503, 429, network exceptions). + /// + public virtual async Task GraphGetWithResponseAsync(string tenantId, string relativePath, bool forceRefresh = false, IEnumerable? scopes = null, CancellationToken ct = default) + { + if (!await EnsureGraphHeadersAsync(tenantId, forceRefresh: forceRefresh, scopes: scopes, ct: ct)) + return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = "NoAuth", Body = "Failed to acquire token" }; + + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + + try + { + return await _retryHelper.ExecuteWithRetryAsync(async (token) => + { + using var resp = await _httpClient.GetAsync(url, token); + var body = await resp.Content.ReadAsStringAsync(token); + + JsonDocument? json = null; + if (resp.IsSuccessStatusCode && !string.IsNullOrWhiteSpace(body)) + { + try { json = JsonDocument.Parse(body); } catch { /* ignore parse errors */ } + } + + if (!resp.IsSuccessStatusCode) + _logger.LogDebug("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); + + return new GraphResponse + { + IsSuccess = resp.IsSuccessStatusCode, + StatusCode = (int)resp.StatusCode, + ReasonPhrase = resp.ReasonPhrase ?? string.Empty, + Body = body ?? string.Empty, + Json = json + }; + }, cancellationToken: ct); + } + catch (Exception ex) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph GET {Url} threw an exception", url); + return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = ex.Message, Body = string.Empty }; + } + } + + public virtual async Task GraphPostAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) return null; + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + using var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); + try + { + using var resp = await _httpClient.PostAsync(url, content, ct); + var body = await resp.Content.ReadAsStringAsync(ct); + if (!resp.IsSuccessStatusCode) + { + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogWarning("Graph POST {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogWarning("Graph POST {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph POST response body: {Body}", body); + return null; + } + + return string.IsNullOrWhiteSpace(body) ? null : JsonDocument.Parse(body); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph POST {Url} failed with exception", url); + throw; + } + } + + /// + /// POST to Graph but always return HTTP response details (status, body, parsed JSON) + /// + public virtual async Task GraphPostWithResponseAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) + { + return new GraphResponse { IsSuccess = false, StatusCode = 0, ReasonPhrase = "NoAuth", Body = "Failed to acquire token" }; + } + + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + + using var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); + try + { + using var resp = await _httpClient.PostAsync(url, content, ct); + var body = await resp.Content.ReadAsStringAsync(ct); + + JsonDocument? json = null; + if (!string.IsNullOrWhiteSpace(body)) + { + try { json = JsonDocument.Parse(body); } catch { /* ignore parse errors */ } + } + + return new GraphResponse + { + IsSuccess = resp.IsSuccessStatusCode, + StatusCode = (int)resp.StatusCode, + ReasonPhrase = resp.ReasonPhrase ?? string.Empty, + Body = body ?? string.Empty, + Json = json + }; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph POST {Url} failed with exception", url); + throw; + } + } + + /// + /// Executes a PATCH request to Microsoft Graph API. + /// Virtual to allow mocking in unit tests using Moq. + /// + public virtual async Task GraphPatchAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) return false; + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); + try + { + using var request = new HttpRequestMessage(new HttpMethod("PATCH"), url) { Content = content }; + using var resp = await _httpClient.SendAsync(request, ct); + + // Many PATCH calls return 204 NoContent on success + if (!resp.IsSuccessStatusCode) + { + var body = await resp.Content.ReadAsStringAsync(ct); + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogError("Graph PATCH {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogError("Graph PATCH {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph PATCH response body: {Body}", body); + } + + return resp.IsSuccessStatusCode; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph PATCH {Url} failed with exception", url); + throw; + } + } + + public async Task GraphDeleteAsync( + string tenantId, + string relativePath, + CancellationToken ct = default, + bool treatNotFoundAsSuccess = true, + IEnumerable? scopes = null) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) return false; + + var url = GraphApiConstants.BuildUrl(_graphBaseUrl, relativePath); + + try + { + using var req = new HttpRequestMessage(HttpMethod.Delete, url); + using var resp = await _httpClient.SendAsync(req, ct); + + // 404 can be considered success for idempotent deletes + if (treatNotFoundAsSuccess && (int)resp.StatusCode == 404) return true; + + if (!resp.IsSuccessStatusCode) + { + var body = await resp.Content.ReadAsStringAsync(ct); + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogError("Graph DELETE {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogError("Graph DELETE {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph DELETE response body: {Body}", body); + return false; + } + + return true; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogDebug(ex, "Graph DELETE {Url} failed with exception", url); + throw; + } + } + + /// + /// Looks up a service principal by its application (client) ID. + /// Virtual to allow mocking in unit tests using Moq. + /// + 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. + // 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(); + } + + /// + /// Looks up the display name of a service principal by its application ID. + /// Returns null if the service principal is not found. + /// Virtual to allow substitution in unit tests using NSubstitute. + /// + public virtual async Task GetServicePrincipalDisplayNameAsync( + string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) + { + // Validate GUID format to prevent OData injection + if (!Guid.TryParse(appId, out var validGuid)) + { + _logger.LogWarning("Invalid appId format for service principal lookup: {AppId}", appId); + return null; + } + + // Use validated GUID in normalized format to prevent OData injection + using var doc = await GraphGetAsync(tenantId, $"/v1.0/servicePrincipals?$filter=appId eq '{validGuid:D}'&$select=displayName", ct, scopes); + if (doc == null) return null; + if (!doc.RootElement.TryGetProperty("value", out var value) || value.GetArrayLength() == 0) return null; + if (!value[0].TryGetProperty("displayName", out var displayName)) return null; + return displayName.GetString(); + } + + /// + /// Ensures a service principal exists for the given application ID. + /// Creates the service principal if it doesn't already exist. + /// Virtual to allow mocking in unit tests using Moq. + /// + public virtual async Task EnsureServicePrincipalForAppIdAsync( + string tenantId, string appId, CancellationToken ct = default, IEnumerable? scopes = null) + { + // Try existing + var spId = await LookupServicePrincipalByAppIdAsync(tenantId, appId, ct, scopes); + if (!string.IsNullOrWhiteSpace(spId)) return spId!; + + // Create SP for this application + var created = await GraphPostAsync(tenantId, "/v1.0/servicePrincipals", new { appId }, ct, scopes); + if (created == null || !created.RootElement.TryGetProperty("id", out var idProp)) + throw new InvalidOperationException($"Failed to create servicePrincipal for appId {appId}"); + + return idProp.GetString()!; + } + + public async Task CreateOrUpdateOauth2PermissionGrantAsync( + string tenantId, + string clientSpObjectId, + string resourceSpObjectId, + IEnumerable scopes, + CancellationToken ct = default, + IEnumerable? permissionGrantScopes = null) + { + var desiredScopeString = string.Join(' ', scopes); + + // Read existing — extract string values immediately so JsonDocument can be disposed + string? existingId = null; + string existingScopes = ""; + + using (var listDoc = await GraphGetAsync( + tenantId, + $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpObjectId}' and resourceId eq '{resourceSpObjectId}'", + ct, + permissionGrantScopes)) + { + if (listDoc?.RootElement.TryGetProperty("value", out var arr) == true && arr.GetArrayLength() > 0) + { + var grant = arr[0]; + existingId = grant.TryGetProperty("id", out var idProp) ? idProp.GetString() : null; + existingScopes = grant.TryGetProperty("scope", out var scopeProp) ? scopeProp.GetString() ?? "" : ""; + } + } + + if (string.IsNullOrWhiteSpace(existingId)) + { + // AllPrincipals (tenant-wide) grants require Global Administrator. + // Only called from admin paths (setup admin or setup all run by GA). + var payload = new + { + clientId = clientSpObjectId, + consentType = "AllPrincipals", + resourceId = resourceSpObjectId, + scope = desiredScopeString + }; + + _logger.LogDebug("Graph POST /v1.0/oauth2PermissionGrants body: {Body}", JsonSerializer.Serialize(payload)); + + // A freshly-created service principal may not yet be visible to the + // oauth2PermissionGrants replica (Directory_ObjectNotFound). Retry with + // exponential back-off so the command is self-healing without user intervention. + const int maxRetries = 8; + const int baseDelaySeconds = 5; + for (int attempt = 0; attempt < maxRetries; attempt++) + { + var grantResponse = await GraphPostWithResponseAsync(tenantId, "/v1.0/oauth2PermissionGrants", payload, ct, permissionGrantScopes); + // Dispose the error JSON immediately — only IsSuccess and Body are needed below. + grantResponse.Json?.Dispose(); + + if (grantResponse.IsSuccess) + return true; + + if (!grantResponse.Body.Contains("Directory_ObjectNotFound", StringComparison.OrdinalIgnoreCase)) + return false; // non-transient error, do not retry + + if (attempt < maxRetries - 1) + { + var delaySecs = (int)Math.Min(baseDelaySeconds * Math.Pow(2, attempt), 60); + _logger.LogWarning( + "Service principal not yet replicated to grants endpoint — retrying in {Delay}s (attempt {Attempt}/{Max})...", + delaySecs, attempt + 1, maxRetries - 1); + await Task.Delay(TimeSpan.FromSeconds(delaySecs), ct); + } + } + + _logger.LogWarning( + "OAuth2 permission grant failed after {MaxRetries} retries — service principal may still be propagating. " + + "Re-run 'a365 setup admin' to retry.", + maxRetries); + return false; + } + + // Merge scopes if needed + var currentSet = new HashSet(existingScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.OrdinalIgnoreCase); + var desiredSet = new HashSet(desiredScopeString.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.OrdinalIgnoreCase); + + if (desiredSet.IsSubsetOf(currentSet)) return true; // already satisfied + + currentSet.UnionWith(desiredSet); + var merged = string.Join(' ', currentSet); + + return await GraphPatchAsync(tenantId, $"/v1.0/oauth2PermissionGrants/{existingId}", new { scope = merged }, ct, permissionGrantScopes); + } + + /// + /// Checks if the current user has sufficient privileges to create service principals. + /// Virtual to allow mocking in unit tests using Moq. + /// + /// The tenant ID + /// Cancellation token + /// True if user has required roles, false otherwise + public virtual async Task<(bool hasPrivileges, List roles)> CheckServicePrincipalCreationPrivilegesAsync( + string tenantId, + CancellationToken ct = default) + { + try + { + _logger.LogDebug("Checking user's directory roles for service principal creation privileges"); + + var token = await GetGraphAccessTokenAsync(tenantId, ct: ct); + if (token == null) + { + _logger.LogWarning("Could not acquire Graph token to check privileges"); + return (false, new List()); + } + + // Trim token to remove any newline characters that may cause header validation errors + token = token.Trim(); + + using var request = new HttpRequestMessage(HttpMethod.Get, + $"{_graphBaseUrl}/v1.0/me/memberOf/microsoft.graph.directoryRole"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + + using var response = await _httpClient.SendAsync(request, ct); + if (!response.IsSuccessStatusCode) + { + _logger.LogWarning("Could not retrieve user's directory roles: {Status}", response.StatusCode); + return (false, new List()); + } + + var json = await response.Content.ReadAsStringAsync(ct); + var doc = JsonDocument.Parse(json); + + var roles = new List(); + if (doc.RootElement.TryGetProperty("value", out var rolesArray)) + { + roles = rolesArray.EnumerateArray() + .Where(role => role.TryGetProperty("displayName", out var displayName)) + .Select(role => role.GetProperty("displayName").GetString()) + .Where(roleName => !string.IsNullOrEmpty(roleName)) + .ToList()!; + } + + _logger.LogDebug("User has {Count} directory roles", roles.Count); + + // Check for required roles + var requiredRoles = new[] + { + "Application Administrator", + "Cloud Application Administrator", + "Global Administrator" + }; + + var hasRequiredRole = roles.Any(r => requiredRoles.Contains(r, StringComparer.OrdinalIgnoreCase)); + + if (hasRequiredRole) + { + _logger.LogDebug("User has sufficient privileges for service principal creation"); + } + else + { + _logger.LogDebug("User does not have required roles for service principal creation. Roles: {Roles}", + string.Join(", ", roles)); + } + + return (hasRequiredRole, roles); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to check service principal creation privileges: {Message}", ex.Message); + return (false, new List()); + } + } + + /// + /// Checks if a user is an owner of an application (read-only validation). + /// Does not attempt to add the user as owner, only verifies ownership. + /// + /// The tenant ID + /// The application object ID (not the client/app ID) + /// The user's object ID to check. If null, uses the current authenticated user. + /// Cancellation token + /// OAuth2 scopes for elevated permissions (e.g., Application.ReadWrite.All, Directory.ReadWrite.All) + /// True if the user is an owner, false otherwise + public virtual async Task IsApplicationOwnerAsync( + string tenantId, + string applicationObjectId, + string? userObjectId = null, + CancellationToken ct = default, + IEnumerable? scopes = null) + { + try + { + // Get current user's object ID if not provided + if (string.IsNullOrWhiteSpace(userObjectId)) + { + if (!await EnsureGraphHeadersAsync(tenantId, scopes: scopes, ct: ct)) + { + _logger.LogWarning("Could not acquire Graph token to check application owner"); + return false; + } + + using var meRequest = new HttpRequestMessage(HttpMethod.Get, + $"{_graphBaseUrl}/v1.0/me?$select=id"); + meRequest.Headers.Authorization = _httpClient.DefaultRequestHeaders.Authorization; + + using var meResponse = await _httpClient.SendAsync(meRequest, ct); + if (!meResponse.IsSuccessStatusCode) + { + _logger.LogWarning("Could not retrieve current user's ID: {Status}", meResponse.StatusCode); + return false; + } + + var meJson = await meResponse.Content.ReadAsStringAsync(ct); + using var meDoc = JsonDocument.Parse(meJson); + + if (!meDoc.RootElement.TryGetProperty("id", out var idElement)) + { + _logger.LogWarning("Could not extract user ID from Graph response"); + return false; + } + + userObjectId = idElement.GetString(); + } + + if (string.IsNullOrWhiteSpace(userObjectId)) + { + _logger.LogWarning("User object ID is empty, cannot check owner"); + return false; + } + + // Check if user is an owner + _logger.LogDebug("Checking if user {UserId} is an owner of application {AppObjectId}", userObjectId, applicationObjectId); + + var ownersDoc = await GraphGetAsync(tenantId, $"/v1.0/applications/{applicationObjectId}/owners?$select=id", ct, scopes); + if (ownersDoc != null && ownersDoc.RootElement.TryGetProperty("value", out var ownersArray)) + { + var isOwner = ownersArray.EnumerateArray() + .Where(owner => owner.TryGetProperty("id", out var ownerId)) + .Any(owner => string.Equals(owner.GetProperty("id").GetString(), userObjectId, StringComparison.OrdinalIgnoreCase)); + + return isOwner; + } + + return false; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error checking if user is owner of application: {Message}", ex.Message); + return false; + } + } + + /// + /// Checks whether the currently signed-in user holds the Global Administrator role, + /// which is required to grant tenant-wide admin consent interactively. + /// Uses only — works for both admin and non-admin users. + /// Returns (non-blocking) if the check cannot be completed. + /// + public virtual async Task IsCurrentUserAdminAsync( + string tenantId, + CancellationToken ct = default) + { + return await CheckDirectoryRoleAsync(tenantId, AuthenticationConstants.GlobalAdminRoleTemplateId, ct); + } + + /// + /// Checks whether the currently signed-in user holds the Agent ID Administrator role, + /// which is required to create or update inheritable permissions on agent blueprints. + /// Uses only — works for both admin and non-admin users. + /// Returns (non-blocking) if the check cannot be completed. + /// + public virtual async Task IsCurrentUserAgentIdAdminAsync( + string tenantId, + CancellationToken ct = default) + { + return await CheckDirectoryRoleAsync(tenantId, AuthenticationConstants.AgentIdAdminRoleTemplateId, ct); + } + + /// + /// Returns if the role is confirmed active, + /// if confirmed absent, or + /// if the check itself failed (e.g. network error, + /// throttling, auth failure) — in which case the caller should attempt the operation + /// anyway and let the API surface the real error. + /// Queries /me/transitiveMemberOf/microsoft.graph.directoryRole, which requires only + /// User.Read and succeeds for both admin and non-admin users. + /// Note: PIM-eligible-but-not-activated assignments are not considered active. + /// + private async Task CheckDirectoryRoleAsync(string tenantId, string roleTemplateId, CancellationToken ct) + { + try + { + // /me/transitiveMemberOf is a directory query — Directory.Read.All is required. + // User.Read is insufficient and would return Unknown for most users. + IEnumerable? scopes = _tokenProvider != null + ? [AuthenticationConstants.DirectoryReadAllScope] + : null; + + string? nextUrl = "/v1.0/me/transitiveMemberOf/microsoft.graph.directoryRole?$select=roleTemplateId"; + + while (nextUrl != null) + { + using var doc = await GraphGetAsync(tenantId, nextUrl, ct, scopes); + + if (doc == null) + return Models.RoleCheckResult.Unknown; + + if (!doc.RootElement.TryGetProperty("value", out var roles)) + { + _logger.LogWarning("Unexpected Graph response shape — 'value' property missing from transitiveMemberOf response."); + return Models.RoleCheckResult.Unknown; + } + + if (roles.EnumerateArray().Any(r => + r.TryGetProperty("roleTemplateId", out var id) && + string.Equals(id.GetString(), roleTemplateId, StringComparison.OrdinalIgnoreCase))) + return Models.RoleCheckResult.HasRole; + + nextUrl = doc.RootElement.TryGetProperty("@odata.nextLink", out var nextLink) + ? nextLink.GetString() + : null; + } + + return Models.RoleCheckResult.DoesNotHaveRole; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Role check for {TemplateId} failed — will attempt operation anyway: {Message}", + roleTemplateId, ex.Message); + return Models.RoleCheckResult.Unknown; + } + } + + /// + /// Resolves the Azure CLI login hint once per instance from 'az account show'. + /// The hint is passed to MSAL so that WAM and silent auth target the correct + /// Azure CLI identity instead of the Windows default account. + /// Returns null if az account show fails or the user field is absent. + /// + private async Task ResolveLoginHintAsync() + { + if (_loginHintResolved) + return _loginHint; + + _loginHintResolved = true; + _loginHint = await _loginHintResolver(); + return _loginHint; + } + + /// + /// Attempts to extract a human-readable error message from a Graph API JSON error response body. + /// Returns null if the body cannot be parsed or does not contain an error message. + /// + private static string? TryExtractGraphErrorMessage(string body) + { + if (string.IsNullOrWhiteSpace(body)) return null; + try + { + using var doc = JsonDocument.Parse(body); + if (doc.RootElement.TryGetProperty("error", out var error) && + error.TryGetProperty("message", out var msg)) + { + return msg.GetString(); + } + } + catch { /* ignore parse errors */ } + return null; + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AzCliHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AzCliHelper.cs index f1f93439..da519e7c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AzCliHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AzCliHelper.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; -using System.Collections.Concurrent; using System.Diagnostics; using System.Runtime.InteropServices; using System.Text.Json; @@ -11,12 +10,14 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; /// /// Shared helper for invoking the Azure CLI and parsing its output. -/// Consolidates az CLI interactions to ensure consistent behavior across services. +/// Used for login-hint resolution (az account show) and az CLI command execution. +/// Token acquisition has been moved to AuthenticationService (MSAL/WAM) — this +/// helper no longer acquires tokens directly. /// internal static class AzCliHelper { // Process-level cache: 'az account show' returns the same user for the lifetime of a CLI - // invocation. Caching eliminates repeated 20-40s subprocess calls that occur when multiple + // invocation. Caching eliminates repeated subprocess calls that occur when multiple // services and commands each call ResolveLoginHintAsync independently. private static volatile Task? _cachedLoginHintTask; @@ -41,119 +42,6 @@ internal static class AzCliHelper /// Clears the login-hint process-level cache. For use in tests only. internal static void ResetLoginHintCacheForTesting() => _cachedLoginHintTask = null; - // ------------------------------------------------------------------------- - // az account get-access-token — process-level token cache - // ------------------------------------------------------------------------- - // Tokens acquired via 'az account get-access-token' are valid for 60+ minutes. - // Caching at the process level means a single CLI invocation only spawns one - // subprocess per (resource, tenantId) pair, regardless of how many services or - // commands request the same token. Expected savings: 40–60s per command run. - - private static readonly ConcurrentDictionary> _azCliTokenCache = new(); - - // Test seam: replace the underlying acquirer without touching the cache layer. - // The override is invoked inside GetOrAdd, so the result is still cached after - // the first call — only one invocation per cache key, even in tests. - internal static Func>? AzCliTokenAcquirerOverride { get; set; } - - /// - /// Acquires an Azure CLI access token for the given resource and tenant. - /// The result is cached for the process lifetime — a single CLI command cannot - /// invalidate a token except through explicit re-authentication (az login). - /// Call after 'az login' to bust the cache. - /// - internal static Task AcquireAzCliTokenAsync(string resource, string tenantId = "", CancellationToken ct = default) - { - var key = $"{resource}::{tenantId}"; - return _azCliTokenCache.GetOrAdd(key, _ => - AzCliTokenAcquirerOverride != null - ? AzCliTokenAcquirerOverride(resource, tenantId) - : AcquireAzCliTokenCoreAsync(resource, tenantId, ct)); - } - - /// - /// Injects a token acquired via an alternative auth flow (e.g., after 'az login' recovery) - /// so that subsequent callers across all services receive the fresh token from cache. - /// - internal static void WarmAzCliTokenCache(string resource, string tenantId, string token) - { - var key = $"{resource}::{tenantId}"; - _azCliTokenCache[key] = Task.FromResult(token); - } - - /// - /// Clears the token cache. Call after 'az login' or 'az logout' to ensure - /// subsequent callers acquire a fresh token rather than a now-invalid cached one. - /// - internal static void InvalidateAzCliTokenCache() => _azCliTokenCache.Clear(); - - /// Clears the token cache. For use in tests only. - internal static void ResetAzCliTokenCacheForTesting() => _azCliTokenCache.Clear(); - - private static async Task AcquireAzCliTokenCoreAsync(string resource, string tenantId, CancellationToken ct = default) - { - Process? process = null; - try - { - // On Windows az is az.cmd which requires cmd.exe to launch. On all platforms - // arguments are passed via ArgumentList (not string interpolation) so - // resource/tenantId values cannot alter the command line. - var isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); - var startInfo = new ProcessStartInfo - { - FileName = isWindows ? "cmd.exe" : "az", - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true - }; - // On Windows: cmd.exe /c az . On other platforms: az . - if (isWindows) - { - startInfo.ArgumentList.Add("/c"); - startInfo.ArgumentList.Add("az"); - } - startInfo.ArgumentList.Add("account"); - startInfo.ArgumentList.Add("get-access-token"); - startInfo.ArgumentList.Add("--resource"); - startInfo.ArgumentList.Add(resource); - if (!string.IsNullOrEmpty(tenantId)) - { - startInfo.ArgumentList.Add("--tenant"); - startInfo.ArgumentList.Add(tenantId); - } - startInfo.ArgumentList.Add("--query"); - startInfo.ArgumentList.Add("accessToken"); - startInfo.ArgumentList.Add("-o"); - startInfo.ArgumentList.Add("tsv"); - - process = Process.Start(startInfo); - if (process == null) return null; - // Start reads concurrently so the pipe buffers never fill up and block the process. - // WaitForExitAsync(ct) is awaited first so cancellation is observed immediately; - // the reads complete naturally once the process exits and the pipes close. - var outputTask = process.StandardOutput.ReadToEndAsync(); - var errorTask = process.StandardError.ReadToEndAsync(); - await process.WaitForExitAsync(ct); - await Task.WhenAll(outputTask, errorTask); - var output = outputTask.Result.Trim(); - return process.ExitCode == 0 && !string.IsNullOrWhiteSpace(output) ? output : null; - } - catch (OperationCanceledException) - { - try { process?.Kill(entireProcessTree: true); } catch { } - throw; - } - catch - { - return null; - } - finally - { - process?.Dispose(); - } - } - private static async Task ResolveLoginHintCoreAsync() { try diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/NetworkHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/NetworkHelper.cs new file mode 100644 index 00000000..95b86259 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/NetworkHelper.cs @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Net.Sockets; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; + +/// +/// Shared network diagnostics helpers used by ArmApiService and GraphApiService. +/// +internal static class NetworkHelper +{ + /// + /// Returns true when the exception chain contains a SocketException with ConnectionReset (10054). + /// This error occurs on corporate networks where a TLS inspection proxy (Zscaler, Netskope, etc.) + /// drops long-running connections. Logging a targeted warning helps users self-diagnose and retry. + /// + internal static bool IsConnectionResetByProxy(Exception ex) + { + var inner = ex.InnerException; + while (inner != null) + { + if (inner is SocketException se && se.SocketErrorCode == SocketError.ConnectionReset) + return true; + inner = inner.InnerException; + } + return ex is SocketException root && root.SocketErrorCode == SocketError.ConnectionReset; + } + + internal const string ConnectionResetWarning = + "Network connection was reset by a remote host (SocketError 10054). " + + "This often occurs on corporate networks with TLS inspection proxies. " + + "Re-running the command usually succeeds."; +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/RetryHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/RetryHelper.cs index 9979790b..eab9e917 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/RetryHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/RetryHelper.cs @@ -12,10 +12,14 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; public class RetryHelper { private readonly ILogger _logger; + private readonly int _maxRetries; + private readonly int _baseDelaySeconds; - public RetryHelper(ILogger logger) + public RetryHelper(ILogger logger, int maxRetries = 3, int baseDelaySeconds = 1) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _maxRetries = maxRetries; + _baseDelaySeconds = baseDelaySeconds; } /// @@ -24,8 +28,8 @@ public RetryHelper(ILogger logger) /// Return type of the operation /// The async operation to execute. Receives a cancellation token and returns a result. /// Predicate that determines if retry is needed. Returns TRUE when the operation should be retried (operation failed), FALSE when operation succeeded and no retry is needed. - /// Maximum number of retry attempts before giving up (default: 5) - /// Base delay in seconds for exponential backoff calculation (default: 2). Actual delay doubles with each attempt. + /// Maximum number of retry attempts. Defaults to the value set in the constructor. + /// Base delay in seconds for exponential backoff. Defaults to the value set in the constructor. /// Cancellation token to cancel the operation /// Result of the operation when shouldRetry returns false (success), or the last result after all retries are exhausted (may be null/default(T) if operation never succeeded) /// Thrown when HTTP request fails on the last retry attempt @@ -33,15 +37,17 @@ public RetryHelper(ILogger logger) public async Task ExecuteWithRetryAsync( Func> operation, Func shouldRetry, - int maxRetries = 5, - int baseDelaySeconds = 2, + int? maxRetries = null, + int? baseDelaySeconds = null, CancellationToken cancellationToken = default) { + var retries = maxRetries ?? _maxRetries; + var delay = baseDelaySeconds ?? _baseDelaySeconds; int attempt = 0; Exception? lastException = null; T? lastResult = default; - while (attempt < maxRetries) + while (attempt < retries) { try { @@ -52,14 +58,14 @@ public async Task ExecuteWithRetryAsync( return lastResult; } - if (attempt < maxRetries - 1) + if (attempt < retries - 1) { - var delay = CalculateDelay(attempt, baseDelaySeconds); + var delaySpan = CalculateDelay(attempt, delay); _logger.LogInformation( "Retry attempt {AttemptNumber} of {MaxRetries}. Waiting {DelaySeconds} seconds...", - attempt + 1, maxRetries, (int)delay.TotalSeconds); + attempt + 1, retries, (int)delaySpan.TotalSeconds); - await Task.Delay(delay, cancellationToken); + await Task.Delay(delaySpan, cancellationToken); } attempt++; @@ -72,14 +78,14 @@ public async Task ExecuteWithRetryAsync( lastException = ex; _logger.LogWarning("Exception: {Message}", ex.Message); - if (attempt < maxRetries - 1) + if (attempt < retries - 1) { - var delay = CalculateDelay(attempt, baseDelaySeconds); + var delaySpan = CalculateDelay(attempt, delay); _logger.LogInformation( "Retry attempt {AttemptNumber} of {MaxRetries}. Waiting {DelaySeconds} seconds...", - attempt + 1, maxRetries, (int)delay.TotalSeconds); + attempt + 1, retries, (int)delaySpan.TotalSeconds); - await Task.Delay(delay, cancellationToken); + await Task.Delay(delaySpan, cancellationToken); } attempt++; @@ -97,7 +103,7 @@ public async Task ExecuteWithRetryAsync( { throw new RetryExhaustedException( "Async operation with retry", - maxRetries, + retries, "Operation did not return a value and no exception was thrown"); } @@ -109,14 +115,14 @@ public async Task ExecuteWithRetryAsync( /// /// Return type of the operation /// The operation to execute - /// Maximum number of retry attempts (default: 5) - /// Base delay in seconds for exponential backoff (default: 2) + /// Maximum number of retry attempts. Defaults to the value set in the constructor. + /// Base delay in seconds for exponential backoff. Defaults to the value set in the constructor. /// Cancellation token /// Result of the operation public async Task ExecuteWithRetryAsync( Func> operation, - int maxRetries = 5, - int baseDelaySeconds = 2, + int? maxRetries = null, + int? baseDelaySeconds = null, CancellationToken cancellationToken = default) { return await ExecuteWithRetryAsync( @@ -135,22 +141,24 @@ public async Task ExecuteWithRetryAsync( /// Return type of the operation /// The async operation to execute. Receives a cancellation token and returns a result. /// Async predicate that determines if retry is needed. Returns TRUE when the operation should be retried, FALSE when done. - /// Maximum number of retry attempts before giving up (default: 5) - /// Base delay in seconds for exponential backoff calculation (default: 2). + /// Maximum number of retry attempts. Defaults to the value set in the constructor. + /// Base delay in seconds for exponential backoff. Defaults to the value set in the constructor. /// Cancellation token to cancel the operation /// Result of the operation when shouldRetryAsync returns false (success), or the last result after all retries are exhausted. public async Task ExecuteWithRetryAsync( Func> operation, Func> shouldRetryAsync, - int maxRetries = 5, - int baseDelaySeconds = 2, + int? maxRetries = null, + int? baseDelaySeconds = null, CancellationToken cancellationToken = default) { + var retries = maxRetries ?? _maxRetries; + var delay = baseDelaySeconds ?? _baseDelaySeconds; int attempt = 0; Exception? lastException = null; T? lastResult = default; - while (attempt < maxRetries) + while (attempt < retries) { try { @@ -161,14 +169,14 @@ public async Task ExecuteWithRetryAsync( return lastResult; } - if (attempt < maxRetries - 1) + if (attempt < retries - 1) { - var delay = CalculateDelay(attempt, baseDelaySeconds); + var delaySpan = CalculateDelay(attempt, delay); _logger.LogInformation( "Retry attempt {AttemptNumber} of {MaxRetries}. Waiting {DelaySeconds} seconds...", - attempt + 1, maxRetries, (int)delay.TotalSeconds); + attempt + 1, retries, (int)delaySpan.TotalSeconds); - await Task.Delay(delay, cancellationToken); + await Task.Delay(delaySpan, cancellationToken); } attempt++; @@ -181,14 +189,14 @@ public async Task ExecuteWithRetryAsync( lastException = ex; _logger.LogWarning("Exception: {Message}", ex.Message); - if (attempt < maxRetries - 1) + if (attempt < retries - 1) { - var delay = CalculateDelay(attempt, baseDelaySeconds); + var delaySpan = CalculateDelay(attempt, delay); _logger.LogInformation( "Retry attempt {AttemptNumber} of {MaxRetries}. Waiting {DelaySeconds} seconds...", - attempt + 1, maxRetries, (int)delay.TotalSeconds); + attempt + 1, retries, (int)delaySpan.TotalSeconds); - await Task.Delay(delay, cancellationToken); + await Task.Delay(delaySpan, cancellationToken); } attempt++; @@ -204,7 +212,7 @@ public async Task ExecuteWithRetryAsync( { throw new RetryExhaustedException( "Async operation with retry", - maxRetries, + retries, "Operation did not return a value and no exception was thrown"); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/IMicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/IMicrosoftGraphTokenProvider.cs index e64c711e..df1c41d0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/IMicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/IMicrosoftGraphTokenProvider.cs @@ -23,5 +23,6 @@ public interface IMicrosoftGraphTokenProvider bool useDeviceCode = false, string? clientAppId = null, CancellationToken ct = default, - string? loginHint = null); + string? loginHint = null, + bool forceRefresh = false); } 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 b988ee49..a4caa792 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -91,11 +91,12 @@ public MicrosoftGraphTokenProvider( bool useDeviceCode = false, string? clientAppId = null, CancellationToken ct = default, - string? loginHint = null) + string? loginHint = null, + bool forceRefresh = false) { var validatedScopes = ValidateAndPrepareScopes(scopes); ValidateTenantId(tenantId); - + if (!string.IsNullOrWhiteSpace(clientAppId)) { ValidateClientAppId(clientAppId); @@ -104,6 +105,10 @@ public MicrosoftGraphTokenProvider( var cacheKey = MakeCacheKey(tenantId, validatedScopes, clientAppId, loginHint); var tokenExpirationMinutes = AuthenticationConstants.TokenExpirationBufferMinutes; + // When forceRefresh is requested, evict the cached entry so the acquire path always runs. + if (forceRefresh) + _tokenCache.TryRemove(cacheKey, out _); + // Fast path: cached + not expiring soon if (_tokenCache.TryGetValue(cacheKey, out var cached) && cached.ExpiresOnUtc > DateTimeOffset.UtcNow.AddMinutes(tokenExpirationMinutes) && @@ -119,7 +124,7 @@ public MicrosoftGraphTokenProvider( await gate.WaitAsync(ct); try { - // Re-check inside lock + // Re-check inside lock (forceRefresh already evicted the entry above the lock) if (_tokenCache.TryGetValue(cacheKey, out cached) && cached.ExpiresOnUtc > DateTimeOffset.UtcNow.AddMinutes(tokenExpirationMinutes) && !string.IsNullOrWhiteSpace(cached.AccessToken)) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/design.md b/src/Microsoft.Agents.A365.DevTools.Cli/design.md index bde68d4d..60344271 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/design.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/design.md @@ -171,7 +171,62 @@ By default the CLI targets the commercial Microsoft Graph endpoint. For sovereig This field is optional. When omitted, `https://graph.microsoft.com` is used. -The value is read from `Agent365Config.GraphBaseUrl` and forwarded to `GraphApiService` via its `GraphBaseUrl` property after config is loaded. This controls both the HTTP endpoint used for all Graph API calls and the token resource identifier passed to `az account get-access-token`. +The value is read from `Agent365Config.GraphBaseUrl` and forwarded to `GraphApiService` via its `GraphBaseUrl` property after config is loaded. This controls both the HTTP endpoint used for all Graph API calls and the token resource identifier passed to `AuthenticationService.GetAccessTokenAsync`. + +--- + +## Authentication Architecture + +All token acquisition goes through **MSAL.NET via `AuthenticationService`**. No az CLI subprocess is used for tokens. + +### Token Acquisition Flow + +``` +All callers (GraphApiService, ArmApiService, BotConfigurator, ...) + | + v +AuthenticationService.GetAccessTokenAsync(resource, tenantId) + | + +-- Check persistent disk cache (%LocalAppData%\Agent365\token-cache.json) + | Cache key: {resource}[:tenant:{tenantId}][:user:{userId}] + | + +-- Cache hit + not expiring: return token immediately (0 prompts) + | + +-- Cache miss / expired: MsalBrowserCredential.GetTokenAsync(scopes) + | + +-- Windows: WAM broker (no browser, SSO, CAP-compliant) + +-- macOS: System browser → device code fallback if restricted + +-- Linux: Device code flow +``` + +### Two Auth Paths + +| Path | When used | Scopes | Client app | +|---|---|---|---| +| **Default (MSAL)** | ARM, Graph `.default` calls | `{resource}/.default` | PowerShell client ID | +| **Delegated scopes (MSAL)** | Graph calls needing specific permissions (e.g. `AgentInstance.ReadWrite.All`) | Explicit scope list | `clientAppId` from config | + +### Login Hint Resolution + +To prevent WAM from selecting a stale or wrong account, a login hint (UPN) is resolved before interactive auth: +1. `AzCliHelper.ResolveLoginHintAsync()` — reads `az account show` if az CLI is present +2. `AuthenticationService.ResolveLoginHintFromCacheAsync()` — decodes `upn`/`preferred_username` from a cached JWT if az CLI is unavailable + +### IAuthenticationService Interface + +`IAuthenticationService` is defined in the same file as `AuthenticationService` (`AuthenticationService.cs`). This is intentional — the interface is narrow (two methods), tightly coupled to its single implementation, and co-location follows the **related-interfaces convention** in the copilot instructions. It exists solely to enable test substitution in `ArmApiService` and `GraphApiService` without triggering real MSAL/WAM prompts. + +Only `GetAccessTokenAsync` and `ResolveLoginHintFromCacheAsync` are on the interface. Other methods (`GetAccessTokenWithScopesAsync`, `GetAccessTokenForMcpAsync`, `ClearCache`) stay on the concrete class and are used by commands that take `AuthenticationService` directly. + +### Token Caching + +- **Persistent cache** (`AuthenticationService`): survives across CLI invocations, keyed by resource + tenant + user +- **Process-level login hint cache** (`AzCliHelper`): caches the result of `az account show` for the process lifetime — invalidated after `az login` operations + +### Platform Notes + +- **Windows**: WAM handles token acquisition at the OS level — no browser popup, no Python subprocess, corporate proxy not involved +- **macOS/Linux**: Browser redirect or device code — falls back to device code automatically if browser auth is blocked by tenant policy (e.g., corp-managed macOS) --- 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 c0d9c8da..4217fff6 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 @@ -51,12 +51,8 @@ public BlueprintSubcommandTests() var mockPlatformDetectorLogger = Substitute.For>(); _mockPlatformDetector = Substitute.ForPartsOf(mockPlatformDetectorLogger); _mockBotConfigurator = Substitute.For(); - // Pass a no-op loginHintResolver to prevent AzCliHelper.ResolveLoginHintAsync from spawning - // a real "az account show" process on every test that touches GraphApiService. - Func> noOpLoginHint = () => Task.FromResult(null); _mockGraphApiService = Substitute.ForPartsOf( - Substitute.For>(), _mockExecutor, - (HttpMessageHandler?)null, (IMicrosoftGraphTokenProvider?)null, noOpLoginHint, (string?)null); + Substitute.For>(), _mockExecutor, (Func>)(() => Task.FromResult(null))); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); _mockClientAppValidator = Substitute.For(); _mockBlueprintLookupService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs index d03e8782..3a4e90f3 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs @@ -68,7 +68,7 @@ public CleanupCommandBotEndpointTests() .Returns("test-token"); var mockGraphLogger = Substitute.For>(); - _graphApiService = new GraphApiService(mockGraphLogger, _mockExecutor, null, _mockTokenProvider); + _graphApiService = new GraphApiService(mockGraphLogger, _mockExecutor, Substitute.For(), null, _mockTokenProvider); var mockBlueprintLogger = Substitute.For>(); _agentBlueprintService = new AgentBlueprintService(mockBlueprintLogger, _graphApiService); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index fee5cb22..f6f4768c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -63,7 +63,7 @@ public CleanupCommandTests() // Pass a TestHttpMessageHandler (returns 404 when queue empty) instead of null to avoid // real HTTPS calls to graph.microsoft.com — the handler returns immediately, no network needed. var mockGraphLogger = Substitute.For>(); - _graphApiService = new GraphApiService(mockGraphLogger, _mockExecutor, new TestHttpMessageHandler(), _mockTokenProvider, + _graphApiService = new GraphApiService(mockGraphLogger, _mockExecutor, Substitute.For(), new TestHttpMessageHandler(), _mockTokenProvider, loginHintResolver: () => Task.FromResult(null)); // Create AgentBlueprintService wrapping GraphApiService diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index c053c8a3..d85e33af 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -7,8 +7,8 @@ using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -30,7 +30,6 @@ public AgentBlueprintServiceTests() // instead of falling through to the real implementation and spawning actual az processes. _mockExecutor = Substitute.For(mockExecutorLogger); _mockTokenProvider = Substitute.For(); - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tid", "fake-graph-token"); } [Fact] @@ -63,7 +62,7 @@ public async Task SetInheritablePermissionsAsync_Creates_WhenMissing() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var graphService = new GraphApiService(_mockGraphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(_mockLogger, graphService); // ResolveBlueprintObjectIdAsync: First GET to check if blueprintAppId is objectId (returns 404 NotFound) @@ -123,7 +122,7 @@ public async Task SetInheritablePermissionsAsync_Patches_WhenPresent() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var graphService = new GraphApiService(_mockGraphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(_mockLogger, graphService); // Existing entry with one scope @@ -232,7 +231,7 @@ public async Task DeleteAgentIdentityAsync_WhenTokenProviderIsNull_ReturnsFalse( { // Arrange using var handler = new FakeHttpMessageHandler(); - var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, handler, tokenProvider: null); + var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, Substitute.For(), handler, tokenProvider: null); var service = new AgentBlueprintService(_mockLogger, graphService); const string tenantId = "12345678-1234-1234-1234-123456789012"; @@ -441,6 +440,15 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() } } + private static IAuthenticationService FakeAuth() + { + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token")); + return mock; + } + private (AgentBlueprintService service, FakeHttpMessageHandler handler) CreateServiceWithFakeHandler() { var handler = new FakeHttpMessageHandler(); @@ -456,7 +464,7 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() // Pass a no-op login hint resolver to skip the real 'az account show' process spawned by // AzCliHelper.ResolveLoginHintAsync — that static call bypasses the mocked CommandExecutor // and causes each test to wait several seconds for the real az CLI. - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider, + var graphService = new GraphApiService(_mockGraphLogger, executor, Substitute.For(), handler, _mockTokenProvider, loginHintResolver: () => Task.FromResult(null)); return (new AgentBlueprintService(_mockLogger, graphService), handler); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ArmApiServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ArmApiServiceTests.cs index 5ec9f0f3..c003ff7d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ArmApiServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ArmApiServiceTests.cs @@ -7,6 +7,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; using Xunit; namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; @@ -14,10 +15,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// /// Unit tests for ArmApiService. /// Uses TestHttpMessageHandler (defined in GraphApiServiceTests.cs, same assembly) -/// to inject fake HTTP responses. The AzCliHelper process-level token cache is -/// pre-warmed in the constructor so no real az subprocess is spawned. +/// to inject fake HTTP responses. /// -[Collection("AzCliTokenCache")] public class ArmApiServiceTests { private const string TenantId = "tid"; @@ -27,14 +26,17 @@ public class ArmApiServiceTests private const string WebAppName = "webapp-test"; private const string UserObjectId = "user-obj-id"; - public ArmApiServiceTests() + private static IAuthenticationService FakeAuth() { - AzCliHelper.ResetAzCliTokenCacheForTesting(); - AzCliHelper.WarmAzCliTokenCache(ArmApiService.ArmResource, TenantId, "fake-arm-token"); + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-arm-token")); + return mock; } private static ArmApiService CreateService(HttpMessageHandler handler) => - new ArmApiService(NullLogger.Instance, handler); + new ArmApiService(NullLogger.Instance, FakeAuth(), handler, retryHelper: new RetryHelper(NullLogger.Instance, maxRetries: 1, baseDelaySeconds: 0)); // ──────────────────────────── ResourceGroupExistsAsync ──────────────────────────── diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AuthenticationServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AuthenticationServiceTests.cs index 6a040653..1a8217a5 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AuthenticationServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AuthenticationServiceTests.cs @@ -979,4 +979,83 @@ await act.Should().ThrowAsync( } #endregion + + #region ResolveLoginHintFromCacheAsync + + private static string BuildJwt(object payload) + { + var json = System.Text.Json.JsonSerializer.Serialize(payload); + // JWT uses Base64Url: replace standard Base64 chars to match real MSAL token format. + var payloadB64Url = Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes(json)) + .Replace('+', '-').Replace('/', '_').TrimEnd('='); + return $"header.{payloadB64Url}.signature"; + } + + private void WriteTokenCache(string accessToken) + { + var cacheDir = Path.GetDirectoryName(_testCachePath)!; + Directory.CreateDirectory(cacheDir); + var cache = new + { + Tokens = new Dictionary + { + ["key"] = new { AccessToken = accessToken, ExpiresOn = DateTime.UtcNow.AddHours(1), TenantId = "tid" } + } + }; + File.WriteAllText(_testCachePath, System.Text.Json.JsonSerializer.Serialize(cache)); + } + + [Fact] + public async Task ResolveLoginHintFromCacheAsync_WhenCacheHasUpnClaim_ReturnsUpn() + { + WriteTokenCache(BuildJwt(new { upn = "user@test.com" })); + + var result = await _authService.ResolveLoginHintFromCacheAsync(); + + result.Should().Be("user@test.com", + because: "the upn claim in the cached JWT should be returned as the login hint"); + } + + [Fact] + public async Task ResolveLoginHintFromCacheAsync_WhenCacheHasPreferredUsernameClaim_ReturnsIt() + { + WriteTokenCache(BuildJwt(new { preferred_username = "user@contoso.com" })); + + var result = await _authService.ResolveLoginHintFromCacheAsync(); + + result.Should().Be("user@contoso.com", + because: "preferred_username is the fallback claim when upn is absent"); + } + + [Fact] + public async Task ResolveLoginHintFromCacheAsync_WhenCacheFileDoesNotExist_ReturnsNull() + { + _authService.ClearCache(); + + var result = await _authService.ResolveLoginHintFromCacheAsync(); + + result.Should().BeNull(because: "no cache file means no login hint can be resolved"); + } + + [Fact] + public async Task ResolveLoginHintFromCacheAsync_WhenJwtHasNoUpnClaims_ReturnsNull() + { + WriteTokenCache(BuildJwt(new { sub = "some-subject", oid = "some-oid" })); + + var result = await _authService.ResolveLoginHintFromCacheAsync(); + + result.Should().BeNull(because: "a JWT without upn or preferred_username cannot provide a login hint"); + } + + [Fact] + public async Task ResolveLoginHintFromCacheAsync_WhenJwtIsMalformed_ReturnsNull() + { + WriteTokenCache("not-a-valid-jwt"); + + var result = await _authService.ResolveLoginHintFromCacheAsync(); + + result.Should().BeNull(because: "a malformed JWT should be swallowed and null returned"); + } + + #endregion } 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 e92d1a55..cb8bacf6 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 @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.Text.Json; using Xunit; @@ -128,8 +129,9 @@ public async Task EnsureValidClientAppAsync_WhenGraphQueryFails_ThrowsClientAppV _graphApiService.GraphGetWithResponseAsync( Arg.Any(), Arg.Is(p => p.Contains("displayName")), - Arg.Any(), - Arg.Any?>()) + Arg.Any(), + Arg.Any?>(), + Arg.Any()) .Returns(_ => Task.FromResult(new GraphApiService.GraphResponse { IsSuccess = false, @@ -462,8 +464,9 @@ private void SetupAppInfoGet(string appId, string requiredResourceAccess = "[]") _graphApiService.GraphGetWithResponseAsync( Arg.Any(), Arg.Is(p => p.Contains("displayName")), - Arg.Any(), - Arg.Any?>()) + Arg.Any(), + Arg.Any?>(), + Arg.Any()) .Returns(_ => Task.FromResult(new GraphApiService.GraphResponse { IsSuccess = true, @@ -480,8 +483,9 @@ private void SetupAppInfoGetEmpty() _graphApiService.GraphGetWithResponseAsync( Arg.Any(), Arg.Is(p => p.Contains("displayName")), - Arg.Any(), - Arg.Any?>()) + Arg.Any(), + Arg.Any?>(), + Arg.Any()) .Returns(_ => Task.FromResult(new GraphApiService.GraphResponse { IsSuccess = true, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddRequiredResourceAccessTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddRequiredResourceAccessTests.cs index 909846c4..c8495c70 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddRequiredResourceAccessTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddRequiredResourceAccessTests.cs @@ -5,20 +5,18 @@ using System.Text.Json; using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// -/// Isolated from other tests because AzCliHelper token cache is static state. -/// Without isolation, parallel tests calling ResetAzCliTokenCacheForTesting() clear -/// the warmup and cause real az subprocess spawns (~20s per test). +/// Tests for AgentBlueprintService.AddRequiredResourceAccessAsync. /// [Collection("AgentBlueprintServiceAddRequiredResourceAccessTests")] -public class AgentBlueprintServiceAddRequiredResourceAccessTests : IDisposable +public class AgentBlueprintServiceAddRequiredResourceAccessTests { private const string TenantId = "test-tenant-id"; private const string AppId = "test-app-id"; @@ -26,14 +24,6 @@ public class AgentBlueprintServiceAddRequiredResourceAccessTests : IDisposable private const string ObjectId = "object-id-123"; private const string SpObjectId = "sp-object-id-456"; - public AgentBlueprintServiceAddRequiredResourceAccessTests() - { - AzCliHelper.ResetAzCliTokenCacheForTesting(); - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", TenantId, "fake-graph-token"); - } - - public void Dispose() => AzCliHelper.ResetAzCliTokenCacheForTesting(); - [Fact] public async Task AddRequiredResourceAccessAsync_Success_WithValidPermissionIds() { @@ -42,7 +32,7 @@ public async Task AddRequiredResourceAccessAsync_Success_WithValidPermissionIds( var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Queue responses @@ -70,7 +60,7 @@ public async Task AddRequiredResourceAccessAsync_HandlesNullPermissionId_Gracefu var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Queue responses @@ -97,7 +87,7 @@ public async Task AddRequiredResourceAccessAsync_FailsWhen_ApplicationNotFound() var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Queue empty application response @@ -125,7 +115,7 @@ public async Task AddRequiredResourceAccessAsync_FailsWhen_ApplicationIdIsNull() var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Queue application response with null id @@ -150,7 +140,7 @@ public async Task AddRequiredResourceAccessAsync_SkipsInvalidScopes_AndProcesses var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Queue responses @@ -192,7 +182,7 @@ public async Task AddRequiredResourceAccessAsync_MergesWithExisting_Permissions( var graphLogger = Substitute.For>(); var blueprintLogger = Substitute.For>(); var executor = CreateMockExecutor(); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Application with existing requiredResourceAccess @@ -238,6 +228,15 @@ public async Task AddRequiredResourceAccessAsync_MergesWithExisting_Permissions( result.Should().BeTrue(); } + private static IAuthenticationService FakeAuth() + { + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token")); + return mock; + } + private static CommandExecutor CreateMockExecutor() { var executor = Substitute.For(Substitute.For>()); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceIsApplicationOwnerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceIsApplicationOwnerTests.cs index 9ae8d3b1..8de6a56e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceIsApplicationOwnerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceIsApplicationOwnerTests.cs @@ -6,7 +6,6 @@ using System.Text.Json; using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; @@ -23,32 +22,25 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// all queued responses in their Dispose methods. Suppressing CA2000 for this pattern. /// #pragma warning disable CA2000 // Dispose objects before losing scope -[Collection("AzCliTokenCache")] public class GraphApiServiceIsApplicationOwnerTests { private readonly ILogger _mockLogger; private readonly CommandExecutor _mockExecutor; + private static IAuthenticationService FakeAuth() + { + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token")); + return mock; + } + public GraphApiServiceIsApplicationOwnerTests() { _mockLogger = Substitute.For>(); var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); - AzCliHelper.ResetAzCliTokenCacheForTesting(); - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tenant-123", "fake-graph-token"); - - // Mock Azure CLI authentication - _mockExecutor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var cmd = callInfo.ArgAt(0); - var args = callInfo.ArgAt(1); - if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase)) - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty }); - if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase)) - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "fake-token", StandardError = string.Empty }); - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); - }); } [Fact] @@ -56,7 +48,7 @@ public async Task IsApplicationOwnerAsync_WhenUserIsOwner_ReturnsTrue() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -88,7 +80,7 @@ public async Task IsApplicationOwnerAsync_WhenUserIsNotOwner_ReturnsFalse() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -120,7 +112,7 @@ public async Task IsApplicationOwnerAsync_WhenNoOwners_ReturnsFalse() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -152,7 +144,7 @@ public async Task IsApplicationOwnerAsync_WithoutUserObjectId_RetrievesCurrentUs capturedMeRequest = req; } }); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -191,7 +183,7 @@ public async Task IsApplicationOwnerAsync_WhenGetCurrentUserFails_ReturnsFalse() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -214,7 +206,7 @@ public async Task IsApplicationOwnerAsync_IsCaseInsensitiveForUserIdComparison() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -245,7 +237,7 @@ public async Task IsApplicationOwnerAsync_WhenGraphApiCallFails_ReturnsFalse() { // Arrange using var handler = new TestHttpMessageHandler(); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; @@ -276,7 +268,7 @@ public async Task IsApplicationOwnerAsync_UsesV1Endpoint() capturedOwnersRequest = req; } }); - var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + var service = new GraphApiService(_mockLogger, _mockExecutor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var tenantId = "tenant-123"; var appObjectId = "app-obj-456"; 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 7866a283..d90de8b9 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 @@ -9,12 +9,12 @@ using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; -[Collection("AzCliTokenCache")] public class GraphApiServiceTests { private readonly ILogger _mockLogger; @@ -27,9 +27,6 @@ public GraphApiServiceTests() var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); _mockTokenProvider = Substitute.For(); - AzCliHelper.ResetAzCliTokenCacheForTesting(); - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tenant-123", "fake-graph-token"); - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tid", "fake-graph-token"); } @@ -53,7 +50,7 @@ public async Task GraphPostWithResponseAsync_Returns_Success_And_ParsesJson() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue successful POST with JSON body var bodyObj = new { result = "ok" }; @@ -94,7 +91,7 @@ public async Task GraphPostWithResponseAsync_Returns_Failure_With_Body() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue failing POST with JSON error body var errorBody = new { error = new { code = "Authorization_RequestDenied", message = "Insufficient privileges" } }; @@ -162,7 +159,7 @@ public async Task LookupServicePrincipalAsync_DoesNotIncludeConsistencyLevelHead }); // Create GraphApiService with our capturing handler - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue response for service principal lookup var spResponse = new { value = new[] { new { id = "sp-object-id-123", appId = "blueprint-456" } } }; @@ -244,7 +241,7 @@ public async Task GraphGetAsync_SanitizesTokenWithNewlineCharacters(string token return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue a successful response using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK) @@ -293,7 +290,7 @@ public async Task GraphGetAsync_TokenFromTokenProvider_SanitizesNewlines() Arg.Any()) .Returns("token-from-provider\r\nwith-embedded-newlines\n"); - var service = new GraphApiService(logger, executor, handler, tokenProvider, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, tokenProvider, loginHintResolver: () => Task.FromResult(null)); // Queue a successful response using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK) @@ -318,15 +315,9 @@ public async Task GraphGetAsync_TokenFromTokenProvider_SanitizesNewlines() [Fact] public async Task CheckServicePrincipalCreationPrivilegesAsync_SanitizesTokenWithNewlines() { - // This test verifies that CheckServicePrincipalCreationPrivilegesAsync also - // sanitizes tokens with newlines. This method has its own token handling code - // separate from EnsureGraphHeadersAsync. - - // Overwrite the "fake-graph-token" warmed in the constructor with a token that has - // embedded newlines. GetGraphAccessTokenAsync returns it from the process-level cache; - // CheckServicePrincipalCreationPrivilegesAsync must trim it before using it in the - // Authorization header. Warming directly avoids spawning a real az subprocess. - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tenant-123", "privileges-check-token\r\n\n"); + // This test verifies that CheckServicePrincipalCreationPrivilegesAsync sanitizes + // tokens with embedded whitespace before setting the Authorization header. + // Token acquisition now goes through IAuthenticationService (MSAL), not az CLI. // Arrange HttpRequestMessage? capturedRequest = null; @@ -334,38 +325,13 @@ public async Task CheckServicePrincipalCreationPrivilegesAsync_SanitizesTokenWit var logger = Substitute.For>(); var executor = Substitute.For(Substitute.For>()); - // Mock az CLI to return a token WITH newline characters - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var cmd = callInfo.ArgAt(0); - var args = callInfo.ArgAt(1); + // Auth service returns a token WITH embedded newlines — simulates a whitespace-padded token. + var authWithNewlines = Substitute.For(); + authWithNewlines.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("privileges-check-token\r\n\n")); - if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase)) - { - return Task.FromResult(new CommandResult - { - ExitCode = 0, - StandardOutput = "{}", - StandardError = string.Empty - }); - } - - if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase)) - { - // Return token WITH embedded newlines - return Task.FromResult(new CommandResult - { - ExitCode = 0, - StandardOutput = "privileges-check-token\r\n\n", - StandardError = string.Empty - }); - } - - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); - }); - - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, authWithNewlines, handler, loginHintResolver: () => Task.FromResult(null)); // Queue a successful response for the directory roles query using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK) @@ -415,7 +381,7 @@ public async Task GetServicePrincipalDisplayNameAsync_SuccessfulLookup_ReturnsDi return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue successful response with Microsoft Graph service principal var spResponse = new { value = new[] { new { displayName = "Microsoft Graph" } } }; @@ -451,7 +417,7 @@ public async Task GetServicePrincipalDisplayNameAsync_ServicePrincipalNotFound_R return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue response with empty array (service principal not found) var spResponse = new { value = Array.Empty() }; @@ -487,7 +453,7 @@ public async Task GetServicePrincipalDisplayNameAsync_NullResponse_ReturnsNull() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue error response (simulating network error or Graph API error) handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.InternalServerError) @@ -522,7 +488,7 @@ public async Task GetServicePrincipalDisplayNameAsync_MissingDisplayNameProperty return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); // Queue response with malformed object (missing displayName) var spResponse = new { value = new[] { new { id = "sp-id-123", appId = "00000003-0000-0000-c000-000000000000" } } }; @@ -608,6 +574,15 @@ public async Task IsCurrentUserAdminAsync_GraphFails_ReturnsUnknown() #region IsCurrentUserAgentIdAdminAsync + private static IAuthenticationService FakeAuth() + { + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token")); + return mock; + } + private static GraphApiService CreateServiceWithTokenProvider(TestHttpMessageHandler handler) { var logger = Substitute.For>(); @@ -617,7 +592,7 @@ private static GraphApiService CreateServiceWithTokenProvider(TestHttpMessageHan Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns("fake-token"); - return new GraphApiService(logger, executor, handler, tokenProvider, loginHintResolver: () => Task.FromResult(null)); + return new GraphApiService(logger, executor, FakeAuth(), handler, tokenProvider, loginHintResolver: () => Task.FromResult(null), retryHelper: new RetryHelper(NullLogger.Instance, maxRetries: 1, baseDelaySeconds: 0)); } [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenCacheTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenCacheTests.cs deleted file mode 100644 index 93e9d9da..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenCacheTests.cs +++ /dev/null @@ -1,219 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Net; -using System.Net.Http; -using FluentAssertions; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; -using Microsoft.Extensions.Logging; -using NSubstitute; -using Xunit; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; - -/// -/// Tests to validate that Azure CLI Graph tokens are cached at the process level -/// (via AzCliHelper) so a single CLI invocation only spawns one 'az' subprocess -/// per (resource, tenantId) pair, regardless of how many GraphApiService instances -/// or callers request the same token. -/// -[Collection("GraphApiServiceTokenCacheTests")] -public class GraphApiServiceTokenCacheTests : IDisposable -{ - public GraphApiServiceTokenCacheTests() - { - AzCliHelper.AzCliTokenAcquirerOverride = null; - AzCliHelper.ResetAzCliTokenCacheForTesting(); - } - - public void Dispose() - { - AzCliHelper.AzCliTokenAcquirerOverride = null; - AzCliHelper.ResetAzCliTokenCacheForTesting(); - } - - /// - /// Sets the process-level token acquirer override and returns a counter reference. - /// The override is invoked inside GetOrAdd, so the cache still applies — only one - /// invocation per (resource, tenantId) key within a test. - /// - private static int[] SetupTokenAcquirerWithCounter(string token = "cached-token") - { - var callCount = new int[1]; - AzCliHelper.AzCliTokenAcquirerOverride = (resource, tenantId) => - { - callCount[0]++; - return Task.FromResult(token); - }; - return callCount; - } - - private static (GraphApiService service, TestHttpMessageHandler handler) CreateService() - { - var handler = new TestHttpMessageHandler(); - var logger = Substitute.For>(); - var executor = Substitute.For(Substitute.For>()); - - // 'az account show' is still used in GetGraphAccessTokenAsync for the auth-check - // fallback path; stub it to succeed so tests that hit the fallback don't hang. - executor.ExecuteAsync( - Arg.Any(), Arg.Any(), Arg.Any(), - Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var args = callInfo.ArgAt(1); - if (args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase)) - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty }); - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); - }); - - var service = new GraphApiService(logger, executor, handler); - return (service, handler); - } - - [Fact] - public async Task MultipleGraphGetAsync_SameTenant_AcquiresTokenOnlyOnce() - { - var callCount = SetupTokenAcquirerWithCounter(); - var (service, handler) = CreateService(); - - try - { - for (int i = 0; i < 3; i++) - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - - await service.GraphGetAsync("tenant-1", "/v1.0/path1"); - await service.GraphGetAsync("tenant-1", "/v1.0/path2"); - await service.GraphGetAsync("tenant-1", "/v1.0/path3"); - - callCount[0].Should().Be(1, - because: "the process-level cache must serve the same (resource, tenant) token " + - "from the first acquisition — re-running az account get-access-token on every " + - "Graph call within a single command costs 20-40s per call"); - } - finally { handler.Dispose(); } - } - - [Fact] - public async Task GraphGetAsync_DifferentTenants_AcquiresTokenForEach() - { - var callCount = SetupTokenAcquirerWithCounter(); - var (service, handler) = CreateService(); - - try - { - for (int i = 0; i < 2; i++) - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - - await service.GraphGetAsync("tenant-1", "/v1.0/path1"); - await service.GraphGetAsync("tenant-2", "/v1.0/path2"); - - callCount[0].Should().Be(2, - because: "different tenant IDs are different cache keys — each tenant requires " + - "its own 'az account get-access-token --tenant' call"); - } - finally { handler.Dispose(); } - } - - [Fact] - public async Task MixedGraphOperations_SameTenant_AcquiresTokenOnlyOnce() - { - var callCount = SetupTokenAcquirerWithCounter(); - var (service, handler) = CreateService(); - - try - { - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"id\":\"123\"}") }); - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - - await service.GraphGetAsync("tenant-1", "/v1.0/path1"); - await service.GraphPostAsync("tenant-1", "/v1.0/path2", new { name = "test" }); - await service.GraphGetAsync("tenant-1", "/v1.0/path3"); - - callCount[0].Should().Be(1, - because: "GET and POST operations share the same process-level token cache — " + - "mixed Graph operations within a command must not each re-acquire a token"); - } - finally { handler.Dispose(); } - } - - [Fact] - public async Task MultipleGraphApiServiceInstances_SameTenant_AcquireTokenOnlyOnce() - { - // This is the key regression scenario: previously, each GraphApiService instance had - // its own instance-level cache, so a new instance in each setup phase would re-run - // 'az account get-access-token'. With a process-level cache, all instances share one token. - var callCount = SetupTokenAcquirerWithCounter(); - - var handler1 = new TestHttpMessageHandler(); - var handler2 = new TestHttpMessageHandler(); - - try - { - handler1.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - handler2.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - - var logger = Substitute.For>(); - var executor = Substitute.For(Substitute.For>()); - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), - Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty })); - - var service1 = new GraphApiService(logger, executor, handler1); - var service2 = new GraphApiService(logger, executor, handler2); - - await service1.GraphGetAsync("tenant-1", "/v1.0/path1"); - await service2.GraphGetAsync("tenant-1", "/v1.0/path1"); - - callCount[0].Should().Be(1, - because: "the process-level cache is shared across all GraphApiService instances — " + - "a second instance must not re-run 'az account get-access-token' for the same tenant"); - } - finally - { - handler1.Dispose(); - handler2.Dispose(); - } - } - - [Fact] - public async Task GraphGetAsync_AfterCacheInvalidation_AcquiresNewToken() - { - // Validates that InvalidateAzCliTokenCache() forces fresh token acquisition — - // used by ClientAppValidator and DelegatedConsentService after az login/CAE events. - var callCount = SetupTokenAcquirerWithCounter(); - var (service, handler) = CreateService(); - - try - { - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { Content = new StringContent("{\"value\":[]}") }); - - await service.GraphGetAsync("tenant-1", "/v1.0/path1"); - - // Simulate a CAE event or forced re-auth that invalidates all cached tokens - AzCliHelper.InvalidateAzCliTokenCache(); - - await service.GraphGetAsync("tenant-1", "/v1.0/path2"); - - callCount[0].Should().Be(2, - because: "InvalidateAzCliTokenCache clears the process-level cache — " + - "the next call must re-acquire a fresh token (e.g., after CAE revocation or az login)"); - } - finally { handler.Dispose(); } - } -} - -[CollectionDefinition("GraphApiServiceTokenCacheTests", DisableParallelization = true)] -public class GraphApiServiceTokenCacheTestCollection { } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenTrimTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenTrimTests.cs index ec07a71b..80a2b253 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenTrimTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenTrimTests.cs @@ -5,8 +5,8 @@ using System.Net.Http; using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -20,9 +20,6 @@ public class GraphApiServiceTokenTrimTests { public GraphApiServiceTokenTrimTests() { - // Pre-warm the process-level token cache with a token that includes a newline so - // EnsureGraphHeadersAsync reads from cache and the trimming at line 256 is exercised. - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tid", "fake-graph-token\n"); } [Theory] @@ -52,7 +49,11 @@ public async Task EnsureGraphHeadersAsync_TrimsNewlineCharactersFromToken(string return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var mockAuth = Substitute.For(); + mockAuth.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(tokenWithWhitespace)); + var service = new GraphApiService(logger, executor, mockAuth, handler, loginHintResolver: () => Task.FromResult(null)); // Queue successful GET response using var response = new HttpResponseMessage(HttpStatusCode.OK) @@ -87,7 +88,7 @@ public async Task EnsureGraphHeadersAsync_WithTokenProvider_TrimsNewlineCharacte Arg.Any()) .Returns("fake-token\n"); - var service = new GraphApiService(logger, executor, handler, tokenProvider, loginHintResolver: () => Task.FromResult(null)); + var service = new GraphApiService(logger, executor, Substitute.For(), handler, tokenProvider, loginHintResolver: () => Task.FromResult(null)); // Queue successful GET response using var response = new HttpResponseMessage(HttpStatusCode.OK) @@ -123,7 +124,11 @@ public async Task CheckServicePrincipalCreationPrivilegesAsync_TrimsNewlineChara return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var service = new GraphApiService(logger, executor, handler, loginHintResolver: () => Task.FromResult(null)); + var mockAuth = Substitute.For(); + mockAuth.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token\r\n")); + var service = new GraphApiService(logger, executor, mockAuth, handler, loginHintResolver: () => Task.FromResult(null)); // Queue successful response for directory roles using var response = new HttpResponseMessage(HttpStatusCode.OK) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceVerifyInheritablePermissionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceVerifyInheritablePermissionsTests.cs index ee4b32a5..10b31b51 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceVerifyInheritablePermissionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceVerifyInheritablePermissionsTests.cs @@ -5,8 +5,8 @@ using System.Text.Json; using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -14,9 +14,13 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; public class AgentBlueprintServiceVerifyInheritablePermissionsTests { - public AgentBlueprintServiceVerifyInheritablePermissionsTests() + private static IAuthenticationService FakeAuth() { - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tid", "fake-graph-token"); + var mock = Substitute.For(); + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("fake-token")); + return mock; } [Fact] @@ -40,7 +44,7 @@ public async Task VerifyInheritablePermissionsAsync_PermissionsExist_ReturnsScop return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); var response = new @@ -103,7 +107,7 @@ public async Task VerifyInheritablePermissionsAsync_PermissionsNotFound_ReturnsF return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); var response = new @@ -163,7 +167,7 @@ public async Task VerifyInheritablePermissionsAsync_ApiFailure_ReturnsError() return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); }); - var graphService = new GraphApiService(graphLogger, executor, handler); + var graphService = new GraphApiService(graphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult(null)); var service = new AgentBlueprintService(blueprintLogger, graphService); // Simulate 404 Not Found to trigger API failure path diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/AzCliHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/AzCliHelperTests.cs index 29379277..429cd7d7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/AzCliHelperTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/AzCliHelperTests.cs @@ -16,11 +16,9 @@ public class AzCliHelperTests : IDisposable { public AzCliHelperTests() { - // Start each test with a clean slate — both static caches + // Start each test with a clean slate AzCliHelper.LoginHintResolverOverride = null; AzCliHelper.ResetLoginHintCacheForTesting(); - AzCliHelper.AzCliTokenAcquirerOverride = null; - AzCliHelper.ResetAzCliTokenCacheForTesting(); } public void Dispose() @@ -28,8 +26,6 @@ public void Dispose() // Restore static state so other tests are not affected AzCliHelper.LoginHintResolverOverride = null; AzCliHelper.ResetLoginHintCacheForTesting(); - AzCliHelper.AzCliTokenAcquirerOverride = null; - AzCliHelper.ResetAzCliTokenCacheForTesting(); } [Fact] @@ -102,96 +98,6 @@ public async Task ResolveLoginHintAsync_AfterCacheReset_InvokesResolverAgain() callCount.Should().Be(2, because: "ResetLoginHintCacheForTesting clears the cache, forcing a fresh resolve — required for test isolation"); } - - // ------------------------------------------------------------------------- - // AcquireAzCliTokenAsync — process-level token cache - // ------------------------------------------------------------------------- - - [Fact] - public async Task AcquireAzCliTokenAsync_WhenOverrideSet_ReturnsOverrideValue() - { - AzCliHelper.AzCliTokenAcquirerOverride = (_, __) => Task.FromResult("test-token"); - - var result = await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - - result.Should().Be("test-token", - because: "the override replaces the real az subprocess — used in tests to inject known tokens"); - } - - [Fact] - public async Task AcquireAzCliTokenAsync_CalledTwiceSameKey_InvokesAcquirerOnce() - { - var callCount = 0; - AzCliHelper.AzCliTokenAcquirerOverride = (_, __) => - { - callCount++; - return Task.FromResult("shared-token"); - }; - - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - - callCount.Should().Be(1, - because: "the process-level cache must serve the same (resource, tenant) token " + - "after the first acquisition — calling az account get-access-token on every " + - "request costs 20-40s per call"); - } - - [Fact] - public async Task AcquireAzCliTokenAsync_DifferentTenants_InvokesAcquirerForEach() - { - var callCount = 0; - AzCliHelper.AzCliTokenAcquirerOverride = (_, __) => - { - callCount++; - return Task.FromResult("token"); - }; - - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-2"); - - callCount.Should().Be(2, - because: "different tenant IDs are different cache keys — each tenant requires its own token"); - } - - [Fact] - public async Task AcquireAzCliTokenAsync_AfterInvalidation_InvokesAcquirerAgain() - { - var callCount = 0; - AzCliHelper.AzCliTokenAcquirerOverride = (_, __) => - { - callCount++; - return Task.FromResult("token"); - }; - - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - AzCliHelper.InvalidateAzCliTokenCache(); - await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - - callCount.Should().Be(2, - because: "InvalidateAzCliTokenCache clears the cache — the next call must re-acquire " + - "a fresh token; this is required after 'az login' or a CAE token revocation event"); - } - - [Fact] - public async Task WarmAzCliTokenCache_InjectedToken_ReturnedOnNextCall() - { - // Override that always fails — should NOT be called after warming the cache - AzCliHelper.AzCliTokenAcquirerOverride = (_, __) => - Task.FromResult(null); - - AzCliHelper.WarmAzCliTokenCache("https://graph.microsoft.com/", "tenant-1", "warmed-token"); - - // The warmup bypasses the GetOrAdd — the cache entry is set directly. - // Reset override so we can verify the warmed value is returned, not re-acquired. - AzCliHelper.AzCliTokenAcquirerOverride = null; - var result = await AzCliHelper.AcquireAzCliTokenAsync("https://graph.microsoft.com/", "tenant-1"); - - result.Should().Be("warmed-token", - because: "WarmAzCliTokenCache injects a token acquired via auth recovery into the " + - "process-level cache — subsequent callers must receive the injected token " + - "without re-running az account get-access-token"); - } } [CollectionDefinition("AzCliHelperTests", DisableParallelization = true)] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/NetworkHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/NetworkHelperTests.cs new file mode 100644 index 00000000..820cad27 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/NetworkHelperTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Net.Sockets; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services.Helpers; + +public class NetworkHelperTests +{ + [Fact] + public void IsConnectionResetByProxy_WhenDirectSocketException_ReturnsTrue() + { + var ex = new SocketException((int)SocketError.ConnectionReset); + + NetworkHelper.IsConnectionResetByProxy(ex).Should().BeTrue( + because: "a direct SocketException with ConnectionReset is a proxy-reset signal"); + } + + [Fact] + public void IsConnectionResetByProxy_WhenHttpRequestExceptionWrapsConnectionReset_ReturnsTrue() + { + var inner = new SocketException((int)SocketError.ConnectionReset); + var ex = new HttpRequestException("Connection reset", inner); + + NetworkHelper.IsConnectionResetByProxy(ex).Should().BeTrue( + because: "HttpRequestException wrapping a ConnectionReset SocketException is the typical TLS proxy reset pattern"); + } + + [Fact] + public void IsConnectionResetByProxy_WhenSocketExceptionIsNotConnectionReset_ReturnsFalse() + { + var ex = new SocketException((int)SocketError.TimedOut); + + NetworkHelper.IsConnectionResetByProxy(ex).Should().BeFalse( + because: "only SocketError.ConnectionReset (10054) indicates a proxy reset — other socket errors should not be misclassified"); + } + + [Fact] + public void IsConnectionResetByProxy_WhenNoSocketExceptionInChain_ReturnsFalse() + { + var ex = new InvalidOperationException("some other error"); + + NetworkHelper.IsConnectionResetByProxy(ex).Should().BeFalse( + because: "exceptions unrelated to socket errors are not proxy resets"); + } + + [Fact] + public void IsConnectionResetByProxy_WhenDeeplyNestedConnectionReset_ReturnsTrue() + { + var socketEx = new SocketException((int)SocketError.ConnectionReset); + var mid = new IOException("IO error", socketEx); + var outer = new HttpRequestException("outer", mid); + + NetworkHelper.IsConnectionResetByProxy(outer).Should().BeTrue( + because: "the chain walk must find SocketException at any nesting depth"); + } +} 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 66b68b96..dd87869b 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 @@ -348,4 +348,38 @@ public async Task GetMgGraphAccessTokenAsync_EscapesSingleQuotesInClientAppId() // Assert token.Should().Be(expectedToken); } + + [Fact] + public async Task GetMgGraphAccessTokenAsync_WithForceRefresh_BypassesCache() + { + // Arrange + var tenantId = "12345678-1234-1234-1234-123456789abc"; + var scopes = new[] { "AgentIdentityBlueprint.DeleteRestore.All" }; + var clientAppId = "87654321-4321-4321-4321-cba987654321"; + // Valid JWT with a future exp claim (year 2099) + var msalToken = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzZWxsYWsiLCJleHAiOjQwNzA5MDg4MDB9.signature"; + var callCount = 0; + + var provider = new MicrosoftGraphTokenProvider(_executor, _logger) + { + MsalTokenAcquirerOverride = (_, _, _, _) => + { + callCount++; + return Task.FromResult(msalToken); + } + }; + + // Prime the cache with a first call + await provider.GetMgGraphAccessTokenAsync(tenantId, scopes, false, clientAppId); + callCount.Should().Be(1); + + // Act — second call with forceRefresh: true should bypass the cache + var token = await provider.GetMgGraphAccessTokenAsync(tenantId, scopes, false, clientAppId, forceRefresh: true); + + // Assert + token.Should().Be(msalToken); + callCount.Should().Be(2, + because: "forceRefresh: true must evict the cached token and re-invoke MSAL, " + + "ensuring a stale CAE-revoked token is not reused"); + } }