diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index 19712bb0..652f1d31 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -392,6 +392,12 @@ private static Command CreatePublishSubcommand( description: "Publisher name for the MCP Server. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'."); command.AddOption(publisherNameOption); + var serviceTreeIdOption = new Option("--service-tree-id", description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)"); + command.AddOption(serviceTreeIdOption); + + var secretLifetimeMonthsOption = new Option(["--secret-lifetime-months", "-l"], description: "Lifetime in months (1-24) for the generated client secret on the A365 proxy Entra app. Default is 2 years. Set a value smaller than the appManagementPolicies cap in your tenant."); + command.AddOption(secretLifetimeMonthsOption); + var yesOption = new Option( ["--yes", "-y"], description: "Skip the interactive 'Proceed with publish? (y/N)' confirmation."); @@ -412,7 +418,9 @@ private static Command CreatePublishSubcommand( DisplayName: context.ParseResult.GetValueForOption(displayNameOption), PublisherName: context.ParseResult.GetValueForOption(publisherNameOption), Yes: context.ParseResult.GetValueForOption(yesOption), - DryRun: context.ParseResult.GetValueForOption(dryRunOption)); + DryRun: context.ParseResult.GetValueForOption(dryRunOption), + ServiceTreeId: context.ParseResult.GetValueForOption(serviceTreeIdOption), + SecretLifetimeMonths: context.ParseResult.GetValueForOption(secretLifetimeMonthsOption)); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); var success = await executor.ExecuteAsync(args, context.GetCancellationToken()); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index c60e3ed0..8b0efa93 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -19,7 +19,9 @@ internal record RawPublishArgs( string? DisplayName, string? PublisherName, bool Yes, - bool DryRun); + bool DryRun, + string? ServiceTreeId = null, + int? SecretLifetimeMonths = null); /// /// Orchestrates first-party MCP server publish in one CLI command. The shape mirrors @@ -68,12 +70,24 @@ private sealed record ResolvedInput // When true, skip the interactive "Proceed with publish? (y/N)" confirmation. Set via // --yes / -y. Required for non-interactive contexts (CI scripts, automation). public required bool Yes { get; init; } + + // ServiceTree ID stamped onto the Entra apps created here. Required in Microsoft corporate + // tenants; null elsewhere. Applied to both the A365 proxy and Public Clients apps. + public string? ServiceTreeId { get; init; } + + // Optional client-secret lifetime (months) for the A365 proxy app's secret. Null uses Graph's + // default; a smaller value avoids the appManagementPolicies cap failing publish in strict tenants. + public int? SecretLifetimeMonths { get; init; } } internal sealed record EntraAppSet( string? PublicClientsClientId, string? PublicClientsObjectId, - string PublicClientsAppName); + string PublicClientsAppName, + string A365AppClientId, + string A365AppSecret, + string A365AppObjectId, + string A365AppName); internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = default) { @@ -84,8 +98,9 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct if (input.DryRun) { - _logger.LogInformation("[DRY RUN] Would create Entra app '{PublicClients}' in tenant", $"{input.ServerName}-PublicClients"); - _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill PPMI scope on the created app"); + _logger.LogInformation("[DRY RUN] Would create Entra apps '{PublicClients}' and '{A365Proxy}' in tenant", $"{input.ServerName}-PublicClients", $"{input.ServerName}-A365Proxy"); + _logger.LogInformation("[DRY RUN] Would call the publish endpoint and forward the A365 proxy app credentials so the platform can create the Power Platform connector for custom (non-Dataverse) servers"); + _logger.LogInformation("[DRY RUN] Would back-fill the PPMI scope on the created apps, add the McpServer API permission and connector redirect URI to the A365 proxy app, and delete the proxy app when the publish response shows no connector was created"); return true; } @@ -131,6 +146,8 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct DisplayName = input.DisplayName, PublicClientsAppId = apps.PublicClientsClientId, PublisherName = input.PublisherName, + A365ProxyClientId = apps.A365AppClientId, + A365ProxyClientSecret = apps.A365AppSecret, }; PublishMcpServerResponse? publishResponse; @@ -183,6 +200,13 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct try { + // Reject an out-of-range secret lifetime before prompting or creating apps; register enforces the same 1-24 bound. + if (args.SecretLifetimeMonths is { } lifetime && (lifetime < 1 || lifetime > 24)) + { + _logger.LogError("--secret-lifetime-months must be between 1 and 24 (Graph's maximum is ~2 years). Got: {Value}", lifetime); + return null; + } + var environmentId = args.EnvironmentId; if (string.IsNullOrWhiteSpace(environmentId)) { @@ -276,6 +300,8 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct PublisherName = string.IsNullOrWhiteSpace(publisherName) ? null : publisherName, Yes = args.Yes, DryRun = args.DryRun, + ServiceTreeId = string.IsNullOrWhiteSpace(args.ServiceTreeId) ? null : args.ServiceTreeId, + SecretLifetimeMonths = args.SecretLifetimeMonths, }; } catch (ArgumentException ex) @@ -298,6 +324,10 @@ private void DisplayPublishSummary(ResolvedInput input) DevelopMcpCommand.WriteLabel(" Alias: "); Console.WriteLine(input.Alias); DevelopMcpCommand.WriteLabel(" Display Name: "); Console.WriteLine(input.DisplayName); DevelopMcpCommand.WriteLabel(" Publisher: "); Console.WriteLine(input.PublisherName ?? "(none — platform will reject if this is a custom server)"); + if (input.SecretLifetimeMonths is { } lifetime) + { + DevelopMcpCommand.WriteLabel(" Secret Lifetime: "); Console.WriteLine($"{lifetime} month(s)"); + } Console.WriteLine(); } @@ -318,13 +348,40 @@ private void DisplayPublishSummary(ResolvedInput input) { var provisioner = new EntraAppProvisioner(_logger, _graphApiService!, _retryHelper); - var publicClients = await provisioner.CreatePublicClientsAppAsync( - input.ServerName, tenantId, serviceTreeId: null, warnings, ct); - - return new EntraAppSet( - PublicClientsClientId: publicClients.ClientId, - PublicClientsObjectId: publicClients.ObjectId, - PublicClientsAppName: publicClients.AppName); + // Confidential A365 proxy app + secret. Required for custom (non-Dataverse) servers: the + // platform creates the Power Platform connector only when its credentials are supplied. + var a365ProxyApp = await provisioner.CreateProxyAppAsync( + input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", + serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths, ct: ct); + if (a365ProxyApp is null) return null; + + // If Public Clients creation throws after the proxy app exists, the proxy app (with its + // secret) is orphaned — RollbackEntraAppsAsync only runs once we have a full EntraAppSet and + // the platform call fails. Clean it up here so a Graph error / throttling / cancellation + // doesn't leak a credential. Use CancellationToken.None for the compensating delete so a + // caller Ctrl+C still removes the orphan. + try + { + var publicClients = await provisioner.CreatePublicClientsAppAsync( + input.ServerName, tenantId, serviceTreeId: input.ServiceTreeId, warnings, ct); + + return new EntraAppSet( + PublicClientsClientId: publicClients.ClientId, + PublicClientsObjectId: publicClients.ObjectId, + PublicClientsAppName: publicClients.AppName, + A365AppClientId: a365ProxyApp.ClientId, + A365AppSecret: a365ProxyApp.Secret, + A365AppObjectId: a365ProxyApp.ObjectId, + A365AppName: a365ProxyApp.AppName); + } + catch (Exception ex) + { + _logger.LogError("Failed to create the Public Clients Entra app after the A365 proxy app was created; deleting the orphaned proxy app '{A365Proxy}'.", a365ProxyApp.AppName); + _logger.LogDebug("Exception details: {Exception}", ex.ToString()); + await TryDeleteEntraAppAsync(tenantId, a365ProxyApp.ObjectId, a365ProxyApp.ClientId, a365ProxyApp.AppName, CancellationToken.None); + if (ex is OperationCanceledException && ct.IsCancellationRequested) throw; + return null; + } } // Best-effort compensating delete for the Entra apps created in CreateEntraAppsAsync, run when @@ -335,41 +392,47 @@ internal async Task RollbackEntraAppsAsync(EntraAppSet apps, string tenantId, Ca { if (_graphApiService is null) { - _logger.LogWarning("Graph API service is unavailable; cannot roll back Entra app '{PublicClients}'. Delete it manually in the Azure portal.", apps.PublicClientsAppName); + _logger.LogWarning("Graph API service is unavailable; cannot roll back Entra apps '{PublicClients}' and '{A365Proxy}'. Delete them manually in the Azure portal.", apps.PublicClientsAppName, apps.A365AppName); return; } _logger.LogInformation("Rolling back Entra app registrations created for failed publish..."); - if (!string.IsNullOrWhiteSpace(apps.PublicClientsObjectId)) + await TryDeleteEntraAppAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct); + await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct); + } + + // Best-effort compensating delete for a single Entra app. No-op when the object id is unknown or + // Graph is unavailable. Failures are logged with both clientId and objectId so the user can clean + // up manually; the delete never throws. + private async Task TryDeleteEntraAppAsync(string tenantId, string? objectId, string? clientId, string appName, CancellationToken ct) + { + if (_graphApiService is null || string.IsNullOrWhiteSpace(objectId)) { - await DeleteOneAsync(apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct); + return; } - async Task DeleteOneAsync(string objectId, string? clientId, string appName, CancellationToken cancellationToken) + try { - try + var deleted = await _graphApiService.DeleteEntraAppAsync(tenantId, objectId, ct); + if (deleted) { - var deleted = await _graphApiService!.DeleteEntraAppAsync(tenantId, objectId, cancellationToken); - if (deleted) - { - _logger.LogInformation("Rolled back Entra app '{AppName}' (objectId {ObjectId})", appName, objectId); - } - else - { - _logger.LogError( - "Failed to roll back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", - appName, clientId ?? "", objectId); - } + _logger.LogInformation("Rolled back Entra app '{AppName}' (objectId {ObjectId})", appName, objectId); } - catch (Exception ex) + else { _logger.LogError( - ex, - "Exception rolling back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", + "Failed to roll back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", appName, clientId ?? "", objectId); } } + catch (Exception ex) + { + _logger.LogError( + ex, + "Exception rolling back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", + appName, clientId ?? "", objectId); + } } private async Task ConfigureEntraAppsAsync( @@ -383,6 +446,19 @@ private async Task ConfigureEntraAppsAsync( var tasks = new List(); var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); + // The proxy app + secret are forwarded on every publish because the CLI can't classify the + // server (custom vs 1p/Dataverse) before the platform does. Only custom servers actually get + // a Power Platform connector; the platform signals that by returning a connector id and/or a + // redirect URI. When neither is present the proxy app is unused, so delete it here rather than + // leave an unused credential in the tenant (the Public Clients app and its PPMI grant remain). + var connectorCreated = !string.IsNullOrWhiteSpace(response.A365ProxyConnectorId) + || !string.IsNullOrWhiteSpace(response.A365ProxyRedirectUri); + if (!connectorCreated) + { + _logger.LogInformation("Publish returned no A365 proxy connector for '{ServerName}' (first-party / Dataverse server); removing the unused A365 proxy app '{A365Proxy}'.", input.ServerName, apps.A365AppName); + await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct); + } + // Grant required-resource-access on the just-created Public Clients Entra app. // The platform resolves the right resource per server type (Custom: managedidentityid; app-based // / Dataverse MCP: 1p mappings; fallback: platform's own app id) and returns both the resource @@ -418,6 +494,16 @@ private async Task ConfigureEntraAppsAsync( if (resourceScopeId.HasValue) { + // The platform wires the A365 proxy connector with the proxy app as its OAuth client and + // McpServerAppId as the resource, so the proxy app must hold this required-resource-access + // grant or Entra rejects the token request (AADSTS650057). Grant it on the proxy app only + // when a connector was created (otherwise the proxy app was just deleted above); always + // grant it on the Public Clients app, mirroring register. + if (connectorCreated) + { + tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); + } + if (apps.PublicClientsObjectId != null) { tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); @@ -430,12 +516,65 @@ private async Task ConfigureEntraAppsAsync( concurrentWarnings.Add(msg); } + // Custom (non-Dataverse) servers get a Power Platform connector whose redirect URI the + // platform returns here. Write it onto the A365 proxy app so the connector's OAuth flow works. + // Only relevant when a connector was created; first-party servers get no connector (and the + // proxy app was already removed), so no redirect URI is expected and none is warned about. + if (connectorCreated) + { + var a365RedirectUri = response.A365ProxyRedirectUri; + if (!string.IsNullOrWhiteSpace(a365RedirectUri)) + { + tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct)); + } + else + { + var msg = "A365 Proxy connector was created but publish returned no redirect URI. Redirect URI configuration skipped."; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); + } + } + await Task.WhenAll(tasks); foreach (var w in concurrentWarnings) warnings.Add(w); } + private async Task UpdateA365RedirectUrisAsync( + string tenantId, EntraAppSet apps, string a365RedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + try + { + var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); + var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); + var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, retryCt), + result => !result, + cancellationToken: ct); + if (!success) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + } + } + catch (Exception ex) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + private async Task AddRequiredResourceAccessAsync( string tenantId, string appObjectId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs index ceb92080..b767f6ce 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs @@ -37,4 +37,20 @@ public class PublishMcpServerRequest /// [JsonPropertyName("publisherName")] public string? PublisherName { get; set; } + + /// + /// A365 proxy (confidential) Entra app client id created CLI-side. The platform's v2 publish + /// path creates the Power Platform connector for custom (non-Dataverse) servers only when both + /// this and are supplied; otherwise connector creation is + /// skipped (A365ProxyConnectorCreation=SkippedNoCredentials). + /// + [JsonPropertyName("a365ProxyClientId")] + public string? A365ProxyClientId { get; set; } + + /// + /// Client secret for the A365 proxy Entra app. Paired with so the + /// platform can create the Power Platform connector for custom servers. + /// + [JsonPropertyName("a365ProxyClientSecret")] + public string? A365ProxyClientSecret { get; set; } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs index 2ea1e2b9..e9e05166 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs @@ -47,6 +47,22 @@ public class PublishMcpServerResponse [JsonPropertyName("PublicClientsAppId")] public string? PublicClientsAppId { get; set; } + /// + /// Redirect URI the platform assigns to the A365 proxy connector for custom servers. When + /// present, the CLI writes the tc/non-tc redirect URI list onto the A365 proxy Entra app it + /// created. Emitted PascalCase by the platform, same as . + /// + [JsonPropertyName("A365ProxyRedirectUri")] + public string? A365ProxyRedirectUri { get; set; } + + /// + /// Id of the Power Platform connector the platform created for the custom server, when proxy + /// credentials were supplied. Surfaced for logging/parity; empty when connector creation was + /// skipped. + /// + [JsonPropertyName("A365ProxyConnectorId")] + public string? A365ProxyConnectorId { get; set; } + /// /// Whether the operation was successful. /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index 9a47768f..b6b7e6cd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -239,7 +239,7 @@ private static void RedactSecretFields(System.Text.Json.Nodes.JsonObject obj) { var secretKeys = new HashSet(StringComparer.OrdinalIgnoreCase) { - "clientApp1Secret", "clientApp2Secret", "clientSecret" + "clientApp1Secret", "clientApp2Secret", "clientSecret", "a365ProxyClientSecret" }; foreach (var key in obj.Select(p => p.Key).ToList()) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs index 5d9a079d..5915ce78 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs @@ -155,6 +155,12 @@ public async Task PublishCommand_ForwardsParsedParametersToToolingService() TestTenantId, TestPublicClientsObjectId, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); + // Publish now also creates the confidential A365 proxy app + secret (required so the platform + // creates the Power Platform connector for custom servers). Stub the secret so proxy creation succeeds. + graphApiService.AddAppPasswordAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("a365-proxy-secret")); + // Mock Graph for ConfigureEntraAppsAsync → required-resource-access grant on Public Clients. graphApiService.GetOAuth2PermissionScopeIdAsync( TestTenantId, Arg.Any(), Arg.Any(), Arg.Any()) @@ -224,6 +230,14 @@ public async Task PublishCommand_ForwardsParsedParametersToToolingService() because: "the just-created Public Clients Entra app's clientId must be carried to the " + "platform so it can be echoed back and the CLI can grant the PPMI scope on it " + "post-publish."); + capturedRequest.A365ProxyClientId.Should().NotBeNullOrEmpty( + because: "the confidential A365 proxy app's clientId must be forwarded so the platform " + + "creates the Power Platform connector for custom servers instead of logging " + + "SkippedNoCredentials."); + capturedRequest.A365ProxyClientSecret.Should().Be( + "a365-proxy-secret", + because: "the proxy app's secret must be forwarded alongside its clientId; the platform " + + "requires both to create the connector."); } /// @@ -256,6 +270,9 @@ public async Task PublishCommand_ExplicitEmptyPublisherName_SkipsPromptAndForwar graphApiService.UpdateAppPublicClientRedirectUrisAsync( TestTenantId, TestPublicClientsObjectId, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); + graphApiService.AddAppPasswordAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult("a365-proxy-secret")); graphApiService.GetOAuth2PermissionScopeIdAsync( TestTenantId, Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(Guid.NewGuid())); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index 0f4efc91..be4e349a 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -124,8 +124,10 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var options = subcommand.Options.ToList(); // Verify all expected options exist. Tenant ID is auto-detected from the current az login - // session, so publish does not expose --tenant-id; ServiceTree tagging is not required for - // publish since it targets Dataverse environments rather than Microsoft corp tenants. + // session, so publish does not expose --tenant-id. Publish now registers the A365 proxy and + // Public Clients Entra apps in the operator's own tenant (via az login) — which may be a + // ServiceTree-enrolled Microsoft corp tenant — so it exposes --service-tree-id and + // --secret-lifetime-months, mirroring register. var optionNames = options.Select(o => o.Name).ToList(); optionNames.Should().Contain("environment-id"); optionNames.Should().Contain("server-name"); @@ -135,10 +137,16 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() "tenant-id", because: "tenant id is auto-detected from the current 'az login' session; exposing " + "--tenant-id would imply per-publish tenant targeting that the executor does not support."); - optionNames.Should().NotContain( + optionNames.Should().Contain( "service-tree-id", - because: "publish targets a customer's Dataverse env, not a Microsoft corp tenant — " + - "the ServiceTree tagging that --service-tree-id provides is not applicable here."); + because: "publish creates Entra app registrations in the operator's own tenant, which may " + + "be ServiceTree-enrolled; those registrations are rejected without a " + + "serviceManagementReference, so --service-tree-id must be available (reviewer request on #499, same as #496)."); + optionNames.Should().Contain( + "secret-lifetime-months", + because: "the A365 proxy app's client secret must fit under the tenant's appManagementPolicies " + + "lifetime cap or publish fails in strict tenants; --secret-lifetime-months lets the " + + "operator set a compliant lifetime, mirroring register."); optionNames.Should().Contain("dry-run"); // Verify critical aliases for Azure CLI compliance @@ -153,6 +161,11 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var displayNameOption = options.FirstOrDefault(o => o.Name == "display-name"); displayNameOption!.Aliases.Should().Contain("-d"); + + var secretLifetimeOption = options.FirstOrDefault(o => o.Name == "secret-lifetime-months"); + secretLifetimeOption!.Aliases.Should().Contain( + "-l", + because: "register exposes --secret-lifetime-months as -l; publish must use the same alias for consistency."); } [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index 1989ea12..7795fe4c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -13,17 +13,18 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// /// Tests for dry-run output. The dry-run log must mirror the /// real Entra app naming scheme (derived from ServerName) so users can predict what will be -/// created — the {ServerName}-PublicClients app. +/// created — the {ServerName}-PublicClients and {ServerName}-A365Proxy apps. /// public class PublishCommandExecutorDryRunTests { /// - /// The dry-run log must (a) name only the Public Clients app — derived from ServerName, - /// not Alias — (b) describe a PPMI-scope-only back-fill (no redirect-URI back-fill), and - /// (c) skip the platform publish call entirely. + /// The dry-run log must (a) name the Public Clients app — derived from ServerName, + /// not Alias — and (b) describe the full post-publish configuration: PPMI-scope back-fill, + /// the A365 proxy app's API permission and redirect URI, and its removal when no connector is + /// created. It must also (c) skip the platform publish call entirely. /// [Fact] - public async Task ExecuteAsync_DryRun_NamesPublicClientsApp_AndBackfillsPpmiScopeOnly() + public async Task ExecuteAsync_DryRun_NamesPublicClientsApp_AndDescribesProxyConfiguration() { var logger = Substitute.For(); var toolingService = Substitute.For(); @@ -58,13 +59,14 @@ public async Task ExecuteAsync_DryRun_NamesPublicClientsApp_AndBackfillsPpmiScop Arg.Any(), Arg.Any>()); - // The back-fill line now mentions only PPMI scope, not redirect URI. + // The back-fill line now also describes the proxy app's permission, redirect URI, and cleanup. logger.Received(1).Log( LogLevel.Information, Arg.Any(), Arg.Is(o => - o.ToString()!.Contains("back-fill PPMI scope") && - !o.ToString()!.Contains("redirect URI")), + o.ToString()!.Contains("PPMI scope") && + o.ToString()!.Contains("A365 proxy app") && + o.ToString()!.Contains("redirect URI")), Arg.Any(), Arg.Any>()); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorEntraAppTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorEntraAppTests.cs new file mode 100644 index 00000000..12f90a8e --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorEntraAppTests.cs @@ -0,0 +1,355 @@ +// 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.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; + +/// +/// Covers the Entra-app orchestration performs for custom +/// (non-Dataverse) MCP servers: the confidential A365 proxy app must be created and its credentials +/// forwarded to the platform so the Power Platform connector is created at publish time, and both +/// created apps must be rolled back on failure. These invariants are what let the platform stop +/// logging A365ProxyConnectorCreation=SkippedNoCredentials. +/// +/// Tests substitute the concrete (all Entra calls are virtual) and let +/// the real run against it, mirroring the production wiring. +/// +public class PublishCommandExecutorEntraAppTests +{ + private const string TenantId = "00000000-0000-0000-0000-000000000001"; + private const string EnvironmentId = "00000000-0000-0000-0000-000000000000"; + private const string ServerName = "mcp_TestServer"; + + private static RawPublishArgs MakeArgs() => new( + EnvironmentId: EnvironmentId, + ServerName: ServerName, + Alias: "myAlias", + DisplayName: "Test Display", + PublisherName: "Contoso", + Yes: true, + DryRun: false); + + private static PublishCommandExecutor MakeExecutor( + ILogger logger, IAgent365ToolingService tooling, GraphApiService graph) + { + var retry = new RetryHelper(logger, maxRetries: 1, baseDelaySeconds: 0); + return new TestablePublishCommandExecutor(logger, tooling, graph, retry, TenantId); + } + + /// + /// Stubs a successful two-app creation: the A365 proxy app (with a secret) and the Public + /// Clients app. Returns the proxy client id / secret / object id the tests assert on. + /// + private static (string ProxyClientId, string ProxySecret, string ProxyObjectId) ArrangeSuccessfulAppCreation(GraphApiService graph) + { + const string proxyObjectId = "proxy-object-id"; + const string proxyClientId = "proxy-client-id"; + const string proxySecret = "proxy-secret"; + + graph.CreateEntraAppAsync(Arg.Any(), Arg.Is(n => n.EndsWith("-A365Proxy")), Arg.Any(), Arg.Any()) + .Returns((proxyObjectId, proxyClientId)); + graph.CreateEntraAppAsync(Arg.Any(), Arg.Is(n => n.EndsWith("-PublicClients")), Arg.Any(), Arg.Any()) + .Returns(("pc-object-id", "pc-client-id")); + graph.AddAppPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(proxySecret); + graph.UpdateAppPublicClientRedirectUrisAsync(Arg.Any(), Arg.Any(), Arg.Any>(), Arg.Any()) + .Returns(true); + graph.UpdateAppRedirectUrisAsync(Arg.Any(), Arg.Any(), Arg.Any>(), Arg.Any()) + .Returns(true); + graph.DeleteEntraAppAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + return (proxyClientId, proxySecret, proxyObjectId); + } + + [Fact] + public async Task ExecuteAsync_WhenProxyAppCreationFails_AbortsPublish_WithoutCallingPlatform() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + // Proxy app creation fails; public-clients creation is never reached because the proxy app + // is mandatory for custom servers. + graph.CreateEntraAppAsync(Arg.Any(), Arg.Is(n => n.EndsWith("-A365Proxy")), Arg.Any(), Arg.Any()) + .Returns(((string, string)?)null); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeFalse("proxy app creation failure must fail the publish for custom servers"); + await tooling.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); + } + + [Fact] + public async Task ExecuteAsync_ForwardsProxyCredentials_ToPlatformPublishRequest() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + var (proxyClientId, proxySecret, _) = ArrangeSuccessfulAppCreation(graph); + + PublishMcpServerRequest? capturedRequest = null; + tooling.PublishServerAsync(EnvironmentId, ServerName, Arg.Do(r => capturedRequest = r), Arg.Any()) + .Returns(new PublishMcpServerResponse { Status = "Success" }); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeTrue(); + capturedRequest.Should().NotBeNull(); + capturedRequest!.A365ProxyClientId.Should().Be(proxyClientId, + because: "the platform creates the Power Platform connector only when the proxy app's client id is supplied"); + capturedRequest.A365ProxyClientSecret.Should().Be(proxySecret, + because: "the platform needs the proxy app's secret to create the connector; without it it logs SkippedNoCredentials"); + } + + /// + /// ServiceTree-enrolled tenants reject app registrations without a serviceManagementReference, + /// and strict tenants cap secret lifetimes; both the A365 proxy and Public Clients apps must + /// therefore receive --service-tree-id, and the proxy secret must honor + /// --secret-lifetime-months, exactly as the register flow does. + /// + [Fact] + public async Task ExecuteAsync_ForwardsServiceTreeIdAndSecretLifetime_ToEntraAppCreation() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + ArrangeSuccessfulAppCreation(graph); + + tooling.PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new PublishMcpServerResponse { Status = "Success" }); + + var args = MakeArgs() with { ServiceTreeId = "st-123", SecretLifetimeMonths = 6 }; + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(args, CancellationToken.None); + + result.Should().BeTrue(); + await graph.Received(1).CreateEntraAppAsync( + TenantId, Arg.Is(n => n.EndsWith("-A365Proxy")), "st-123", Arg.Any()); + await graph.Received(1).CreateEntraAppAsync( + TenantId, Arg.Is(n => n.EndsWith("-PublicClients")), "st-123", Arg.Any()); + await graph.Received(1).AddAppPasswordAsync( + TenantId, "proxy-object-id", Arg.Any(), 6, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_WhenProxyRedirectUriReturned_UpdatesProxyAppRedirectUris() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + var (_, _, proxyObjectId) = ArrangeSuccessfulAppCreation(graph); + + tooling.PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new PublishMcpServerResponse + { + Status = "Success", + A365ProxyRedirectUri = "https://global.consent.azure-apim.net/redirect", + }); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeTrue(); + await graph.Received(1).UpdateAppRedirectUrisAsync( + TenantId, proxyObjectId, Arg.Any>(), Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_WhenConnectorCreatedButRedirectUriMissing_WarnsAndSkipsRedirectUpdate() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + ArrangeSuccessfulAppCreation(graph); + + // Connector was created (id present) but no redirect URI came back — a real anomaly worth a + // warning, unlike the first-party case where no connector is expected at all. + tooling.PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new PublishMcpServerResponse { Status = "Success", A365ProxyConnectorId = "connector-id" }); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeTrue(); + await graph.DidNotReceive().UpdateAppRedirectUrisAsync( + Arg.Any(), Arg.Any(), Arg.Any>(), Arg.Any()); + logger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("connector was created but publish returned no redirect URI")), + Arg.Any(), + Arg.Any>()); + } + + /// + /// The proxy app + secret are forwarded on every publish because the CLI can't classify the + /// server before the platform does. When the response shows no connector was created (a + /// first-party / Dataverse server), the proxy credential is unused and must be deleted so it + /// doesn't linger in the tenant. The Public Clients app must be left in place. + /// + [Fact] + public async Task ExecuteAsync_WhenNoConnectorCreated_DeletesUnusedProxyApp_AndSkipsRedirectUpdate() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + var (_, _, proxyObjectId) = ArrangeSuccessfulAppCreation(graph); + + // No connector id and no redirect URI => the platform created no connector for this server. + tooling.PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new PublishMcpServerResponse { Status = "Success" }); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeTrue(); + await graph.Received(1).DeleteEntraAppAsync(TenantId, proxyObjectId, Arg.Any()); + await graph.DidNotReceive().DeleteEntraAppAsync(TenantId, "pc-object-id", Arg.Any()); + await graph.DidNotReceive().UpdateAppRedirectUrisAsync( + Arg.Any(), Arg.Any(), Arg.Any>(), Arg.Any()); + } + + /// + /// If Public Clients creation throws after the confidential proxy app (with its secret) is + /// created, the proxy app is orphaned unless explicitly cleaned up — the failure predates the + /// full EntraAppSet that the platform-failure rollback path deletes. The executor must + /// delete the proxy app itself and fail the publish without calling the platform. + /// + [Fact] + public async Task ExecuteAsync_WhenPublicClientsCreationThrows_DeletesOrphanedProxyApp_AndAbortsPublish() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + const string proxyObjectId = "proxy-object-id"; + graph.CreateEntraAppAsync(Arg.Any(), Arg.Is(n => n.EndsWith("-A365Proxy")), Arg.Any(), Arg.Any()) + .Returns((proxyObjectId, "proxy-client-id")); + graph.AddAppPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns("proxy-secret"); + graph.CreateEntraAppAsync(Arg.Any(), Arg.Is(n => n.EndsWith("-PublicClients")), Arg.Any(), Arg.Any()) + .Returns<(string, string)?>(_ => throw new InvalidOperationException("graph throttled")); + graph.DeleteEntraAppAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(true); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeFalse("a failure creating the Public Clients app must abort the publish"); + await graph.Received(1).DeleteEntraAppAsync(TenantId, proxyObjectId, Arg.Any()); + await tooling.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); + } + + /// + /// The A365 proxy app is the OAuth client of the platform-created connector, with McpServerAppId + /// as its resource. Without a required-resource-access grant for that resource on the proxy app, + /// Entra rejects the connector's token request (AADSTS650057). The grant must therefore land on + /// BOTH the proxy app and the Public Clients app. + /// + [Fact] + public async Task ExecuteAsync_GrantsMcpServerResourceAccess_OnBothProxyAndPublicClientsApps() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + + var (_, _, proxyObjectId) = ArrangeSuccessfulAppCreation(graph); + + const string mcpServerAppId = "1a2a0eb6-0000-0000-0000-000000000000"; + const string mcpServerScope = "Tools.ListInvoke.All"; + var scopeId = Guid.NewGuid(); + + graph.GetOAuth2PermissionScopeIdAsync(TenantId, mcpServerAppId, mcpServerScope, Arg.Any()) + .Returns(scopeId); + graph.AddRequiredResourceAccessAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + tooling.PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new PublishMcpServerResponse + { + Status = "Success", + McpServerAppId = mcpServerAppId, + McpServerScope = mcpServerScope, + A365ProxyConnectorId = "connector-id", + }); + + var executor = MakeExecutor(logger, tooling, graph); + + var result = await executor.ExecuteAsync(MakeArgs(), CancellationToken.None); + + result.Should().BeTrue(); + await graph.Received(1).AddRequiredResourceAccessAsync( + TenantId, proxyObjectId, mcpServerAppId, scopeId, Arg.Any()); + await graph.Received(1).AddRequiredResourceAccessAsync( + TenantId, "pc-object-id", mcpServerAppId, scopeId, Arg.Any()); + await graph.Received(2).AddRequiredResourceAccessAsync( + Arg.Any(), Arg.Any(), mcpServerAppId, scopeId, Arg.Any()); + } + + [Fact] + public async Task RollbackEntraAppsAsync_DeletesBothPublicClientsAndProxyApps() + { + var logger = Substitute.For(); + var tooling = Substitute.For(); + var graph = Substitute.For(); + graph.DeleteEntraAppAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(true); + + var executor = MakeExecutor(logger, tooling, graph); + + var apps = new PublishCommandExecutor.EntraAppSet( + PublicClientsClientId: "pc-client-id", + PublicClientsObjectId: "pc-object-id", + PublicClientsAppName: $"{ServerName}-PublicClients", + A365AppClientId: "proxy-client-id", + A365AppSecret: "proxy-secret", + A365AppObjectId: "proxy-object-id", + A365AppName: $"{ServerName}-A365Proxy"); + + await executor.RollbackEntraAppsAsync(apps, TenantId, CancellationToken.None); + + await graph.Received(1).DeleteEntraAppAsync(TenantId, "pc-object-id", Arg.Any()); + await graph.Received(1).DeleteEntraAppAsync(TenantId, "proxy-object-id", Arg.Any()); + } + + /// + /// Overrides only the tenant-detection seam (which shells out to Azure CLI) so the rest of the + /// executor runs unchanged against the substituted Graph and tooling services. + /// + private sealed class TestablePublishCommandExecutor : PublishCommandExecutor + { + private readonly string _tenantId; + + public TestablePublishCommandExecutor( + ILogger logger, IAgent365ToolingService toolingService, GraphApiService graphApiService, + RetryHelper retryHelper, string tenantId) + : base(logger, toolingService, graphApiService, retryHelper) + { + _tenantId = tenantId; + } + + protected override Task DetectTenantIdAsync() => Task.FromResult(_tenantId); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorTests.cs new file mode 100644 index 00000000..25d8c5a8 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.CommandLine; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Commands; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; + +/// +/// Invocation tests for the publish subcommand's --secret-lifetime-months pre-flight range +/// validation. Exercises the [1, 24] guard in via the full +/// System.CommandLine pipeline so the resulting exit code is asserted as the user would observe it. +/// +public class PublishCommandExecutorTests +{ + [Theory] + [InlineData("0")] + [InlineData("25")] + [InlineData("-1")] + [InlineData("48")] + public async Task Publish_WithOutOfRangeSecretLifetimeMonths_ReturnsExitCode1AndDoesNotCallTooling(string lifetimeArg) + { + // Arrange + var logger = Substitute.For(); + var toolingService = Substitute.For(); + var command = DevelopMcpCommand.CreateCommand(logger, toolingService, graphApiService: null); + + var args = new[] + { + "publish", + "--environment-id", "env-123", + "--server-name", "Test_Server", + "--alias", "testalias", + "--display-name", "Test Display", + "--yes", + "--secret-lifetime-months", lifetimeArg, + }; + + // Act + var exitCode = await command.InvokeAsync(args); + + // Assert — exit code surfaces failure as the user would observe it + exitCode.Should().Be(1, because: "an out-of-range secret lifetime must fail publish before any Entra app is created or the platform is called"); + + // Assert — error log names the valid range and the rejected value so the user can recover + logger.Received().Log( + LogLevel.Error, + Arg.Any(), + Arg.Is(state => state != null + && state.ToString()!.Contains("--secret-lifetime-months") + && state.ToString()!.Contains("between 1 and 24") + && state.ToString()!.Contains($"Got: {lifetimeArg}")), + Arg.Any(), + Arg.Any>()); + + // Assert — validation short-circuits before any downstream publish call + await toolingService.DidNotReceive().PublishServerAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ToolingServicePureFunctionTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ToolingServicePureFunctionTests.cs index d60ebd8f..4da14a5d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ToolingServicePureFunctionTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ToolingServicePureFunctionTests.cs @@ -208,6 +208,19 @@ public void RedactSecretsFromPayload_RedactsClientSecret() result.Should().Contain("myid"); } + [Fact] + public void RedactSecretsFromPayload_RedactsA365ProxyClientSecret() + { + // The publish request serializes the newly created A365 proxy Entra app secret as + // a365ProxyClientSecret; it must be redacted so verbose request logging never writes the + // live client secret in plaintext. + var payload = """{"a365ProxyClientId":"proxy-id","a365ProxyClientSecret":"proxysecret"}"""; + var result = Agent365ToolingService.RedactSecretsFromPayload(payload); + result.Should().NotContain("proxysecret", because: "the A365 proxy client secret must never be logged in plaintext"); + result.Should().Contain("***REDACTED***"); + result.Should().Contain("proxy-id", because: "the non-secret proxy client id is safe to log and aids diagnostics"); + } + [Fact] public void RedactSecretsFromPayload_PreservesNonSecretFields() {