From c7ef78875bdde57342badba298f43eb1baf1fb7c Mon Sep 17 00:00:00 2001 From: "Lala Sushant Srivastava (from Dev Box)" Date: Fri, 30 Oct 2026 15:06:21 -0700 Subject: [PATCH] Add GSA network commands and address PR review feedback Rebuild the GSA-only change set on current main, dropping shared VNet history. Require --wait to observe the requested value, reject unreadable success responses and unsupported clouds, retry transient reads, and distinguish scope errors from role failures. Clarify prompts, pending output, prerequisites and release notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0ea31405-e8a4-41a7-b9ec-13e2f66d65a7 --- CHANGELOG.md | 3 + docs/commands/README.md | 4 + docs/commands/network-gsa.md | 104 +++ .../Commands/NetworkCommand.cs | 262 +++++++ .../Constants/CommandNames.cs | 1 + .../Constants/ErrorMessages.cs | 10 + .../Models/GsaModels.cs | 50 ++ .../Program.cs | 13 + .../Services/GsaService.cs | 274 ++++++++ .../Services/IGsaService.cs | 54 ++ .../Services/Internal/HttpClientFactory.cs | 9 +- .../Commands/NetworkCommandTests.cs | 393 +++++++++++ .../Services/GsaServiceTests.cs | 658 ++++++++++++++++++ .../Internal/HttpClientFactoryTests.cs | 50 ++ 14 files changed, 1884 insertions(+), 1 deletion(-) create mode 100644 docs/commands/network-gsa.md create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/IGsaService.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs 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