diff --git a/CHANGELOG.md b/CHANGELOG.md index f2b2eb14..4677d6a2 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 +- The CLI now asks the Agent 365 service to register its own service principal the first time it runs against a tenant, removing a manual admin step that previously caused sign-in to fail in newly onboarded tenants. Set `A365_DISABLE_SP_PROVISIONING=true` to opt out. - 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. - `a365 develop-mcp evaluate` command for evaluating MCP server tool schema quality — runs deterministic and semantic checks (via GitHub Copilot or Claude Code CLIs), computes maturity scoring, and generates an interactive HTML report diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index bf2fe665..098f5a28 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -97,6 +97,19 @@ public static class ConfigConstants /// public const string ObservabilityApiOtelWriteScope = "Agent365.Observability.OtelWrite"; + /// + /// Global Power Platform API host. Tenant-scoped Agent 365 provisioning routes are reachable + /// here and are proxied to the tenant's home cluster. + /// + public const string ProductionProvisioningBaseUrl = "https://api.powerplatform.com"; + + /// + /// Route that provisions the Agent 365 CLI service principal in the caller's tenant. + /// Format argument 0 is the tenant ID. + /// + public const string Agent365CliProvisionPathFormat = + "/maven/tenants/{0}/agent365/servicePrincipals/agent365Cli/provision?api-version=1"; + /// /// Delegated scope value exposed on the blueprint app registration to enable /// OBO (On-Behalf-Of) callers to acquire tokens scoped to the agent. @@ -176,4 +189,19 @@ public static string GetAgent365ToolsResourceAppId(string environment) return McpConstants.WorkIQToolsProdAppId; } + + /// + /// Environment-aware Agent 365 provisioning service base URL. + /// + public static string GetProvisioningBaseUrl(string? environment) + { + var customEndpoint = Environment.GetEnvironmentVariable( + $"A365_PROVISIONING_ENDPOINT_{environment?.ToUpperInvariant()}") + ?? Environment.GetEnvironmentVariable("A365_PROVISIONING_ENDPOINT"); + + if (!string.IsNullOrWhiteSpace(customEndpoint)) + return customEndpoint.TrimEnd('/'); + + return ProductionProvisioningBaseUrl; + } } \ No newline at end of file diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 6661d462..3446f3c1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -384,6 +384,15 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); // For AgentApplication.Create permission services.AddSingleton(); // For publish command template extraction + // Provisions the CLI service principal via the Agent 365 service; the CLI is a public + // client and cannot + // provision it itself. + services.AddSingleton(sp => + new ServicePrincipalProvisioningService( + sp.GetRequiredService>(), + sp.GetRequiredService(), + sp.GetRequiredService())); + // Register ProcessService for cross-platform process launching services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BootstrapConfigResolver.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BootstrapConfigResolver.cs index 9899fbf0..1f5dcf11 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BootstrapConfigResolver.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BootstrapConfigResolver.cs @@ -71,17 +71,20 @@ internal sealed class BootstrapConfigResolver : IBootstrapConfigResolver private readonly IConfigService _configService; private readonly CommandExecutor _executor; private readonly GraphApiService? _graphApiService; + private readonly IServicePrincipalProvisioningService? _spProvisioningService; private readonly ILogger _logger; public BootstrapConfigResolver( IConfigService configService, CommandExecutor executor, GraphApiService? graphApiService, - ILoggerFactory loggerFactory) + ILoggerFactory loggerFactory, + IServicePrincipalProvisioningService? spProvisioningService = null) { _configService = configService; _executor = executor; _graphApiService = graphApiService; + _spProvisioningService = spProvisioningService; _logger = loggerFactory.CreateLogger(); } @@ -92,6 +95,49 @@ public BootstrapConfigResolver( FileInfo configFile, bool isCleanupMode = false, CancellationToken ct = default) + { + var config = await ResolveCoreAsync(agentName, tenantIdFlag, configFile, isCleanupMode, ct); + + if (config != null) + { + await EnsureCliServicePrincipalAsync(config.TenantId, ct); + } + + return config; + } + + /// + /// The CLI is a public client and cannot provision its own service principal, so the Agent 365 + /// service is + /// asked to do it once per tenant per process. + /// + private async Task EnsureCliServicePrincipalAsync(string? tenantId, CancellationToken ct) + { + if (_spProvisioningService == null) + { + return; + } + + try + { + await _spProvisioningService.EnsureProvisionedAsync(tenantId, ct: ct); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Service principal provisioning check did not complete."); + } + } + + private async Task ResolveCoreAsync( + string? agentName, + string? tenantIdFlag, + FileInfo configFile, + bool isCleanupMode, + CancellationToken ct) { if (!string.IsNullOrWhiteSpace(agentName)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ServicePrincipalProvisioningService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ServicePrincipalProvisioningService.cs new file mode 100644 index 00000000..f0979d6d --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ServicePrincipalProvisioningService.cs @@ -0,0 +1,286 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Concurrent; +using System.Net; +using System.Text.Json; +using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Outcome of an Agent 365 CLI service principal provisioning attempt. +/// +public enum ServicePrincipalProvisioningStatus +{ + /// Provisioning was not attempted. + Skipped, + + /// The service principal already existed in the tenant. + AlreadyProvisioned, + + /// The service principal was provisioned by this call. + Provisioned, + + /// Provisioning was attempted and did not succeed. + Failed, +} + +/// +/// Result of an Agent 365 CLI service principal provisioning attempt. +/// +/// Outcome of the attempt. +/// Object ID of the service principal, when known. +/// Human-readable detail for diagnostics. +public sealed record ServicePrincipalProvisioningResult( + ServicePrincipalProvisioningStatus Status, + string? ServicePrincipalObjectId, + string? Detail); + +/// +/// Ensures the Agent 365 CLI service principal exists in the target tenant. +/// +public interface IServicePrincipalProvisioningService +{ + /// + /// Ensures the Agent 365 CLI service principal exists in . + /// Runs at most once per tenant for the lifetime of the process and never throws. + /// + /// Target tenant ID. + /// Optional login hint for token acquisition. + /// Cancellation token. + /// The provisioning result. + Task EnsureProvisionedAsync( + string? tenantId, + string? userId = null, + CancellationToken ct = default); +} + +/// +/// Requests Agent 365 CLI service principal provisioning from the Agent 365 service. +/// +/// +/// The CLI is a public client, so its service principal is not created automatically on first +/// sign-in. The Agent 365 service performs the provisioning on the caller's behalf. +/// +public sealed class ServicePrincipalProvisioningService : IServicePrincipalProvisioningService +{ + /// + /// Set to "true" or "1" to skip the provisioning call entirely. + /// + public const string DisableEnvironmentVariable = "A365_DISABLE_SP_PROVISIONING"; + + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNameCaseInsensitive = true, + }; + + private readonly ILogger _logger; + private readonly IAuthenticationService _authService; + private readonly HttpMessageHandler? _handler; + private readonly RetryHelper _retryHelper; + private readonly IConfigService? _configService; + + private readonly ConcurrentDictionary> _inFlight = + new(StringComparer.OrdinalIgnoreCase); + + /// + /// Initializes a new instance of the class. + /// + /// Logger instance. + /// Authentication service used to acquire the delegated token. + /// Optional config service used to resolve the environment. + /// Optional handler override, used by tests. + /// Optional retry helper override, used by tests. + public ServicePrincipalProvisioningService( + ILogger logger, + IAuthenticationService authService, + IConfigService? configService = null, + HttpMessageHandler? handler = null, + RetryHelper? retryHelper = null) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _authService = authService ?? throw new ArgumentNullException(nameof(authService)); + _configService = configService; + _handler = handler; + _retryHelper = retryHelper ?? new RetryHelper(logger); + } + + /// + public Task EnsureProvisionedAsync( + string? tenantId, + string? userId = null, + CancellationToken ct = default) + { + if (IsDisabled()) + { + _logger.LogDebug( + "Agent 365 CLI service principal provisioning disabled by {Variable}.", + DisableEnvironmentVariable); + + return Task.FromResult(new ServicePrincipalProvisioningResult( + ServicePrincipalProvisioningStatus.Skipped, null, "Disabled")); + } + + // The tenant ID reaches a request URL, so only accept a well-formed GUID. + if (!Guid.TryParse(tenantId, out var parsedTenantId) || parsedTenantId == Guid.Empty) + { + _logger.LogDebug("Skipping service principal provisioning: no valid tenant ID available."); + + return Task.FromResult(new ServicePrincipalProvisioningResult( + ServicePrincipalProvisioningStatus.Skipped, null, "NoTenantId")); + } + + var key = parsedTenantId.ToString(); + + // One attempt per tenant per process; concurrent callers share the same attempt. + return _inFlight.GetOrAdd(key, _ => ProvisionAsync(parsedTenantId, userId, ct)); + } + + private static bool IsDisabled() + { + var value = Environment.GetEnvironmentVariable(DisableEnvironmentVariable); + + return string.Equals(value, "true", StringComparison.OrdinalIgnoreCase) + || string.Equals(value, "1", StringComparison.Ordinal); + } + + private async Task ProvisionAsync( + Guid tenantId, + string? userId, + CancellationToken ct) + { + try + { + var environment = await ResolveEnvironmentAsync(); + var baseUrl = ConfigConstants.GetProvisioningBaseUrl(environment); + var requestUrl = baseUrl + string.Format( + System.Globalization.CultureInfo.InvariantCulture, + ConfigConstants.Agent365CliProvisionPathFormat, + tenantId); + + // The Power Platform API gateway fronts the service, so the token is issued for that + // resource. + var authToken = await _authService.GetAccessTokenAsync( + PowerPlatformConstants.PowerPlatformApiIdentifierUri, + tenantId.ToString(), + userId: userId, + ct: ct); + + var correlationId = HttpClientFactory.GenerateCorrelationId(); + + using var httpClient = HttpClientFactory.CreateAuthenticatedClient( + authToken, correlationId: correlationId, handler: _handler); + + _logger.LogDebug( + "Requesting Agent 365 CLI service principal provisioning for tenant {TenantId}.", + tenantId); + + using var response = await _retryHelper.ExecuteWithRetryAsync( + sendCt => httpClient.PostAsync(requestUrl, content: null, sendCt), + cancellationToken: ct); + + return await InterpretResponseAsync(response, tenantId, ct); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + // Provisioning is best-effort; setup continues and surfaces its own errors. + _logger.LogDebug( + ex, + "Agent 365 CLI service principal provisioning failed for tenant {TenantId}.", + tenantId); + + return new ServicePrincipalProvisioningResult( + ServicePrincipalProvisioningStatus.Failed, null, ex.Message); + } + } + + private async Task ResolveEnvironmentAsync() + { + if (_configService == null) + { + return null; + } + + try + { + var config = await _configService.LoadAsync(); + return config?.Environment; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Could not resolve environment for the provisioning endpoint; using default."); + return null; + } + } + + private async Task InterpretResponseAsync( + HttpResponseMessage response, + Guid tenantId, + CancellationToken ct) + { + var body = await response.Content.ReadAsStringAsync(ct); + + if (!response.IsSuccessStatusCode) + { + _logger.LogDebug( + "Agent 365 CLI service principal provisioning returned {StatusCode} for tenant {TenantId}. {Body}", + (int)response.StatusCode, + tenantId, + body); + + var detail = response.StatusCode == HttpStatusCode.Forbidden + ? "Forbidden" + : $"Http{(int)response.StatusCode}"; + + return new ServicePrincipalProvisioningResult( + ServicePrincipalProvisioningStatus.Failed, null, detail); + } + + ProvisionResponseBody? payload = null; + + try + { + payload = JsonSerializer.Deserialize(body, JsonOptions); + } + catch (JsonException ex) + { + _logger.LogDebug(ex, "Could not parse service principal provisioning response."); + } + + var status = payload?.Status switch + { + "Provisioned" => ServicePrincipalProvisioningStatus.Provisioned, + "AlreadyProvisioned" => ServicePrincipalProvisioningStatus.AlreadyProvisioned, + "Disabled" => ServicePrincipalProvisioningStatus.Skipped, + "Failed" => ServicePrincipalProvisioningStatus.Failed, + _ => ServicePrincipalProvisioningStatus.Provisioned, + }; + + _logger.LogDebug( + "Agent 365 CLI service principal provisioning for tenant {TenantId} returned {Status}.", + tenantId, + status); + + return new ServicePrincipalProvisioningResult( + status, payload?.ServicePrincipalObjectId, payload?.Detail); + } + + private sealed class ProvisionResponseBody + { + public string? Status { get; set; } + + public string? ApplicationId { get; set; } + + public string? ServicePrincipalObjectId { get; set; } + + public string? Detail { get; set; } + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ServicePrincipalProvisioningServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ServicePrincipalProvisioningServiceTests.cs new file mode 100644 index 00000000..32085da0 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ServicePrincipalProvisioningServiceTests.cs @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Net; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +/// +/// Tests must run sequentially because the service reads process environment variables. +/// +[CollectionDefinition("ServicePrincipalProvisioningTests", DisableParallelization = true)] +public class ServicePrincipalProvisioningTestCollection { } + +/// +/// Unit tests for . +/// Uses TestHttpMessageHandler and CapturingHttpMessageHandler (defined in +/// GraphApiServiceTests.cs, same assembly) to inject fake HTTP responses. +/// +[Collection("ServicePrincipalProvisioningTests")] +public class ServicePrincipalProvisioningServiceTests +{ + private const string TenantId = "01eed126-1111-2222-3333-444455556666"; + + private static IAuthenticationService FakeAuth() + { + var mock = Substitute.For(); + mock.GetAccessTokenAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any(), + Arg.Any()) + .Returns(Task.FromResult("fake-pp-token")); + return mock; + } + + private static ServicePrincipalProvisioningService CreateService( + HttpMessageHandler handler, + IAuthenticationService? auth = null) => + new( + NullLogger.Instance, + auth ?? FakeAuth(), + configService: null, + handler: handler, + retryHelper: new RetryHelper(NullLogger.Instance, maxRetries: 1, baseDelaySeconds: 0)); + + private static HttpResponseMessage JsonResponse(HttpStatusCode code, string json) => + new(code) { Content = new StringContent(json, System.Text.Encoding.UTF8, "application/json") }; + + [Fact] + public async Task EnsureProvisionedAsync_WhenServiceReportsProvisioned_ReturnsProvisioned() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(JsonResponse( + HttpStatusCode.OK, + """{"status":"Provisioned","servicePrincipalObjectId":"sp-obj-1"}""")); + + var result = await CreateService(handler).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Provisioned, + because: "the CLI must surface the provisioning outcome reported by the service"); + result.ServicePrincipalObjectId.Should().Be( + "sp-obj-1", + because: "the returned object ID identifies the service principal that was created"); + } + + [Fact] + public async Task EnsureProvisionedAsync_WhenServiceReportsAlreadyProvisioned_ReturnsAlreadyProvisioned() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(JsonResponse( + HttpStatusCode.OK, + """{"status":"AlreadyProvisioned","servicePrincipalObjectId":"sp-obj-1"}""")); + + var result = await CreateService(handler).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.AlreadyProvisioned, + because: "an existing service principal is not an error and must be distinguishable from a fresh provision"); + } + + [Fact] + public async Task EnsureProvisionedAsync_PostsToTenantScopedProvisioningRoute() + { + HttpRequestMessage? captured = null; + using var handler = new CapturingHttpMessageHandler(r => captured = r); + handler.QueueResponse(JsonResponse(HttpStatusCode.OK, """{"status":"Provisioned"}""")); + + await CreateService(handler).EnsureProvisionedAsync(TenantId); + + captured.Should().NotBeNull(); + captured!.Method.Should().Be( + HttpMethod.Post, + because: "the provisioning route is defined as an HTTP POST"); + captured.RequestUri!.AbsoluteUri.Should().Contain( + $"/maven/tenants/{TenantId}/agent365/servicePrincipals/agent365Cli/provision", + because: "the gateway routes on the service namespace and the tenant path segment"); + captured.RequestUri.Query.Should().Contain( + "api-version=1", + because: "the Power Platform API gateway rejects requests without an api-version"); + captured.Headers.Authorization!.Scheme.Should().Be( + "Bearer", + because: "the service authenticates the caller with a delegated bearer token"); + } + + [Fact] + public async Task EnsureProvisionedAsync_CalledTwiceForSameTenant_IssuesOneRequest() + { + var requestCount = 0; + using var handler = new CapturingHttpMessageHandler(_ => requestCount++); + handler.QueueResponse(JsonResponse(HttpStatusCode.OK, """{"status":"Provisioned"}""")); + handler.QueueResponse(JsonResponse(HttpStatusCode.OK, """{"status":"Provisioned"}""")); + + var svc = CreateService(handler); + await svc.EnsureProvisionedAsync(TenantId); + await svc.EnsureProvisionedAsync(TenantId); + + requestCount.Should().Be( + 1, + because: "provisioning must run at most once per tenant per process so setup is not slowed by repeat calls"); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not-a-guid")] + [InlineData("00000000-0000-0000-0000-000000000000")] + public async Task EnsureProvisionedAsync_WithInvalidTenantId_SkipsWithoutHttpCall(string? tenantId) + { + var requestCount = 0; + using var handler = new CapturingHttpMessageHandler(_ => requestCount++); + + var result = await CreateService(handler).EnsureProvisionedAsync(tenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Skipped, + because: "a tenant ID that is not a usable GUID must never be interpolated into a request URL"); + requestCount.Should().Be(0, because: "no request may be sent without a valid tenant ID"); + } + + [Fact] + public async Task EnsureProvisionedAsync_WhenServiceReturnsForbidden_FailsWithoutThrowing() + { + using var handler = new TestHttpMessageHandler(); + handler.QueueResponse(JsonResponse(HttpStatusCode.Forbidden, """{"error":"denied"}""")); + + var result = await CreateService(handler).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Failed, + because: "provisioning is best-effort and a rejection must not abort the caller's command"); + } + + [Fact] + public async Task EnsureProvisionedAsync_WhenTransportThrows_FailsWithoutThrowing() + { + using var handler = new ThrowingHttpMessageHandler(); + + var result = await CreateService(handler).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Failed, + because: "a network failure must not surface as an exception to setup commands"); + } + + [Fact] + public async Task EnsureProvisionedAsync_WhenTokenAcquisitionThrows_FailsWithoutThrowing() + { + var auth = Substitute.For(); + auth.GetAccessTokenAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any(), Arg.Any(), + Arg.Any()) + .Returns>(_ => throw new InvalidOperationException("no token")); + + using var handler = new TestHttpMessageHandler(); + + var result = await CreateService(handler, auth).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Failed, + because: "a user who cannot obtain a Power Platform token must still be able to run setup"); + } + + [Fact] + public async Task EnsureProvisionedAsync_WhenDisabledByEnvironmentVariable_SkipsWithoutHttpCall() + { + var requestCount = 0; + using var handler = new CapturingHttpMessageHandler(_ => requestCount++); + + Environment.SetEnvironmentVariable( + ServicePrincipalProvisioningService.DisableEnvironmentVariable, "true"); + try + { + var result = await CreateService(handler).EnsureProvisionedAsync(TenantId); + + result.Status.Should().Be( + ServicePrincipalProvisioningStatus.Skipped, + because: "operators must have a documented way to turn the extra call off"); + requestCount.Should().Be(0, because: "the disable switch must prevent the request entirely"); + } + finally + { + Environment.SetEnvironmentVariable( + ServicePrincipalProvisioningService.DisableEnvironmentVariable, null); + } + } +}