diff --git a/CHANGELOG.md b/CHANGELOG.md index 59aa369b..9834b459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g **Option B — CLI** (`a365 setup admin`) has been removed in this release. Use Option A above, or copy the PowerShell instructions printed in the `a365 setup all` summary output. ### Added +- `a365 network gsa enable|disable|status` — turns Global Secure Access on or off for the tenant's Agent 365 environment. `NotConfigured` is reported distinctly from `Disabled`, because a tenant that has never set the value has not turned it off. Requires Global Administrator or Power Platform Administrator. See [docs/commands/network-gsa.md](docs/commands/network-gsa.md) (#497). - Setup and bootstrap now use Microsoft's first-party Agent 365 CLI application when it is present in your tenant, validating it without changing Microsoft's app registration, and fall back to a tenant-owned "Agent 365 CLI" app when it is not (#489). - Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text. - Authentication context (tenant and user) is now logged at the `Information` level whenever the resolved sign-in identity changes, giving operators a clear audit trail in the log file of who the CLI is acting as, without exposing credentials. diff --git a/docs/commands/README.md b/docs/commands/README.md index 29153973..29dcc110 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](network-gsa.md) | Configure tenant networking 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..f7374519 --- /dev/null +++ b/docs/commands/network-gsa.md @@ -0,0 +1,91 @@ +# `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` to the tenant you intend to configure. +- Public cloud only. Sovereign clouds are not supported. + +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. + +Exit code is `1` on any request error, and `0` otherwise — including a change that is still being +applied, which is a legitimate outcome when `--wait` is not passed. + +## Typical flow + +```bash +a365 network gsa enable --wait +a365 network gsa status +``` + +## Troubleshooting + +| Symptom | Cause | +| --- | --- | +| `403` from the platform | Caller is not a Global or Power Platform Administrator, or the CLI app lacks consent for the `AgentTools.Gsa.*` scopes. | +| `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 ` for the tenant you want to configure. | +| 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..836429b0 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs @@ -0,0 +1,250 @@ +// 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) + { + if (yes) + { + return true; + } + + return await confirmationProvider.ConfirmAsync( + $"{action} for tenant {tenantId}. This changes networking for every Agent 365 agent in the tenant. Continue?"); + } + + /// + /// 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)) + { + 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) + { + return 1; + } + } + + LogGsaStatus(logger, result); + + // Still pending is not a failure. The platform accepted the change and the environment + // will catch up; reporting non-zero here would break scripts that chain on success. + if (result.Pending) + { + logger.LogInformation( + "Still being applied. Check on it with: a365 network gsa status"); + } + + return 0; + } + + private static void LogGsaStatus(ILogger logger, GsaStatusResponse status) + { + logger.LogInformation("Global Secure Access: {Status}", 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/Models/GsaModels.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs new file mode 100644 index 00000000..f06cbf19 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs @@ -0,0 +1,47 @@ +// 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; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 546dd535..73607b22 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -185,6 +185,11 @@ await Task.WhenAll( var logsLogger = serviceProvider.GetRequiredService>(); var logRedactionService = serviceProvider.GetRequiredService(); rootCommand.AddCommand(LogsCommand.CreateCommand(logsLogger, logRedactionService)); + var networkLogger = serviceProvider.GetRequiredService().CreateLogger("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 @@ -376,6 +381,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..301f4991 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs @@ -0,0 +1,251 @@ +// 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) + { + // The ceiling elapsed mid-request. That is a timeout, not a failure: report the + // last known state, exactly as the pre-sleep check below does. + _logger.LogInformation( + "Stopped waiting after {Elapsed:0}s. The change is still being applied.", + 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(); + var baseUrl = BuildBaseUrl(); + var url = $"{baseUrl}{path}"; + + try + { + 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); + 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); + + using var request = new HttpRequestMessage(method, url); + + // The platform derives everything it needs from the token, so enable and disable are + // distinguished by route rather than by a body. + 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, cancellationToken); + var body = await response.Content.ReadAsStringAsync(cancellationToken); + _logger.LogDebug("Response {StatusCode}: {Body}", response.StatusCode, body); + + if (!response.IsSuccessStatusCode) + { + LogFailure(response.StatusCode, body, operationName, correlationId); + return null; + } + + // 200 and 202 share a shape as far as the CLI is concerned: a status, plus a pending + // flag when the change has not surfaced yet. + return string.IsNullOrWhiteSpace(body) + ? new GsaStatusResponse() + : JsonSerializer.Deserialize(body); + } + // 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) + { + string? message = null; + try + { + message = JsonSerializer.Deserialize(body)?.Error; + } + 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, + message ?? "No error detail was returned."); + + if (statusCode == HttpStatusCode.Forbidden) + { + _logger.LogError( + "This command requires the Global Administrator or Power Platform Administrator role, " + + "and a client application consented for AgentTools.Gsa.Manage.All."); + } + + 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); + } + + private string BuildBaseUrl() + { + var discoverUrl = ConfigConstants.GetDiscoverEndpointUrl(_environment); + var uri = new Uri(discoverUrl); + return $"{uri.Scheme}://{uri.Authority}"; + } +} 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..5adf3ae5 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs @@ -0,0 +1,363 @@ +// 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.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) => + NetworkCommand.CreateCommand( + 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 })); + 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))); + 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 result = new GsaStatusResponse { Status = "Disabled", Pending = true }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, 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"); + 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(); + 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( + NullLogger.Instance, gsa, Account(), result, wait: true, enabled: true, CancellationToken.None); + + exitCode.Should().Be(1); + } + + [Fact] + public async Task ReportGsaAsync_WhenStillPendingAfterWaiting_ReturnsSuccess() + { + var gsa = Substitute.For(); + gsa.WaitForStatusAsync(Arg.Any(), "Enabled", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult( + new GsaStatusResponse { Status = "Disabled", Pending = true })); + var result = new GsaStatusResponse { Status = "Disabled", Pending = true }; + + var exitCode = await NetworkCommand.ReportGsaAsync( + NullLogger.Instance, gsa, Account(), result, wait: true, enabled: true, CancellationToken.None); + + exitCode.Should().Be(0, because: "the platform accepted the change; the environment is catching up"); + } + + [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..c3fa808b --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs @@ -0,0 +1,580 @@ +// 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.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services; +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(); + + // The 8th parameter is the CancellationToken. Omitting a matcher for it pins the setup to + // ct == default, so any call carrying a real token -- a caller's, or the wait ceiling's -- + // silently misses and returns null, which the service reports as a failed token acquisition. + mock.GetAccessTokenAsync(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()); + } + + [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()); + } + + // ──────────────────────────────── 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"); + } + + [Fact] + public async Task GetStatusAsync_WhenTheBodyIsEmpty_ReturnsAnEmptyStatus() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(string.Empty), + }); + var svc = CreateService(handler); + + var result = await svc.GetStatusAsync(Account()); + + result.Should().NotBeNull(); + result!.Status.Should().BeNull(); + result.Pending.Should().BeFalse(); + result.Reason.Should().BeNull(); + } + + [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_WhenAReadFails_GivesUpRatherThanSpinning() + { + using var handler = new TestHttpMessageHandler(); + 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(1); + } + + [Fact] + public async Task WaitForStatusAsync_WhenTheBudgetCannotCoverAnotherPoll_ReturnsTheLastRead() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(StatusResponse(HttpStatusCode.OK, "Disabled", pending: true)); + 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().BeTrue(); + 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