diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be935fe..41822f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added +- `a365 network gsa enable|disable|status` turns Global Secure Access on or off for your Agent 365 environment (#497). + ## [2.0.0] - 2026-10-20 ### Upgrade Notes diff --git a/docs/commands/README.md b/docs/commands/README.md index 29153973..7a38400f 100644 --- a/docs/commands/README.md +++ b/docs/commands/README.md @@ -27,6 +27,10 @@ There is reference documentation for each command. | [develop-mcp list-servers](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/develop-mcp#develop-mcp-list-servers) | List MCP servers in a specific Dataverse environment. | | [develop-mcp publish](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/develop-mcp#develop-mcp-publish) | Publish an MCP server to a Dataverse environment. | | [develop-mcp unpublish](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/develop-mcp#develop-mcp-unpublish) | Unpublish an MCP server from a Dataverse environment. | +| [network gsa](network-gsa.md) | Configure Global Secure Access for Agent 365. | +| [network gsa enable](network-gsa.md#enable-and-disable) | Turn Global Secure Access on for your Agent 365 environment. | +| [network gsa disable](network-gsa.md#enable-and-disable) | Turn Global Secure Access off for your Agent 365 environment. | +| [network gsa status](network-gsa.md#status) | Show whether Global Secure Access is on for your Agent 365 environment. | | [publish](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/publish) | Update manifest.json ID values and publish the package. Configure federated identity and app role assignments. | | [query-entra](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/query-entra) | Query Microsoft Entra ID for agent information including scopes, permissions, and consent status. | | [query-entra blueprint-scopes](https://learn.microsoft.com/microsoft-agent-365/developer/reference/cli/query-entra#query-entra-blueprint-scopes) | List configured scopes and consent status for the agent blueprint. | diff --git a/docs/commands/network-gsa.md b/docs/commands/network-gsa.md new file mode 100644 index 00000000..7f2df8a1 --- /dev/null +++ b/docs/commands/network-gsa.md @@ -0,0 +1,104 @@ +# `a365 network gsa` + +Turns **Global Secure Access** on or off for the tenant's Agent 365 Power Platform environment, +without needing the id of that environment. + +## Why this command exists + +Global Secure Access is a per-environment Power Platform setting. Agent 365 provisions a managed +environment for the tenant and does not publish its id, so the setting cannot be reached through +the Power Platform admin surfaces that take an environment id. These subcommands ask the Agent 365 +platform to apply the change against the environment it resolves for your tenant. + +## Prerequisites + +- **Global Administrator** or **Power Platform Administrator** in the tenant. The platform rejects + anyone else. +- An `az login --tenant --allow-no-subscriptions` to the tenant you intend to configure; + no Azure subscription is needed. +- Public cloud only. Sovereign cloud configurations are rejected before authentication. +- The Agent 365 Tools API must expose `AgentTools.Gsa.Manage.All` for enable/disable and + `AgentTools.Gsa.Read.All` for status, and preauthorize them for the CLI's Microsoft client application. + These are platform deployment prerequisites, not permissions an admin can add to Microsoft's app registration. + +The `az login` is what selects the tenant. These commands read `az account show` once per run and +authenticate against the tenant and account it reports, so `az login --tenant ` is how you +choose which tenant to configure when you have more than one. Nothing else is read from Azure — the +setting itself lives in Power Platform, not in your subscription. Without an explicit tenant the +Windows broker silently returns whichever account Windows prefers, which would apply a tenant-wide +setting to the wrong tenant. If the account you are signed into cannot be matched, the command +fails rather than falling back. + +## Subcommands + +| Command | Description | +| --- | --- | +| `a365 network gsa enable` | Turn Global Secure Access on. | +| `a365 network gsa disable` | Turn Global Secure Access off. | +| `a365 network gsa status` | Show whether Global Secure Access is on. | + +### `enable` and `disable` + +```bash +a365 network gsa enable [--wait] [--yes] +a365 network gsa disable [--wait] [--yes] +``` + +| Option | Description | +| --- | --- | +| `--wait` | Keep polling until the change appears on the environment, instead of returning while it is still being applied. | +| `--yes`, `-y` | Skip the confirmation prompt. | + +Both verbs prompt before changing the tenant-wide setting; pass `--yes` in automation. +Requesting the value the environment already holds is a no-op and succeeds. + +### `status` + +```bash +a365 network gsa status +``` + +There is no operation handle to pass. Power Platform applies the change asynchronously but issues +no operation id for it, so the CLI reports progress by re-reading the setting rather than by +polling a handle. + +## Statuses and exit codes + +| Status | Meaning | +| --- | --- | +| `Enabled` | Global Secure Access is on. | +| `Disabled` | Global Secure Access is off. | +| `NotConfigured` | The tenant has never set the value. This is **not** the same as `Disabled`. | + +A change that has been accepted but has not yet surfaced is reported as still being applied, with +the status still showing the value it has not yet displaced. + +Without `--wait`, an accepted change returns `0` even while it is still being applied; output labels +the requested and current values separately. With `--wait`, `0` means the requested value was observed. +A wait that cannot confirm that value within ten minutes returns `1`, names the last observed value +when available, and tells you to check `a365 network gsa status`. The change remains accepted; +the timeout does not cancel it. + +Transient status-read failures are retried up to three attempts with bounded backoff, within the +wait's overall time budget. Permanent request errors and exhausted retries return `1`; if a change +was already accepted, the CLI reports that separately from a failure to verify its status. + +## Typical flow + +```bash +a365 network gsa enable --wait +a365 network gsa status +``` + +## Troubleshooting + +| Symptom | Cause | +| --- | --- | +| `403`, reporting `insufficient_scope` | The token lacks the reported scope. Contact support to verify the API exposes it and preauthorizes it for the CLI's Microsoft client application. | +| Other `403` from the platform | Check the reported error and confirm the caller has the Global Administrator or Power Platform Administrator role. | +| `409`, reporting a governing policy | A Power Platform policy owns this setting. Change it through that policy; the environment-level value is ignored while the policy applies. | +| `404`, reporting no environment | The tenant has no Agent 365 environment yet. | +| Status stays `NotConfigured` after `disable` | Read it again — the change is applied asynchronously and `--wait` is the way to block on it. | +| `Could not determine your Azure tenant` | No usable `az login`. Run `az login --tenant --allow-no-subscriptions` for the tenant you want to configure. | +| Public-cloud-only error | The selected cloud is unsupported; no token was requested and no change was sent. | +| Sign-in prompt names the wrong account | The tenant comes from `az account show`. Run `az account set` / `az login --tenant ` to point at the intended tenant, then retry. | diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs new file mode 100644 index 00000000..03bac9e2 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs @@ -0,0 +1,262 @@ +// 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; +using Microsoft.Extensions.Logging; +using System.CommandLine; +using System.CommandLine.Invocation; + +namespace Microsoft.Agents.A365.DevTools.Cli.Commands; + +/// +/// Tenant network configuration for Agent 365. +/// +/// Global Secure Access is normally set per Power Platform environment, which needs the id of the +/// environment being configured. Agent 365 does not publish that id, so these subcommands ask the +/// platform to apply the setting to the environment it resolves for your tenant. +/// +public static class NetworkCommand +{ + private static readonly TimeSpan DefaultWaitTimeout = TimeSpan.FromMinutes(10); + + /// + /// Creates the network command and its gsa subcommand tree. + /// + public static Command CreateCommand( + ILogger logger, + IAzureCliService azureCliService, + IGsaService gsaService, + IConfirmationProvider confirmationProvider) + { + var networkCommand = new Command(CommandNames.Network, "Configure tenant networking for Agent 365"); + + var gsaCommand = new Command( + "gsa", + "Turn Global Secure Access on or off for your Agent 365 environment. " + + "Requires the Global Administrator or Power Platform Administrator role."); + + gsaCommand.AddCommand(CreateGsaSetSubcommand( + logger, gsaService, azureCliService, confirmationProvider, enabled: true)); + gsaCommand.AddCommand(CreateGsaSetSubcommand( + logger, gsaService, azureCliService, confirmationProvider, enabled: false)); + gsaCommand.AddCommand(CreateGsaStatusSubcommand(logger, gsaService, azureCliService)); + + networkCommand.AddCommand(gsaCommand); + return networkCommand; + } + + /// + /// Resolves the Azure account to act as, or logs why it could not and returns null. + /// + /// + /// Resolved once per invocation and passed to every call that follows. az account show + /// reads mutable local state, so reading it again for authentication after prompting could + /// confirm one tenant and change another, and a transient CLI failure between two reads could + /// fail a command that had already succeeded at resolving the tenant. + /// + internal static async Task ResolveAccountAsync( + ILogger logger, + IAzureCliService azureCliService) + { + var account = await azureCliService.GetCurrentAccountAsync(); + if (account is null || string.IsNullOrWhiteSpace(account.TenantId)) + { + logger.LogError("Could not determine your Azure tenant. Run 'az login' and try again."); + return null; + } + + return account; + } + + /// + /// Asks the operator to confirm a change to tenant-wide networking, naming the tenant and the + /// action so the prompt is answerable without scrolling back. + /// + internal static async Task ConfirmChangeAsync( + IConfirmationProvider confirmationProvider, + bool yes, + string action, + string tenantId, + string? userName = null) + { + if (yes) + { + return true; + } + + var actor = string.IsNullOrWhiteSpace(userName) ? string.Empty : $" as {userName}"; + return await confirmationProvider.ConfirmAsync( + $"{action} for tenant {tenantId}{actor}. This changes networking for every Agent 365 agent in the tenant. Continue? (y/N): "); + } + + /// + /// Creates the gsa enable or disable subcommand. The two differ only in the value they send + /// and the words they use, so they share one builder. + /// + private static Command CreateGsaSetSubcommand( + ILogger logger, + IGsaService gsaService, + IAzureCliService azureCliService, + IConfirmationProvider confirmationProvider, + bool enabled) + { + var verb = enabled ? "enable" : "disable"; + var command = new Command( + verb, + $"Turn Global Secure Access {(enabled ? "on" : "off")} for your Agent 365 environment."); + + var waitOption = new Option( + "--wait", + "Keep polling until the change appears on the environment, instead of returning while " + + "it is still being applied."); + + var yesOption = new Option( + ["--yes", "-y"], + "Skip the confirmation prompt."); + + var verboseOption = new Option(["--verbose", "-v"], "Enable verbose logging"); + + command.AddOption(waitOption); + command.AddOption(yesOption); + command.AddOption(verboseOption); + + command.SetHandler(async (InvocationContext context) => + { + var wait = context.ParseResult.GetValueForOption(waitOption); + var yes = context.ParseResult.GetValueForOption(yesOption); + var ct = context.GetCancellationToken(); + + // Resolved once, then used for both the prompt and the call, so the tenant named in + // the prompt is provably the tenant changed. + var account = await ResolveAccountAsync(logger, azureCliService); + if (account == null) + { + context.ExitCode = 1; + return; + } + + if (!await ConfirmChangeAsync( + confirmationProvider, yes, $"Turn Global Secure Access {(enabled ? "on" : "off")}", + account.TenantId, account.User.Name)) + { + logger.LogInformation("Cancelled."); + context.ExitCode = 1; + return; + } + + var result = await gsaService.SetAsync(account, enabled, ct); + context.ExitCode = await ReportGsaAsync(logger, gsaService, account, result, wait, enabled, ct); + }); + + return command; + } + + private static Command CreateGsaStatusSubcommand( + ILogger logger, + IGsaService gsaService, + IAzureCliService azureCliService) + { + var command = new Command( + "status", + "Show whether Global Secure Access is on for your Agent 365 environment."); + + var verboseOption = new Option(["--verbose", "-v"], "Enable verbose logging"); + command.AddOption(verboseOption); + + command.SetHandler(async (InvocationContext context) => + { + var ct = context.GetCancellationToken(); + + var account = await ResolveAccountAsync(logger, azureCliService); + if (account == null) + { + context.ExitCode = 1; + return; + } + + var status = await gsaService.GetStatusAsync(account, ct); + if (status == null) + { + context.ExitCode = 1; + return; + } + + LogGsaStatus(logger, status); + context.ExitCode = 0; + }); + + return command; + } + + /// + /// Renders the outcome of a Global Secure Access change, optionally waiting for it to appear + /// first, and maps it to a process exit code. + /// + internal static async Task ReportGsaAsync( + ILogger logger, + IGsaService gsaService, + AzureAccountInfo account, + GsaStatusResponse? result, + bool wait, + bool enabled, + CancellationToken cancellationToken) + { + if (result == null) + { + return 1; + } + + var expectedStatus = enabled ? "Enabled" : "Disabled"; + + if (wait && result.Pending) + { + logger.LogInformation("The change is still being applied. Waiting for it to appear..."); + result = await gsaService.WaitForStatusAsync(account, expectedStatus, DefaultWaitTimeout, cancellationToken); + + if (result == null) + { + logger.LogWarning( + "The change was accepted, but its current status could not be verified. Check with: a365 network gsa status"); + return 1; + } + } + + if (wait && !string.Equals(result.Status, expectedStatus, StringComparison.OrdinalIgnoreCase)) + { + LogGsaStatus(logger, result); + logger.LogWarning( + "The change was accepted, but Global Secure Access is not {Expected} yet. Check with: a365 network gsa status", + expectedStatus); + return 1; + } + + if (result.Pending) + { + logger.LogInformation( + "Requested: {Expected} (still being applied). Check with: a365 network gsa status", + expectedStatus); + } + + LogGsaStatus(logger, result, result.Pending ? "Current" : "Global Secure Access"); + return 0; + } + + private static void LogGsaStatus(ILogger logger, GsaStatusResponse status, string label = "Global Secure Access") + { + logger.LogInformation("{Label}: {Status}", label, status.Status ?? "Unknown"); + + if (string.Equals(status.Status, "NotConfigured", StringComparison.OrdinalIgnoreCase)) + { + // Worth spelling out: a tenant that has never set this is not the same as one that + // turned it off, and the distinction changes what an admin should do next. + logger.LogInformation("This tenant has never set Global Secure Access, so no value is stored."); + } + + if (!string.IsNullOrWhiteSpace(status.Reason)) + { + logger.LogWarning("Reason: {Reason}", status.Reason); + } + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs index 8c82ee86..7ea93af2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs @@ -18,4 +18,5 @@ public static class CommandNames public const string Develop = "develop"; public const string CreateInstance = "create-instance"; public const string Logs = "logs"; + public const string Network = "network"; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs index ea8e180c..5f96f444 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs @@ -67,4 +67,14 @@ public static class ErrorMessages #endregion + public const string GsaPublicCloudOnly = + "Global Secure Access commands support public cloud only. No request was sent."; + + public const string GsaStatusMissing = + "The platform returned no readable Global Secure Access status. Correlation ID: {CorrelationId}"; + + public const string GsaScopeMissing = + "The access token is missing required scope '{RequiredScope}'. " + + "Contact support to verify this scope is exposed and preauthorized for the CLI's Microsoft client application."; + } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs new file mode 100644 index 00000000..076836bd --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json.Serialization; + +namespace Microsoft.Agents.A365.DevTools.Cli.Models; + +/// +/// Status of Global Secure Access on the tenant's Agent 365 Power Platform environment, and the +/// shape returned by enable and disable. +/// +public class GsaStatusResponse +{ + /// + /// Enabled, Disabled, or NotConfigured. + /// + /// NotConfigured is not the same as Disabled: it means the tenant has never set the value. + /// The platform keeps the two apart, so the CLI does too. + /// + [JsonPropertyName("status")] + public string? Status { get; set; } + + /// + /// True when a change was accepted but has not yet appeared on the environment. The + /// accompanying is then the value it has not yet displaced. + /// + [JsonPropertyName("pending")] + public bool Pending { get; set; } + + /// + /// Explanation the platform has to offer, when there is one. + /// + [JsonPropertyName("reason")] + public string? Reason { get; set; } +} + +/// +/// Error body returned by the platform's Global Secure Access endpoints. +/// +public class GsaErrorResponse +{ + /// + /// Human-readable error message. + /// + [JsonPropertyName("error")] + public string? Error { get; set; } + + [JsonPropertyName("required_scope")] + public string? RequiredScope { get; set; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 794d823f..e1b635f9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using Microsoft.Agents.A365.DevTools.Cli.Commands; +using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services; @@ -186,6 +187,11 @@ await Task.WhenAll( var logsLogger = serviceProvider.GetRequiredService>(); var logRedactionService = serviceProvider.GetRequiredService(); rootCommand.AddCommand(LogsCommand.CreateCommand(logsLogger, logRedactionService)); + var networkLogger = serviceProvider.GetRequiredService().CreateLogger(CommandNames.Network); + var azureCliService = serviceProvider.GetRequiredService(); + var gsaService = serviceProvider.GetRequiredService(); + rootCommand.AddCommand(NetworkCommand.CreateCommand( + networkLogger, azureCliService, gsaService, confirmationProvider)); // Build pipeline manually so we can skip UseTypoCorrections() ("Did you mean?" noise) // and UseParseErrorReporting() (full help dump on any parse error), replacing both @@ -380,6 +386,13 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); services.AddSingleton(); + + // Reuses the environment the tooling service already resolved (env var, then config file), + // so the two never disagree about which Agent 365 deployment the CLI is talking to. + services.AddSingleton(provider => new GsaService( + provider.GetRequiredService>(), + provider.GetRequiredService(), + provider.GetRequiredService().Environment)); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs new file mode 100644 index 00000000..2a7f2cc5 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs @@ -0,0 +1,274 @@ +// 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 System.Diagnostics; +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using System.Text.Json; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Calls the Agent 365 platform's /agents/gsa endpoints. +/// +/// The setting lives on the tenant's Power Platform environment, whose id Agent 365 does not +/// publish. The platform resolves that environment itself, so these calls carry no environment +/// identifier at all. +/// +public class GsaService : IGsaService +{ + private const string EnablePath = "/agents/gsa/enable"; + private const string DisablePath = "/agents/gsa/disable"; + private const string StatusPath = "/agents/gsa/status"; + + private static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(10); + + private readonly ILogger _logger; + private readonly IAuthenticationService _authService; + private readonly string _environment; + private readonly HttpMessageHandler? _handler; + + public GsaService( + ILogger logger, + IAuthenticationService authService, + string environment = "prod", + HttpMessageHandler? handler = null) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _authService = authService ?? throw new ArgumentNullException(nameof(authService)); + _environment = environment ?? "prod"; + _handler = handler; + } + + /// + public async Task SetAsync( + AzureAccountInfo account, + bool enabled, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(account); + + var path = enabled ? EnablePath : DisablePath; + var operationName = enabled ? "enable Global Secure Access" : "disable Global Secure Access"; + + _logger.LogInformation( + "{Action} Global Secure Access on your Agent 365 environment...", + enabled ? "Enabling" : "Disabling"); + + return await SendAsync(account, HttpMethod.Post, path, operationName, cancellationToken); + } + + /// + public async Task GetStatusAsync( + AzureAccountInfo account, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(account); + + return await SendAsync( + account, HttpMethod.Get, StatusPath, "read Global Secure Access status", cancellationToken); + } + + /// + public async Task WaitForStatusAsync( + AzureAccountInfo account, + string expectedStatus, + TimeSpan timeout, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(account); + + if (string.IsNullOrWhiteSpace(expectedStatus)) + throw new ArgumentException("Expected status is required.", nameof(expectedStatus)); + + // Wall clock, not summed sleeps: each status call costs real time, and a caller who asked + // for five minutes should not wait eight because the service was slow. + // + // The stopwatch alone only bounds the gap between completed polls. A poll that starts just + // inside the ceiling can still run to the HttpClient's own timeout, overshooting by + // minutes, so the ceiling is also armed on the token every request is made with. + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(timeout); + + var stopwatch = Stopwatch.StartNew(); + GsaStatusResponse? last = null; + + while (true) + { + try + { + last = await GetStatusAsync(account, timeoutCts.Token); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + _logger.LogInformation( + "Stopped waiting after {Elapsed:0}s without confirming the requested state.", + stopwatch.Elapsed.TotalSeconds); + return last; + } + + if (last == null || string.Equals(last.Status, expectedStatus, StringComparison.OrdinalIgnoreCase)) + return last; + + if (stopwatch.Elapsed + PollInterval >= timeout) + return last; + + _logger.LogInformation("Still applying... ({Elapsed:0}s elapsed)", stopwatch.Elapsed.TotalSeconds); + + // The pre-sleep check above guarantees this delay finishes inside the ceiling, so it + // waits on the caller's token only -- the timeout can't fire here. + await Task.Delay(PollInterval, cancellationToken); + } + } + + private async Task SendAsync( + AzureAccountInfo account, + HttpMethod method, + string path, + string operationName, + CancellationToken cancellationToken) + { + var correlationId = HttpClientFactory.GenerateCorrelationId(); + + try + { + var authorityHost = ConfigConstants.GetAuthorityHost(_environment); + if (ConfigConstants.NormalizeEnvironmentKey(_environment) is + "GCC" or "GCC_MODERATE" or "GCC_HIGH" or "DOD" or "AZUREUSGOVERNMENT" or "CHINA" or "AZURECHINACLOUD" || + !string.Equals(authorityHost, ConfigConstants.DefaultAuthorityHost, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogError(ErrorMessages.GsaPublicCloudOnly); + return null; + } + + var url = ConfigConstants.BuildAgent365ToolsEndpointUrl(_environment, path); + var audience = ConfigConstants.GetAgent365ToolsResourceAppId(_environment); + + // Authenticate against the tenant of the az login the caller resolved, not whichever + // account the Windows broker happens to prefer. Without an explicit tenant the + // authority is "common", and WAM silently returns the Windows account even when a login + // hint names a different one — so a tenant-wide setting would be changed on the wrong + // tenant. Passing the tenant also arms the mismatch self-heal in AuthenticationService. + var authToken = await _authService.GetAccessTokenAsync( + audience, account.TenantId, userId: account.User.Name, ct: cancellationToken, + authorityHost: authorityHost); + if (string.IsNullOrWhiteSpace(authToken)) + { + _logger.LogError("Failed to acquire an Agent 365 access token."); + return null; + } + + using var httpClient = HttpClientFactory.CreateAuthenticatedClient( + authToken, correlationId: correlationId, handler: _handler); + + async Task<(HttpStatusCode StatusCode, string Body)> SendRequestAsync(CancellationToken ct) + { + using var request = new HttpRequestMessage(method, url); + if (method == HttpMethod.Post) + { + request.Content = new StringContent(string.Empty, Encoding.UTF8); + request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/json"); + } + + _logger.LogDebug("{Method} {Url} (CorrelationId: {CorrelationId})", method, url, correlationId); + using var response = await httpClient.SendAsync(request, ct); + var responseBody = await response.Content.ReadAsStringAsync(ct); + _logger.LogDebug("Response {StatusCode}: {Body}", response.StatusCode, responseBody); + return (response.StatusCode, responseBody); + } + + // Retry reads only; replaying a change after a transport failure could apply it twice. + var (statusCode, body) = method == HttpMethod.Get + ? await new RetryHelper(_logger).ExecuteWithRetryAsync( + SendRequestAsync, + response => response.StatusCode is HttpStatusCode.RequestTimeout or HttpStatusCode.TooManyRequests || + (int)response.StatusCode >= 500, + cancellationToken: cancellationToken) + : await SendRequestAsync(cancellationToken); + + if ((int)statusCode is < 200 or >= 300) + { + LogFailure(statusCode, body, operationName, correlationId); + return null; + } + + var result = string.IsNullOrWhiteSpace(body) + ? null + : JsonSerializer.Deserialize(body); + if (string.IsNullOrWhiteSpace(result?.Status)) + { + _logger.LogError(ErrorMessages.GsaStatusMissing, correlationId); + return null; + } + + return result; + } + // Cancellation is the caller's business, or the wait ceiling firing on a linked token. + // HttpClient's own timeout also surfaces as OperationCanceledException with no token + // cancelled, and that is an ordinary request failure — it belongs in the catch below so it + // is logged and reported, not thrown at whoever called enable, disable or status. + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + if (NetworkHelper.IsConnectionResetByProxy(ex)) + _logger.LogWarning(NetworkHelper.ConnectionResetWarning); + else + _logger.LogError(ex, "Failed to {Operation}. Correlation ID: {CorrelationId}", operationName, correlationId); + return null; + } + } + + private void LogFailure(HttpStatusCode statusCode, string body, string operationName, string correlationId) + { + GsaErrorResponse? error = null; + try + { + error = JsonSerializer.Deserialize(body); + } + catch (JsonException) + { + // The platform always sends a typed error body, so a non-JSON body means something + // upstream of it answered. The status code is then the only usable signal. + } + + _logger.LogError( + "Failed to {Operation}. Status: {StatusCode}. {Message}", + operationName, + statusCode, + error?.Error ?? "No error detail was returned."); + + if (statusCode == HttpStatusCode.Forbidden) + { + if (string.Equals(error?.Error, "insufficient_scope", StringComparison.OrdinalIgnoreCase)) + { + _logger.LogError(ErrorMessages.GsaScopeMissing, + error?.RequiredScope ?? (operationName == "read Global Secure Access status" + ? "AgentTools.Gsa.Read.All" : "AgentTools.Gsa.Manage.All")); + } + else + { + _logger.LogError( + "This command requires the Global Administrator or Power Platform Administrator role."); + } + } + + if (statusCode == HttpStatusCode.Conflict) + { + // Retrying cannot fix this one, so say why rather than letting it look transient. + _logger.LogError( + "A Power Platform policy governs this setting. Change it through that policy instead."); + } + + _logger.LogError("Correlation ID: {CorrelationId}", correlationId); + } + +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IGsaService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IGsaService.cs new file mode 100644 index 00000000..bf00cdfa --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IGsaService.cs @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Models; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Turns Global Secure Access on and off for the tenant's Agent 365 Power Platform environment +/// through the Agent 365 platform, which resolves that environment itself. +/// +/// +/// Every method is told which Azure account to act as rather than reading the Azure CLI itself. +/// az account show reflects mutable local state, so resolving it once for the confirmation +/// prompt and again for authentication could confirm one tenant and change another. +/// +public interface IGsaService +{ + /// + /// Sets Global Secure Access to the requested value. + /// + /// The Azure account to authenticate as. Its tenant is the tenant changed. + /// The value to apply. + /// Cancellation token. + /// The resulting status, or null when the change could not be requested. + Task SetAsync( + AzureAccountInfo account, + bool enabled, + CancellationToken cancellationToken = default); + + /// + /// Reads the current Global Secure Access setting. + /// + /// The Azure account to authenticate as. Its tenant is the tenant read. + /// Cancellation token. + /// The current status, or null when it could not be read. + Task GetStatusAsync( + AzureAccountInfo account, + CancellationToken cancellationToken = default); + + /// + /// Polls status until the environment reports the requested value or the timeout elapses. + /// + /// The Azure account to authenticate as. Its tenant is the tenant polled. + /// The status being waited for, Enabled or Disabled. + /// How long to keep polling. + /// Cancellation token. + /// The last status read, which may still differ if the timeout elapsed. + Task WaitForStatusAsync( + AzureAccountInfo account, + string expectedStatus, + TimeSpan timeout, + CancellationToken cancellationToken = default); +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/HttpClientFactory.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/HttpClientFactory.cs index 1b2bb95d..fff80898 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/HttpClientFactory.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/HttpClientFactory.cs @@ -20,6 +20,13 @@ public static class HttpClientFactory /// Optional correlation ID for request tracing. If null, empty, or whitespace, /// a new GUID will be generated automatically. /// + /// + /// Optional message handler. It stays owned by whoever supplied it: the returned client does + /// not dispose it, so one handler can back several clients. Callers that build a client per + /// request from a handler they hold as a field depend on this — the default + /// ownership would let the first client's disposal take the shared + /// handler down and fail every later request with . + /// /// A configured HttpClient instance with the correlation ID applied. public static HttpClient CreateAuthenticatedClient( string? authToken = null, @@ -28,7 +35,7 @@ public static HttpClient CreateAuthenticatedClient( HttpMessageHandler? handler = null) { var client = handler != null - ? new HttpClient(handler) { Timeout = TimeSpan.FromMinutes(2) } + ? new HttpClient(handler, disposeHandler: false) { Timeout = TimeSpan.FromMinutes(2) } : new HttpClient { Timeout = TimeSpan.FromMinutes(2) }; if (!string.IsNullOrWhiteSpace(authToken)) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs new file mode 100644 index 00000000..750171a2 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs @@ -0,0 +1,393 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Commands; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using System.CommandLine; +using System.CommandLine.Parsing; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; + +/// +/// Unit tests for the network command tree, its handlers and its result reporting. +/// Handlers are driven through InvokeAsync against substituted services so that tenant +/// resolution, confirmation and exit codes are covered, not just option parsing. +/// +public class NetworkCommandTests +{ + private const string TenantId = "tid"; + + private static Command CreateCommand( + IAzureCliService? azure = null, + IConfirmationProvider? confirmation = null, + IGsaService? gsa = null, + ILogger? logger = null) => + NetworkCommand.CreateCommand( + logger ?? NullLogger.Instance, + azure ?? SignedInAzureCli(), + gsa ?? Substitute.For(), + confirmation ?? Confirming(true)); + + private static IAzureCliService SignedInAzureCli(string? tenantId = TenantId) + { + var azure = Substitute.For(); + azure.GetCurrentAccountAsync().Returns( + Task.FromResult(tenantId == null ? null : new AzureAccountInfo + { + TenantId = tenantId, + User = new AzureUser { Name = "admin@contoso.com" }, + })); + return azure; + } + + private static AzureAccountInfo Account(string tenantId = TenantId) => + new() { TenantId = tenantId }; + + private static IConfirmationProvider Confirming(bool answer) + { + var confirmation = Substitute.For(); + confirmation.ConfirmAsync(Arg.Any()).Returns(Task.FromResult(answer)); + return confirmation; + } + + // ──────────────────────────── Command tree shape ──────────────────────────── + + [Fact] + public void CreateCommand_ExposesTheGsaSubcommandTree() + { + var command = CreateCommand(); + + command.Name.Should().Be("network"); + command.Subcommands.Select(c => c.Name).Should().BeEquivalentTo("gsa"); + + var gsa = command.Subcommands.Single(c => c.Name == "gsa"); + gsa.Subcommands.Select(c => c.Name).Should().BeEquivalentTo("enable", "disable", "status"); + } + + // ─────────────────────────── GSA subcommand shape ─────────────────────────── + + [Theory] + [InlineData("enable")] + [InlineData("disable")] + public void GsaSetSubcommands_OfferWaitYesAndVerboseOnly(string name) + { + var gsa = CreateCommand().Subcommands.Single(c => c.Name == "gsa"); + + var subcommand = gsa.Subcommands.Single(c => c.Name == name); + + subcommand.Options.Select(o => o.Name).Should().BeEquivalentTo("wait", "yes", "verbose"); + } + + // ───────────────────────── GSA handler invocation ─────────────────────────── + + [Theory] + [InlineData("enable", true)] + [InlineData("disable", false)] + public async Task GsaSetHandler_PromptsThenAppliesTheRequestedValue(string verb, bool enabled) + { + var gsa = Substitute.For(); + gsa.SetAsync(Arg.Any(), enabled, Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = enabled ? "Enabled" : "Disabled" })); + var confirmation = Confirming(true); + var command = CreateCommand(confirmation: confirmation, gsa: gsa); + + var exitCode = await command.InvokeAsync($"gsa {verb}"); + + exitCode.Should().Be(0); + await confirmation.Received(1).ConfirmAsync(Arg.Any()); + await gsa.Received(1).SetAsync(Arg.Any(), enabled, Arg.Any()); + } + + [Theory] + [InlineData("enable")] + [InlineData("disable")] + public async Task GsaSetHandler_WhenDeclined_DoesNotCallTheService(string verb) + { + var gsa = Substitute.For(); + var command = CreateCommand(confirmation: Confirming(false), gsa: gsa); + + var exitCode = await command.InvokeAsync($"gsa {verb}"); + + exitCode.Should().Be(1); + await gsa.DidNotReceive().SetAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GsaSetHandler_WithYes_SkipsThePrompt() + { + var gsa = Substitute.For(); + gsa.SetAsync(Arg.Any(), true, Arg.Any()) + .Returns(Task.FromResult(new GsaStatusResponse { Status = "Enabled" })); + var confirmation = Confirming(false); + var command = CreateCommand(confirmation: confirmation, gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa enable --yes"); + + exitCode.Should().Be(0); + await confirmation.DidNotReceive().ConfirmAsync(Arg.Any()); + await gsa.Received(1).SetAsync(Arg.Any(), true, Arg.Any()); + } + + [Fact] + public async Task GsaSetHandler_WhenNoTenantCanBeResolved_FailsWithoutCallingTheService() + { + var gsa = Substitute.For(); + var command = CreateCommand(azure: SignedInAzureCli(tenantId: null), gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa enable"); + + exitCode.Should().Be(1); + await gsa.DidNotReceive().SetAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GsaSetHandler_ReadsTheAzAccountOnceAndActsOnTheTenantItConfirmed() + { + // az account show reads mutable local state, so a second read for authentication could + // confirm one tenant and change another. The account is resolved once and passed down. + const string tenantId = "44444444-4444-4444-4444-444444444444"; + var azure = SignedInAzureCli(tenantId); + var gsa = Substitute.For(); + gsa.SetAsync(Arg.Any(), true, Arg.Any()) + .Returns(Task.FromResult(new GsaStatusResponse { Status = "Enabled" })); + var confirmation = Confirming(true); + var command = CreateCommand(azure: azure, confirmation: confirmation, gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa enable"); + + exitCode.Should().Be(0); + await azure.Received(1).GetCurrentAccountAsync(); + await confirmation.Received(1).ConfirmAsync(Arg.Is(m => + m.Contains(tenantId) && m.Contains("admin@contoso.com") && m.EndsWith("(y/N): "))); + await gsa.Received(1).SetAsync( + Arg.Is(a => a.TenantId == tenantId), true, Arg.Any()); + } + + [Fact] + public async Task GsaStatusHandler_WhenNoTenantCanBeResolved_FailsWithoutCallingTheService() + { + var gsa = Substitute.For(); + var command = CreateCommand(azure: SignedInAzureCli(tenantId: null), gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa status"); + + exitCode.Should().Be(1); + await gsa.DidNotReceive().GetStatusAsync( + Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GsaSetHandler_WhenTheServiceFails_ReturnsFailure() + { + var gsa = Substitute.For(); + gsa.SetAsync(Arg.Any(), true, Arg.Any()).Returns(Task.FromResult(null)); + var command = CreateCommand(gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa enable --yes"); + + exitCode.Should().Be(1); + } + + [Fact] + public async Task GsaSetHandler_WithWait_PollsForTheRequestedValue() + { + var gsa = Substitute.For(); + gsa.SetAsync(Arg.Any(), true, Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = "Disabled", Pending = true })); + gsa.WaitForStatusAsync(Arg.Any(), "Enabled", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new GsaStatusResponse { Status = "Enabled" })); + var command = CreateCommand(gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa enable --yes --wait"); + + exitCode.Should().Be(0); + await gsa.Received(1).WaitForStatusAsync(Arg.Any(), "Enabled", Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GsaStatusHandler_ReadsStatusWithoutPrompting() + { + var gsa = Substitute.For(); + gsa.GetStatusAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new GsaStatusResponse { Status = "Enabled" })); + var confirmation = Confirming(false); + var command = CreateCommand(confirmation: confirmation, gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa status"); + + exitCode.Should().Be(0); + await confirmation.DidNotReceive().ConfirmAsync(Arg.Any()); + await gsa.Received(1).GetStatusAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GsaStatusHandler_WhenStatusUnreadable_ReturnsFailure() + { + var gsa = Substitute.For(); + gsa.GetStatusAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(null)); + var command = CreateCommand(gsa: gsa); + + var exitCode = await command.InvokeAsync("gsa status"); + + exitCode.Should().Be(1); + } + + [Fact] + public void GsaStatusSubcommand_TakesNoOperationHandle() + { + var gsa = CreateCommand().Subcommands.Single(c => c.Name == "gsa"); + + var status = gsa.Subcommands.Single(c => c.Name == "status"); + + // GSA converges on re-read rather than issuing a handle, so there is nothing to look up. + status.Options.Select(o => o.Name).Should().BeEquivalentTo(new[] { "verbose" }); + } + + [Fact] + public void GsaEnableSubcommand_ParsesItsOptions() + { + var parsed = CreateCommand().Parse("gsa enable --wait"); + + parsed.Errors.Should().BeEmpty(); + } + + // ──────────────────────────────── ReportGsaAsync ──────────────────────────── + + [Fact] + public async Task ReportGsaAsync_WhenResultNull_ReturnsFailure() + { + var gsa = Substitute.For(); + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, gsa, Account(), result: null, wait: true, enabled: true, CancellationToken.None); + + exitCode.Should().Be(1); + await gsa.DidNotReceive().WaitForStatusAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ReportGsaAsync_WhenSettled_ReturnsSuccessWithoutWaiting() + { + var gsa = Substitute.For(); + var result = new GsaStatusResponse { Status = "Enabled", Pending = false }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, gsa, Account(), result, wait: true, enabled: true, CancellationToken.None); + + exitCode.Should().Be(0); + await gsa.DidNotReceive().WaitForStatusAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ReportGsaAsync_WhenPendingAndNotWaiting_ReturnsSuccess() + { + var gsa = Substitute.For(); + var logger = Substitute.For(); + var result = new GsaStatusResponse { Status = "Disabled", Pending = true }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + logger, gsa, Account(), result, wait: false, enabled: true, CancellationToken.None); + + exitCode.Should().Be(0, because: "an accepted change that has not surfaced yet is not a failure"); + var output = string.Join("\n", logger.ReceivedCalls() + .Where(c => c.GetMethodInfo().Name == "Log").Select(c => c.GetArguments()[2]?.ToString())); + output.Should().Contain("Requested: Enabled").And.Contain("Current: Disabled") + .And.Contain("a365 network gsa status", + because: "acceptance must not be presented as the requested setting already being effective"); + await gsa.DidNotReceive().WaitForStatusAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory] + [InlineData(true, "Enabled")] + [InlineData(false, "Disabled")] + public async Task ReportGsaAsync_WhenPendingAndWaiting_PollsForTheRequestedStatus( + bool enabled, string expectedStatus) + { + var gsa = Substitute.For(); + gsa.WaitForStatusAsync(Arg.Any(), expectedStatus, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = expectedStatus, Pending = false })); + var result = new GsaStatusResponse { Status = "NotConfigured", Pending = true }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, gsa, Account(), result, wait: true, enabled, CancellationToken.None); + + exitCode.Should().Be(0); + await gsa.Received(1).WaitForStatusAsync( + Arg.Any(), expectedStatus, Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ReportGsaAsync_WhenWaitCannotReadStatus_ReturnsFailure() + { + var gsa = Substitute.For(); + var logger = Substitute.For(); + gsa.WaitForStatusAsync(Arg.Any(), "Enabled", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(null)); + var result = new GsaStatusResponse { Status = "Disabled", Pending = true }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + logger, gsa, Account(), result, wait: true, enabled: true, CancellationToken.None); + + exitCode.Should().Be(1); + var output = string.Join("\n", logger.ReceivedCalls() + .Where(c => c.GetMethodInfo().Name == "Log").Select(c => c.GetArguments()[2]?.ToString())); + output.Should().Contain("accepted").And.Contain("a365 network gsa status", + because: "a failed verification must not imply the accepted change was rejected"); + } + + [Theory] + [InlineData(true, "Disabled", "Enabled")] + [InlineData(false, "Enabled", "Disabled")] + public async Task GsaSetHandler_WhenWaitTimesOut_ReportsUnconfirmedStateAndReturnsFailure( + bool enabled, string currentStatus, string expectedStatus) + { + var gsa = Substitute.For(); + var logger = Substitute.For(); + gsa.SetAsync(Arg.Any(), enabled, Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = currentStatus, Pending = true })); + gsa.WaitForStatusAsync(Arg.Any(), expectedStatus, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = currentStatus, Pending = false })); + var command = CreateCommand(gsa: gsa, logger: logger); + + var exitCode = await command.InvokeAsync($"gsa {(enabled ? "enable" : "disable")} --yes --wait"); + + exitCode.Should().Be(1, + because: "--wait requires observing the requested value, not just acceptance of the change"); + var output = string.Join("\n", logger.ReceivedCalls() + .Where(c => c.GetMethodInfo().Name == "Log").Select(c => c.GetArguments()[2]?.ToString())); + output.Should().Contain($"not {expectedStatus}").And.Contain("accepted") + .And.Contain("a365 network gsa status", + because: "status reads do not carry pending, so a mismatch must explicitly report incomplete verification"); + } + + [Fact] + public async Task ReportGsaAsync_WithAReasonOnASettledResult_StillSucceeds() + { + var gsa = Substitute.For(); + var result = new GsaStatusResponse + { + Status = "NotConfigured", + Pending = false, + Reason = "This tenant has no Agent 365 environment yet.", + }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, gsa, Account(), result, wait: false, enabled: false, CancellationToken.None); + + exitCode.Should().Be(0); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs new file mode 100644 index 00000000..af1fbf18 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs @@ -0,0 +1,658 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Diagnostics; +using System.Net; +using System.Text.Json; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +/// +/// Unit tests for GsaService. +/// Uses TestHttpMessageHandler / CapturingHttpMessageHandler (defined in GraphApiServiceTests.cs, +/// same assembly) to inject fake platform responses. +/// +public class GsaServiceTests +{ + private static IAuthenticationService FakeAuth(string token = "fake-a365-token") + { + var mock = Substitute.For(); + + mock.GetAccessTokenAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any()) + .Returns(Task.FromResult(token)); + return mock; + } + + private static AzureAccountInfo Account( + string tenantId = "11111111-1111-1111-1111-111111111111", + string upn = "admin@contoso.onmicrosoft.com") => + new() + { + TenantId = tenantId, + User = new AzureUser { Name = upn }, + }; + + private static GsaService CreateService( + HttpMessageHandler handler, + IAuthenticationService? auth = null) => + new(NullLogger.Instance, auth ?? FakeAuth(), "prod", handler); + + private static HttpResponseMessage StatusResponse( + HttpStatusCode code, + string? status = null, + bool pending = false, + string? reason = null) => + new(code) + { + Content = new StringContent(JsonSerializer.Serialize(new { status, pending, reason })), + }; + + // ───────────────────────────────── SetAsync ───────────────────────────────── + + [Theory] + [InlineData(true, "/agents/gsa/enable")] + [InlineData(false, "/agents/gsa/disable")] + public async Task SetAsync_PostsToTheRouteThatCarriesTheIntent(bool enabled, string expectedPath) + { + HttpMethod? method = null; + Uri? uri = null; + using var handler = new CapturingHttpMessageHandler(r => + { + method = r.Method; + uri = r.RequestUri; + }); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, enabled ? "Enabled" : "Disabled")); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled); + + result.Should().NotBeNull(); + result!.Status.Should().Be(enabled ? "Enabled" : "Disabled"); + result.Pending.Should().BeFalse(); + result.Reason.Should().BeNull(); + + method.Should().Be(HttpMethod.Post); + uri!.AbsolutePath.Should().Be(expectedPath); + } + + [Fact] + public async Task SetAsync_SendsNoEnvironmentIdentifierBecauseThePlatformResolvesIt() + { + string? body = null; + Uri? uri = null; + using var handler = new CapturingHttpMessageHandler(r => + { + uri = r.RequestUri; + body = r.Content?.ReadAsStringAsync().GetAwaiter().GetResult(); + }); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Enabled")); + var svc = CreateService(handler); + + await svc.SetAsync(Account(), enabled: true); + + body.Should().BeEmpty(); + uri!.Query.Should().BeEmpty(); + } + + [Fact] + public async Task SetAsync_WhenAccepted_SurfacesThePendingFlagWithTheOldStatus() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.Accepted, "Disabled", pending: true)); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled: true); + + result.Should().NotBeNull(); + result!.Status.Should().Be("Disabled"); + result.Pending.Should().BeTrue(); + result.Reason.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_WhenGovernedByPolicy_ReturnsNull() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Conflict) + { + Content = new StringContent(JsonSerializer.Serialize(new + { + error = "A Power Platform policy governs this setting.", + })), + }); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled: true); + + result.Should().BeNull(); + handler.RequestCount.Should().Be(1, because: "a governed setting cannot be fixed by retrying"); + } + + [Theory] + [InlineData(HttpStatusCode.Forbidden)] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.BadGateway)] + public async Task SetAsync_WhenTheCallFails_ReturnsNull(HttpStatusCode code) + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(code) + { + Content = new StringContent(JsonSerializer.Serialize(new { error = "nope" })), + }); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled: false); + + result.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_WhenTheErrorBodyIsNotJson_StillReturnsNullWithoutThrowing() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.BadGateway) + { + Content = new StringContent("gateway"), + }); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled: true); + + result.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_WhenNoTokenIsAvailable_ReturnsNullWithoutCallingThePlatform() + { + using var handler = new TestHttpMessageHandler(); + var svc = CreateService(handler, FakeAuth(token: string.Empty)); + + var result = await svc.SetAsync(Account(), enabled: true); + + result.Should().BeNull(); + handler.RequestCount.Should().Be(0); + } + + [Fact] + public async Task SetAsync_WhenTheTransportThrows_ReturnsNull() + { + using var handler = new ExceptionThrowingHttpMessageHandler( + () => new HttpRequestException("connection reset")); + var svc = CreateService(handler); + + var result = await svc.SetAsync(Account(), enabled: true); + + result.Should().BeNull(); + } + + // ─────────────────────────── Tenant targeting ─────────────────────────── + // + // The tenant of the current az login is passed explicitly to token acquisition. Without it + // the authority is "common", and the Windows broker silently returns the Windows account even + // when a login hint names a different one — which would apply a tenant-wide setting to the + // wrong tenant. Passing the tenant also arms the mismatch self-heal in AuthenticationService. + + [Fact] + public async Task SetAsync_AuthenticatesAgainstTheTenantAndUserOfTheCurrentAzLogin() + { + const string tenantId = "22222222-2222-2222-2222-222222222222"; + const string upn = "admin@fabrikam.onmicrosoft.com"; + var auth = FakeAuth(); + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Enabled")); + var svc = CreateService(handler, auth); + + await svc.SetAsync(Account(tenantId, upn), enabled: true); + + await auth.Received(1).GetAccessTokenAsync( + Arg.Any(), + tenantId, + Arg.Any(), + Arg.Any(), + Arg.Any?>(), + Arg.Any(), + upn, + Arg.Any(), + ConfigConstants.GetAuthorityHost("prod")); + } + + [Fact] + public async Task GetStatusAsync_AuthenticatesAgainstTheTenantAndUserOfTheCurrentAzLogin() + { + const string tenantId = "33333333-3333-3333-3333-333333333333"; + const string upn = "reader@fabrikam.onmicrosoft.com"; + var auth = FakeAuth(); + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Disabled")); + var svc = CreateService(handler, auth); + + await svc.GetStatusAsync(Account(tenantId, upn)); + + await auth.Received(1).GetAccessTokenAsync( + Arg.Any(), + tenantId, + Arg.Any(), + Arg.Any(), + Arg.Any?>(), + Arg.Any(), + upn, + Arg.Any(), + ConfigConstants.GetAuthorityHost("prod")); + } + + // ──────────────────────────────── GetStatusAsync ──────────────────────────── + + [Fact] + public async Task GetStatusAsync_GetsTheStatusRoute() + { + HttpMethod? method = null; + Uri? uri = null; + using var handler = new CapturingHttpMessageHandler(r => + { + method = r.Method; + uri = r.RequestUri; + }); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "NotConfigured")); + var svc = CreateService(handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().NotBeNull(); + result!.Status.Should().Be("NotConfigured"); + result.Pending.Should().BeFalse(); + result.Reason.Should().BeNull(); + + method.Should().Be(HttpMethod.Get); + uri!.AbsolutePath.Should().Be("/agents/gsa/status"); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("{}")] + [InlineData("{\"status\":null}")] + [InlineData("{\"status\":\" \"}")] + public async Task GetStatusAsync_WhenTheBodyHasNoStatus_ReturnsFailure(string body) + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(body), + }); + var svc = CreateService(handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().BeNull( + because: "a successful HTTP code without a readable status must not make the CLI report success"); + } + + [Theory] + [InlineData("gcc")] + [InlineData("gcc-moderate")] + [InlineData("gcc-high")] + [InlineData("dod")] + [InlineData("AzureUSGovernment")] + public async Task GsaOperations_OutsidePublicCloud_FailBeforeAuthentication(string environment) + { + var auth = FakeAuth(); + using var handler = new TestHttpMessageHandler(); + var svc = new GsaService(NullLogger.Instance, auth, environment, handler); + + var status = await svc.GetStatusAsync(Account()); + var change = await svc.SetAsync(Account(), enabled: true); + + status.Should().BeNull(because: "GSA is supported only in public cloud"); + change.Should().BeNull(because: "unsupported clouds must not send tenant-wide changes"); + auth.ReceivedCalls().Should().BeEmpty( + because: "unsupported clouds must be rejected before acquiring a commercial-cloud token"); + handler.RequestCount.Should().Be(0); + } + + [Theory] + [InlineData("AgentTools.Gsa.Read.All")] + [InlineData("AgentTools.Gsa.Manage.All")] + public async Task GetStatusAsync_WhenScopeIsMissing_ReportsRequiredScopeWithoutAdminConsentAdvice(string scope) + { + var logger = Substitute.For>(); + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Forbidden) + { + Content = new StringContent(JsonSerializer.Serialize(new + { + error = "insufficient_scope", + required_scope = scope, + })), + }); + var svc = new GsaService(logger, FakeAuth(), "prod", handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().BeNull(); + handler.RequestCount.Should().Be(1, because: "a missing scope cannot be repaired by polling"); + var output = string.Join("\n", logger.ReceivedCalls() + .Where(c => c.GetMethodInfo().Name == "Log").Select(c => c.GetArguments()[2]?.ToString())); + output.Should().Contain(scope, because: "scope errors must identify the platform's required permission"); + output.Should().NotContain("requires the Global Administrator") + .And.NotContain("client application consented", + because: "a missing preauthorized scope is not a role failure or something an admin can grant on Microsoft's client"); + } + + [Fact] + public async Task GetStatusAsync_SurfacesTheReason() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse( + HttpStatusCode.OK, "NotConfigured", reason: "This tenant has no Agent 365 environment yet.")); + var svc = CreateService(handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().NotBeNull(); + result!.Status.Should().Be("NotConfigured"); + result.Pending.Should().BeFalse(); + result.Reason.Should().Be("This tenant has no Agent 365 environment yet."); + } + + // ─────────────────────────────── WaitForStatusAsync ───────────────────────── + + [Fact] + public async Task WaitForStatusAsync_WithoutAnExpectedStatus_Throws() + { + using var handler = new TestHttpMessageHandler(); + var svc = CreateService(handler); + + var act = () => svc.WaitForStatusAsync(Account(), " ", TimeSpan.FromMinutes(1)); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task WaitForStatusAsync_WhenTheFirstReadAlreadyMatches_StopsImmediately() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Enabled")); + var svc = CreateService(handler); + + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMinutes(1)); + + result.Should().NotBeNull(); + result!.Status.Should().Be("Enabled"); + result.Pending.Should().BeFalse(); + result.Reason.Should().BeNull(); + handler.RequestCount.Should().Be(1); + } + + [Fact] + public async Task WaitForStatusAsync_MatchesStatusCaseInsensitively() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "enabled")); + var svc = CreateService(handler); + + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMinutes(1)); + + result.Should().NotBeNull(); + result!.Status.Should().Be("enabled"); + result.Pending.Should().BeFalse(); + result.Reason.Should().BeNull(); + handler.RequestCount.Should().Be(1); + } + + [Fact] + public async Task WaitForStatusAsync_WhenReadsKeepFailing_StopsAfterBoundedRetries() + { + using var handler = new TestHttpMessageHandler(); + for (var i = 0; i < 3; i++) + { + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.BadGateway) + { + Content = new StringContent(JsonSerializer.Serialize(new { error = "upstream" })), + }); + } + var svc = CreateService(handler); + + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMinutes(1)); + + result.Should().BeNull(); + handler.RequestCount.Should().Be(3, + because: "transient status failures must be retried but cannot spin indefinitely"); + } + + [Fact] + public async Task WaitForStatusAsync_WhenAReadFailsTransiently_ContinuesToTheRequestedState() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.BadGateway) + { + Content = new StringContent("{\"error\":\"upstream\"}"), + }); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Enabled")); + var svc = CreateService(handler); + + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMinutes(1)); + + result.Should().NotBeNull(because: "a transient gateway error must not abort verification of an accepted change"); + result!.Status.Should().Be("Enabled"); + handler.RequestCount.Should().Be(2); + } + + [Fact] + public async Task WaitForStatusAsync_WhenTheBudgetCannotCoverAnotherPoll_ReturnsTheLastRead() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Disabled")); + var svc = CreateService(handler); + + // Shorter than the poll interval, so the first non-matching read is also the last. + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromSeconds(1)); + + result.Should().NotBeNull(); + result!.Status.Should().Be("Disabled"); + result.Pending.Should().BeFalse(because: "status reads never carry the mutation response's pending flag"); + result.Reason.Should().BeNull(); + handler.RequestCount.Should().Be(1); + } + + [Fact] + public async Task WaitForStatusAsync_WhenCeilingElapsesDuringAPoll_StopsWaitingOnTheInFlightCall() + { + // The pre-sleep stopwatch check only bounds the gap between completed polls. Without the + // ceiling armed on the request's own token, a poll that starts inside the budget runs to + // the HttpClient's timeout -- minutes past what the caller asked for. + using var handler = new SlowHttpMessageHandler(TimeSpan.FromSeconds(30)); + var svc = CreateService(handler); + + var stopwatch = Stopwatch.StartNew(); + var result = await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMilliseconds(200)); + stopwatch.Stop(); + + result.Should().BeNull(because: "the ceiling elapsed before any status was read"); + stopwatch.Elapsed.Should().BeLessThan( + TimeSpan.FromSeconds(10), + because: "the wait must abandon the in-flight request rather than block on it"); + } + + [Fact] + public async Task WaitForStatusAsync_WhenCallerCancels_PropagatesRatherThanReportingATimeout() + { + // The timeout and a Ctrl+C both surface as OperationCanceledException. Only the timeout is + // swallowed into "still applying"; a caller cancel has to reach the caller. + using var handler = new SlowHttpMessageHandler(TimeSpan.FromSeconds(30)); + var svc = CreateService(handler); + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200)); + + var act = async () => await svc.WaitForStatusAsync(Account(), "Enabled", TimeSpan.FromMinutes(5), cts.Token); + + await act.Should().ThrowAsync(); + } + + // ───────────────── The account is supplied, never read from the CLI ───────── + // + // az account show reflects mutable local state. Resolving it once for the confirmation prompt + // and again here would let the command confirm one tenant and change another, so the caller + // resolves it once and every method is told which account to act as. + + [Fact] + public async Task SetAsync_WithoutAnAccount_Throws() + { + using var handler = new TestHttpMessageHandler(); + var svc = CreateService(handler); + + var act = async () => await svc.SetAsync(null!, enabled: true); + + await act.Should().ThrowAsync().WithParameterName("account"); + handler.RequestCount.Should().Be(0); + } + + [Fact] + public async Task GetStatusAsync_WithoutAnAccount_Throws() + { + using var handler = new TestHttpMessageHandler(); + var svc = CreateService(handler); + + var act = async () => await svc.GetStatusAsync(null!); + + await act.Should().ThrowAsync().WithParameterName("account"); + handler.RequestCount.Should().Be(0); + } + + [Fact] + public async Task WaitForStatusAsync_WithoutAnAccount_Throws() + { + using var handler = new TestHttpMessageHandler(); + var svc = CreateService(handler); + + var act = async () => await svc.WaitForStatusAsync(null!, "Enabled", TimeSpan.FromMinutes(1)); + + await act.Should().ThrowAsync().WithParameterName("account"); + handler.RequestCount.Should().Be(0); + } + + [Fact] + public async Task GetStatusAsync_WhenTheTransportTimesOutWithNoCancellation_ReturnsNullRatherThanThrowing() + { + // HttpClient's own timeout surfaces as an OperationCanceledException with no token + // cancelled. That is an ordinary request failure, and callers of a bare status, enable or + // disable expect the documented null, not an exception thrown at them. + using var handler = new ThrowingHttpMessageHandler(new TaskCanceledException("The request timed out.")); + var svc = CreateService(handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().BeNull(); + } + + [Fact] + public async Task GetStatusAsync_WhenTheCallerCancels_PropagatesRatherThanReturningNull() + { + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + using var handler = new ThrowingHttpMessageHandler(new TaskCanceledException("Cancelled.")); + var svc = CreateService(handler); + + var act = async () => await svc.GetStatusAsync(Account(), cts.Token); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task GetStatusAsync_CalledTwice_DoesNotDisposeTheInjectedHandlerOnTheFirstCall() + { + // A client is built per request, but the handler is a field and outlives all of them. + // HttpClient's default ownership would have the first client's disposal take the handler + // down with it, so every later request -- including every poll after the first in + // WaitForStatusAsync -- would fail with ObjectDisposedException. + using var handler = new DisposalAwareHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Disabled")); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Enabled")); + var svc = CreateService(handler); + + var first = await svc.GetStatusAsync(Account()); + var second = await svc.GetStatusAsync(Account()); + + first.Should().NotBeNull(); + first!.Status.Should().Be("Disabled"); + first.Pending.Should().BeFalse(); + first.Reason.Should().BeNull(); + + second.Should().NotBeNull(); + second!.Status.Should().Be("Enabled"); + second.Pending.Should().BeFalse(); + second.Reason.Should().BeNull(); + + handler.DisposeCount.Should().Be(0); + handler.RequestCount.Should().Be(2); + } + + /// + /// Fails every request with a supplied exception, so a test can pin how the service classifies + /// it without racing a real timeout. + /// + private sealed class ThrowingHttpMessageHandler(Exception exception) : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + throw exception; + } + } + + /// + /// Holds each request open until the request's own token is cancelled, so a test can tell + /// "abandoned the call" from "waited for the response". + /// + private sealed class SlowHttpMessageHandler(TimeSpan delay) : HttpMessageHandler + { + protected override async Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + await Task.Delay(delay, cancellationToken); + return new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new { status = "Enabled", pending = false })), + }; + } + } + + /// + /// Mimics a real handler's reaction to being disposed: it counts disposals and refuses to + /// serve afterwards, so a test can prove the service never disposes a handler it does not own. + /// A handler that ignores Dispose would let the defect pass unnoticed. + /// + private sealed class DisposalAwareHttpMessageHandler : HttpMessageHandler + { + private readonly Queue _responses = new(); + + public int DisposeCount { get; private set; } + + public int RequestCount { get; private set; } + + public void QueueResponse(HttpResponseMessage response) => _responses.Enqueue(response); + + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + ObjectDisposedException.ThrowIf(DisposeCount > 0, this); + RequestCount++; + return Task.FromResult(_responses.Dequeue()); + } + + protected override void Dispose(bool disposing) + { + DisposeCount++; + base.Dispose(disposing); + } + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Internal/HttpClientFactoryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Internal/HttpClientFactoryTests.cs index f91a6fa8..f1e81b01 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Internal/HttpClientFactoryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Internal/HttpClientFactoryTests.cs @@ -243,4 +243,54 @@ public void CreateAuthenticatedClient_WithGeneratedCorrelationId_BothHeadersMatc correlationId.Should().Be(clientRequestId, "Both headers should have the same auto-generated correlation ID"); } + + [Fact] + public void CreateAuthenticatedClient_WithASuppliedHandler_LeavesTheHandlerAliveAfterTheClientIsDisposed() + { + // A supplied handler belongs to the caller, who commonly holds one as a field and builds a + // client per request. HttpClient's default ownership would have the first client's disposal + // take that handler down, failing every later request with ObjectDisposedException. + using var handler = new CountingHttpMessageHandler(); + + using (HttpClientFactory.CreateAuthenticatedClient(handler: handler)) + { + } + + handler.DisposeCount.Should().Be(0); + } + + [Fact] + public void CreateAuthenticatedClient_WithASuppliedHandler_CanBackSeveralClients() + { + using var handler = new CountingHttpMessageHandler(); + + var first = HttpClientFactory.CreateAuthenticatedClient(handler: handler); + using var second = HttpClientFactory.CreateAuthenticatedClient(handler: handler); + + first.Should().NotBeSameAs(second); + + // Disposing one client must not take the shared handler, or the other client is already + // broken before it sends anything. + first.Dispose(); + + handler.DisposeCount.Should().Be(0); + } + + /// + /// Counts disposals so a test can pin who owns a supplied handler. + /// + private sealed class CountingHttpMessageHandler : HttpMessageHandler + { + public int DisposeCount { get; private set; } + + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) => + throw new NotSupportedException("This handler exists only to observe disposal."); + + protected override void Dispose(bool disposing) + { + DisposeCount++; + base.Dispose(disposing); + } + } } \ No newline at end of file