From 7fbf97adcbf9e1754e26ddf92e4d1f3fd50295ae Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Sat, 6 Jun 2026 10:05:54 -0700 Subject: [PATCH 01/10] feat: add web consent redirect URIs to PublicClients Entra apps Add environment-aware consent redirect URIs (web platform) to the PublicClients app created during publish and register flows. Test/PreProd gets sdf.admin and ignite.admin URIs; Prod gets admin.cloud.microsoft. Environment is resolved from the explicit parameter, A365_ENVIRONMENT env var, or defaults to prod. Co-Authored-By: Claude Opus 4.6 --- .../Commands/PublishCommandExecutor.cs | 2 +- .../Services/EntraAppProvisioner.cs | 48 +++++++- .../Services/EntraAppProvisionerTests.cs | 105 ++++++++++++++++-- 3 files changed, 143 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 407fc31b..644b4bc2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -317,7 +317,7 @@ private void DisplayPublishSummary(ResolvedInput input) var provisioner = new EntraAppProvisioner(_logger, _graphApiService!, _retryHelper); var publicClients = await provisioner.CreatePublicClientsAppAsync( - input.ServerName, tenantId, serviceTreeId: null, warnings, ct); + input.ServerName, tenantId, serviceTreeId: null, warnings, ct: ct); return new EntraAppSet( PublicClientsClientId: publicClients.ClientId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 14d705a0..7b25776e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -22,6 +22,26 @@ internal class EntraAppProvisioner "http://localhost", ]; + private static readonly string[] TestPreProdConsentRedirectUris = + [ + "https://sdf.admin.cloud.microsoft/?ref=tools/consent", + "https://ignite.admin.cloud.microsoft/?ref=tools/consent", + ]; + + private static readonly string[] ProdConsentRedirectUris = + [ + "https://admin.cloud.microsoft/?ref=tools/consent", + ]; + + internal static string[] GetConsentRedirectUris(string environment) + { + return environment?.ToLowerInvariant() switch + { + "test" or "preprod" or "ppe" => TestPreProdConsentRedirectUris, + _ => ProdConsentRedirectUris, + }; + } + private readonly ILogger _logger; private readonly GraphApiService _graphApiService; private readonly RetryHelper _retryHelper; @@ -147,6 +167,7 @@ internal virtual async Task CreatePublicClientsAppAsync( string tenantId, string? serviceTreeId, List warnings, + string? environment = null, CancellationToken ct = default) { var appName = $"{serverName}-PublicClients"; @@ -168,6 +189,9 @@ internal virtual async Task CreatePublicClientsAppAsync( var brokerRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{clientId}"; var publicClientUris = new[] { brokerRedirectUri }.Concat(PublicClientCanonicalRedirectUris).ToArray(); + var resolvedEnvironment = environment ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var consentUris = GetConsentRedirectUris(resolvedEnvironment); + try { var success = await _retryHelper.ExecuteWithRetryAsync( @@ -176,19 +200,39 @@ internal virtual async Task CreatePublicClientsAppAsync( cancellationToken: ct); if (!success) { - var msg = $"Failed to set redirect URIs on Public Clients app '{appName}' after retries."; + var msg = $"Failed to set publicClient redirect URIs on Public Clients app '{appName}' after retries."; _logger.LogError(msg); warnings.Add(msg); } else { _logger.LogDebug( - "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + "Set {RedirectUriCount} publicClient redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", publicClientUris.Length, appName, objectId, string.Join(", ", publicClientUris)); } + + var webSuccess = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), + result => !result, + cancellationToken: ct); + if (!webSuccess) + { + var msg = $"Failed to set web redirect URIs on Public Clients app '{appName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogDebug( + "Set {RedirectUriCount} web redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + consentUris.Length, + appName, + objectId, + string.Join(", ", consentUris)); + } } catch (Exception ex) { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index 4b7c6624..436ab782 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -136,20 +136,27 @@ public async Task CreateProxyAppAsync_WhenClientIdIsEmpty_ReturnsNullAndLogsErro } [Fact] - public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerAndCanonicalRedirectUris() + public async Task CreatePublicClientsAppAsync_ProdEnvironment_SetsPublicClientAndWebConsentRedirectUris() { - var capturedUris = new List(); + var capturedPublicClientUris = new List(); + var capturedWebUris = new List(); _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-PublicClients", serviceTreeId: "svc-tree-9", Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.UpdateAppPublicClientRedirectUrisAsync( TenantId, AppObjectId, - Arg.Do(uris => capturedUris.Add(uris)), + Arg.Do(uris => capturedPublicClientUris.Add(uris)), + Arg.Any()) + .Returns(Task.FromResult(true)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, + AppObjectId, + Arg.Do(uris => capturedWebUris.Add(uris)), Arg.Any()) .Returns(Task.FromResult(true)); var warnings = new List(); - var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: "svc-tree-9", warnings); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: "svc-tree-9", warnings, environment: "prod"); result.Should().NotBeNull(); result.ClientId.Should().Be(AppClientId); @@ -157,9 +164,8 @@ public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerA result.AppName.Should().Be($"{ServerName}-PublicClients"); warnings.Should().BeEmpty(); - capturedUris.Should().ContainSingle(); - var uris = capturedUris[0]; - uris.Should().BeEquivalentTo( + capturedPublicClientUris.Should().ContainSingle(); + capturedPublicClientUris[0].Should().BeEquivalentTo( new[] { $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", @@ -173,6 +179,65 @@ public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerA "on Windows, the localhost callbacks support MSAL.NET and Copilot CLI flows, " + "and vscode.dev/redirect supports the VS Code web client. Silently " + "adding/removing/reordering entries breaks one of those flows."); + + capturedWebUris.Should().ContainSingle(); + capturedWebUris[0].Should().BeEquivalentTo( + new[] + { + "https://admin.cloud.microsoft/?ref=tools/consent", + }, + opt => opt.WithStrictOrdering()); + } + + [Theory] + [InlineData("test")] + [InlineData("preprod")] + [InlineData("ppe")] + public async Task CreatePublicClientsAppAsync_TestPreProdEnvironment_SetsTestPreProdWebConsentRedirectUris(string environment) + { + var capturedPublicClientUris = new List(); + var capturedWebUris = new List(); + _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-PublicClients", serviceTreeId: null, Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, + AppObjectId, + Arg.Do(uris => capturedPublicClientUris.Add(uris)), + Arg.Any()) + .Returns(Task.FromResult(true)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, + AppObjectId, + Arg.Do(uris => capturedWebUris.Add(uris)), + Arg.Any()) + .Returns(Task.FromResult(true)); + + var warnings = new List(); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings, environment: environment); + + result.Should().NotBeNull(); + result.ClientId.Should().Be(AppClientId); + warnings.Should().BeEmpty(); + + capturedPublicClientUris.Should().ContainSingle(); + capturedPublicClientUris[0].Should().BeEquivalentTo( + new[] + { + $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", + "http://localhost:8080/callback", + "https://vscode.dev/redirect", + "http://localhost", + }, + opt => opt.WithStrictOrdering()); + + capturedWebUris.Should().ContainSingle(); + capturedWebUris[0].Should().BeEquivalentTo( + new[] + { + "https://sdf.admin.cloud.microsoft/?ref=tools/consent", + "https://ignite.admin.cloud.microsoft/?ref=tools/consent", + }, + opt => opt.WithStrictOrdering()); } [Fact] @@ -194,13 +259,16 @@ await _graph.DidNotReceive().UpdateAppPublicClientRedirectUrisAsync( } [Fact] - public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateReturnsFalse_ReturnsIdsAndAppendsRetryWarning() + public async Task CreatePublicClientsAppAsync_WhenPublicClientRedirectUriUpdateReturnsFalse_AppendsWarning() { _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.UpdateAppPublicClientRedirectUrisAsync( TenantId, AppObjectId, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(false)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); var warnings = new List(); var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); @@ -208,7 +276,26 @@ public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateReturnsFalse_ result.ClientId.Should().Be(AppClientId); result.ObjectId.Should().Be(AppObjectId); result.AppName.Should().Be($"{ServerName}-PublicClients"); - warnings.Should().ContainSingle().Which.Should().Be($"Failed to set redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); + warnings.Should().ContainSingle().Which.Should().Be($"Failed to set publicClient redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); + } + + [Fact] + public async Task CreatePublicClientsAppAsync_WhenWebRedirectUriUpdateReturnsFalse_AppendsWarning() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(false)); + + var warnings = new List(); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + + result.ClientId.Should().Be(AppClientId); + warnings.Should().ContainSingle().Which.Should().Be($"Failed to set web redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); } [Fact] From aa1a50b0ac78153693f826da91234054b3ac2cc3 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Sat, 6 Jun 2026 10:18:30 -0700 Subject: [PATCH 02/10] fix: add web consent redirect URIs to all Entra apps, not just PublicClients Extract SetWebConsentRedirectUrisAsync helper and call it from both CreateProxyAppAsync (A365Proxy, RemoteProxy) and CreatePublicClientsAppAsync. All apps created in the addmcpserver flow now get the environment-aware consent redirect URIs on the web platform. Co-Authored-By: Claude Opus 4.6 --- .../Services/EntraAppProvisioner.cs | 48 ++++++++++--------- .../Services/EntraAppProvisionerTests.cs | 32 +++++++++++-- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 7b25776e..0de9db35 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -83,6 +83,7 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId string roleDisplay, string? serviceTreeId, int? lifetimeMonths = null, + string? environment = null, CancellationToken ct = default) { var appName = $"{serverName}-{suffix}"; @@ -111,10 +112,34 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId return null; } + await SetWebConsentRedirectUrisAsync(tenantId, app.Value.ObjectId, appName, roleDisplay, environment, ct); + _logger.LogDebug("Created {Role} app: {ClientId}", roleDisplay, app.Value.ClientId); return new ProxyAppResult(app.Value.ClientId, secret, app.Value.ObjectId, appName); } + private async Task SetWebConsentRedirectUrisAsync( + string tenantId, string objectId, string appName, string roleDisplay, + string? environment, CancellationToken ct, List? warnings = null) + { + var resolvedEnvironment = environment ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var consentUris = GetConsentRedirectUris(resolvedEnvironment); + + var success = await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, ct); + if (success) + { + _logger.LogDebug( + "Set {RedirectUriCount} web redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + consentUris.Length, appName, objectId, string.Join(", ", consentUris)); + } + else + { + var msg = $"Failed to set web redirect URIs on {roleDisplay} app '{appName}'."; + _logger.LogWarning(msg); + warnings?.Add(msg); + } + } + /// /// Best-effort compensating delete for an Entra app that was successfully created but failed /// a follow-up step (secret creation, post-create validation). Without this, partial failures @@ -189,9 +214,6 @@ internal virtual async Task CreatePublicClientsAppAsync( var brokerRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{clientId}"; var publicClientUris = new[] { brokerRedirectUri }.Concat(PublicClientCanonicalRedirectUris).ToArray(); - var resolvedEnvironment = environment ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; - var consentUris = GetConsentRedirectUris(resolvedEnvironment); - try { var success = await _retryHelper.ExecuteWithRetryAsync( @@ -214,25 +236,7 @@ internal virtual async Task CreatePublicClientsAppAsync( string.Join(", ", publicClientUris)); } - var webSuccess = await _retryHelper.ExecuteWithRetryAsync( - async retryCt => await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), - result => !result, - cancellationToken: ct); - if (!webSuccess) - { - var msg = $"Failed to set web redirect URIs on Public Clients app '{appName}' after retries."; - _logger.LogError(msg); - warnings.Add(msg); - } - else - { - _logger.LogDebug( - "Set {RedirectUriCount} web redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", - consentUris.Length, - appName, - objectId, - string.Join(", ", consentUris)); - } + await SetWebConsentRedirectUrisAsync(tenantId, objectId, appName, "Public Clients", environment, ct, warnings); } catch (Exception ex) { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index 436ab782..2570854d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -38,13 +38,17 @@ public EntraAppProvisionerTests() } [Fact] - public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulated() + public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulatedAndSetsWebConsentUris() { + var capturedWebUris = new List(); _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, AppObjectId, Arg.Do(uris => capturedWebUris.Add(uris)), Arg.Any()) + .Returns(Task.FromResult(true)); - var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7"); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7", environment: "prod"); result.Should().NotBeNull(); result!.ClientId.Should().Be(AppClientId); @@ -54,19 +58,37 @@ public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulated() await _graph.Received(1).CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()); await _graph.Received(1).AddAppPasswordAsync(TenantId, AppObjectId); + + capturedWebUris.Should().ContainSingle(); + capturedWebUris[0].Should().BeEquivalentTo( + new[] { "https://admin.cloud.microsoft/?ref=tools/consent" }, + opt => opt.WithStrictOrdering()); } [Fact] - public async Task CreateProxyAppAsync_UsesSuffixInAppName() + public async Task CreateProxyAppAsync_PreProdEnvironment_SetsPreProdWebConsentUris() { + var capturedWebUris = new List(); _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-RemoteProxy", serviceTreeId: null, Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, AppObjectId, Arg.Do(uris => capturedWebUris.Add(uris)), Arg.Any()) + .Returns(Task.FromResult(true)); - var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null, environment: "preprod"); result.Should().NotBeNull(); result!.AppName.Should().Be($"{ServerName}-RemoteProxy"); + + capturedWebUris.Should().ContainSingle(); + capturedWebUris[0].Should().BeEquivalentTo( + new[] + { + "https://sdf.admin.cloud.microsoft/?ref=tools/consent", + "https://ignite.admin.cloud.microsoft/?ref=tools/consent", + }, + opt => opt.WithStrictOrdering()); } [Fact] @@ -295,7 +317,7 @@ public async Task CreatePublicClientsAppAsync_WhenWebRedirectUriUpdateReturnsFal var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); result.ClientId.Should().Be(AppClientId); - warnings.Should().ContainSingle().Which.Should().Be($"Failed to set web redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); + warnings.Should().ContainSingle().Which.Should().Be($"Failed to set web redirect URIs on Public Clients app '{ServerName}-PublicClients'."); } [Fact] From da8b4bbb0ea1183540e977d8150224d74fc0c24b Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Sat, 6 Jun 2026 11:12:02 -0700 Subject: [PATCH 03/10] fix: append consent URIs in register executor to survive platform redirect URI overwrite The register flow's UpdateA365RedirectUrisAsync and UpdateRemoteProxyRedirectUrisAsync overwrite web.redirectUris with the platform-returned redirect URI, erasing consent URIs set earlier by the provisioner. Move consent URI injection to these final PATCH calls so the consent URIs and platform redirect URIs coexist. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 10 +++++-- .../Services/EntraAppProvisioner.cs | 3 -- .../Services/EntraAppProvisionerTests.cs | 30 +++---------------- 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 622e23c0..f705c0fc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -783,7 +783,10 @@ private async Task UpdateA365RedirectUrisAsync( { var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); - var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); + var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri) + .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .ToArray(); _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), @@ -817,7 +820,10 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( { var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); - var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); + var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri) + .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .ToArray(); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId!, remoteUris, retryCt), diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 0de9db35..0ae48aa6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -83,7 +83,6 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId string roleDisplay, string? serviceTreeId, int? lifetimeMonths = null, - string? environment = null, CancellationToken ct = default) { var appName = $"{serverName}-{suffix}"; @@ -112,8 +111,6 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId return null; } - await SetWebConsentRedirectUrisAsync(tenantId, app.Value.ObjectId, appName, roleDisplay, environment, ct); - _logger.LogDebug("Created {Role} app: {ClientId}", roleDisplay, app.Value.ClientId); return new ProxyAppResult(app.Value.ClientId, secret, app.Value.ObjectId, appName); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index 2570854d..11ebb9de 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -38,17 +38,13 @@ public EntraAppProvisionerTests() } [Fact] - public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulatedAndSetsWebConsentUris() + public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulated() { - var capturedWebUris = new List(); _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); - _graph.UpdateAppRedirectUrisAsync( - TenantId, AppObjectId, Arg.Do(uris => capturedWebUris.Add(uris)), Arg.Any()) - .Returns(Task.FromResult(true)); - var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7", environment: "prod"); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7"); result.Should().NotBeNull(); result!.ClientId.Should().Be(AppClientId); @@ -58,37 +54,19 @@ public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulatedAndSets await _graph.Received(1).CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()); await _graph.Received(1).AddAppPasswordAsync(TenantId, AppObjectId); - - capturedWebUris.Should().ContainSingle(); - capturedWebUris[0].Should().BeEquivalentTo( - new[] { "https://admin.cloud.microsoft/?ref=tools/consent" }, - opt => opt.WithStrictOrdering()); } [Fact] - public async Task CreateProxyAppAsync_PreProdEnvironment_SetsPreProdWebConsentUris() + public async Task CreateProxyAppAsync_UsesSuffixInAppName() { - var capturedWebUris = new List(); _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-RemoteProxy", serviceTreeId: null, Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); - _graph.UpdateAppRedirectUrisAsync( - TenantId, AppObjectId, Arg.Do(uris => capturedWebUris.Add(uris)), Arg.Any()) - .Returns(Task.FromResult(true)); - var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null, environment: "preprod"); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); result.Should().NotBeNull(); result!.AppName.Should().Be($"{ServerName}-RemoteProxy"); - - capturedWebUris.Should().ContainSingle(); - capturedWebUris[0].Should().BeEquivalentTo( - new[] - { - "https://sdf.admin.cloud.microsoft/?ref=tools/consent", - "https://ignite.admin.cloud.microsoft/?ref=tools/consent", - }, - opt => opt.WithStrictOrdering()); } [Fact] From 979a2b15d064649d68622eecaa0f30c6d75b58ad Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Sat, 6 Jun 2026 11:24:11 -0700 Subject: [PATCH 04/10] fix: ensure consent URIs are set on RemoteProxy and A365Proxy even without platform redirect URI When the platform does not return a redirect URI (e.g. NoAuth flow for A365Proxy, or missing remoteMCPServerProxyRedirectUri for RemoteProxy), the consent redirect URIs were skipped entirely. Add a fallback that sets consent-only web redirect URIs on all proxy apps regardless. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index f705c0fc..a89145a4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -703,20 +703,25 @@ private async Task ConfigureEntraAppsAsync( } else { - var msg = "A365 Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; + var msg = "A365 Proxy redirect URI was not returned by the server. Setting consent redirect URIs only."; _logger.LogWarning(msg); concurrentWarnings.Add(msg); + tasks.Add(SetConsentOnlyRedirectUrisAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, concurrentWarnings, ct)); } if (input.IsEntra && !string.IsNullOrWhiteSpace(remoteRedirectUri) && apps.RemoteProxyObjectId != null) { tasks.Add(UpdateRemoteProxyRedirectUrisAsync(tenantId, apps, remoteRedirectUri, concurrentWarnings, ct)); } - else if (input.IsEntra && string.IsNullOrWhiteSpace(remoteRedirectUri)) + else if (input.IsEntra && apps.RemoteProxyObjectId != null) { - var msg = "Remote MCP Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; - _logger.LogWarning(msg); - concurrentWarnings.Add(msg); + if (string.IsNullOrWhiteSpace(remoteRedirectUri)) + { + var msg = "Remote MCP Proxy redirect URI was not returned by the server. Setting consent redirect URIs only."; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); + } + tasks.Add(SetConsentOnlyRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, apps.RemoteProxyAppName, concurrentWarnings, ct)); } else if (input.IsEntra && apps.RemoteProxyObjectId == null) { @@ -848,6 +853,38 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( } } + private async Task SetConsentOnlyRedirectUrisAsync( + string tenantId, string objectId, string appName, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + try + { + var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var consentUris = EntraAppProvisioner.GetConsentRedirectUris(environment); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), + result => !result, + cancellationToken: ct); + if (!success) + { + var msg = $"Failed to set web redirect URIs on '{appName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogDebug("Set {Count} web redirect URIs on '{AppName}'", consentUris.Length, appName); + } + } + catch (Exception ex) + { + var msg = $"Failed to set web redirect URIs on '{appName}': {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + private async Task AddRemoteProxyScopePermissionAsync( string tenantId, ResolvedInput input, EntraAppSet apps, System.Collections.Concurrent.ConcurrentBag concurrentWarnings, From 637004bc27e61e5cf91aabcfecc4100f6535614c Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Sat, 6 Jun 2026 11:32:06 -0700 Subject: [PATCH 05/10] fix: address PR review - array copy safety, retry handling, test coverage - GetConsentRedirectUris returns defensive copies to prevent mutation of shared static arrays - SetWebConsentRedirectUrisAsync now uses RetryHelper for transient Graph failures, consistent with publicClient redirect URI handling - Add because: clauses to web consent URI test assertions documenting the admin consent portal contract - Add test for web redirect URI update throw path Co-Authored-By: Claude Opus 4.6 --- .../Services/EntraAppProvisioner.cs | 11 ++++--- .../Services/EntraAppProvisionerTests.cs | 31 +++++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 0ae48aa6..16d9b546 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -37,8 +37,8 @@ internal static string[] GetConsentRedirectUris(string environment) { return environment?.ToLowerInvariant() switch { - "test" or "preprod" or "ppe" => TestPreProdConsentRedirectUris, - _ => ProdConsentRedirectUris, + "test" or "preprod" or "ppe" => [.. TestPreProdConsentRedirectUris], + _ => [.. ProdConsentRedirectUris], }; } @@ -122,7 +122,10 @@ private async Task SetWebConsentRedirectUrisAsync( var resolvedEnvironment = environment ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; var consentUris = GetConsentRedirectUris(resolvedEnvironment); - var success = await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, ct); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), + result => !result, + cancellationToken: ct); if (success) { _logger.LogDebug( @@ -131,7 +134,7 @@ private async Task SetWebConsentRedirectUrisAsync( } else { - var msg = $"Failed to set web redirect URIs on {roleDisplay} app '{appName}'."; + var msg = $"Failed to set web redirect URIs on {roleDisplay} app '{appName}' after retries."; _logger.LogWarning(msg); warnings?.Add(msg); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index 11ebb9de..ee03bfec 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -186,7 +186,9 @@ public async Task CreatePublicClientsAppAsync_ProdEnvironment_SetsPublicClientAn { "https://admin.cloud.microsoft/?ref=tools/consent", }, - opt => opt.WithStrictOrdering()); + opt => opt.WithStrictOrdering(), + because: "Prod consent redirect URIs are required by the admin.cloud.microsoft " + + "consent portal. Changing these URIs breaks the admin consent flow."); } [Theory] @@ -237,7 +239,10 @@ public async Task CreatePublicClientsAppAsync_TestPreProdEnvironment_SetsTestPre "https://sdf.admin.cloud.microsoft/?ref=tools/consent", "https://ignite.admin.cloud.microsoft/?ref=tools/consent", }, - opt => opt.WithStrictOrdering()); + opt => opt.WithStrictOrdering(), + because: "Test/PreProd consent redirect URIs target the sdf and ignite admin " + + "consent portals. Changing these URIs breaks the admin consent flow " + + "in non-production environments."); } [Fact] @@ -295,7 +300,7 @@ public async Task CreatePublicClientsAppAsync_WhenWebRedirectUriUpdateReturnsFal var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); result.ClientId.Should().Be(AppClientId); - warnings.Should().ContainSingle().Which.Should().Be($"Failed to set web redirect URIs on Public Clients app '{ServerName}-PublicClients'."); + warnings.Should().ContainSingle().Which.Should().Be($"Failed to set web redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); } [Fact] @@ -315,4 +320,24 @@ public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateThrows_Return result.AppName.Should().Be($"{ServerName}-PublicClients"); warnings.Should().ContainSingle().Which.Should().Be("Failed to set redirect URIs on Public Clients app: Graph blew up"); } + + [Fact] + public async Task CreatePublicClientsAppAsync_WhenWebRedirectUriUpdateThrows_ReturnsIdsAndAppendsExceptionWarning() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + _graph.UpdateAppRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns>(_ => throw new InvalidOperationException("Graph consent update failed")); + + var warnings = new List(); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + + result.ClientId.Should().Be(AppClientId); + result.ObjectId.Should().Be(AppObjectId); + warnings.Should().ContainSingle().Which.Should().Be("Failed to set redirect URIs on Public Clients app: Graph consent update failed"); + } } From 11d7f8ab55b851074ecfb2a6611e8386bd81da82 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Mon, 8 Jun 2026 08:24:13 -0700 Subject: [PATCH 06/10] fix: read test/preprod consent redirect URIs from env var to avoid leaking internal URLs Replace hardcoded test/preprod consent redirect URIs with A365_CONSENT_REDIRECT_URIS_{ENV} env var (comma-separated). Prod URI remains hardcoded as it is public. When no env var is set, all environments fall back to the prod URI. Co-Authored-By: Claude Opus 4.6 --- .../Services/EntraAppProvisioner.cs | 17 ++-- .../Services/EntraAppProvisionerTests.cs | 88 ++++++++----------- 2 files changed, 45 insertions(+), 60 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 16d9b546..a4fab1fa 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -22,12 +22,6 @@ internal class EntraAppProvisioner "http://localhost", ]; - private static readonly string[] TestPreProdConsentRedirectUris = - [ - "https://sdf.admin.cloud.microsoft/?ref=tools/consent", - "https://ignite.admin.cloud.microsoft/?ref=tools/consent", - ]; - private static readonly string[] ProdConsentRedirectUris = [ "https://admin.cloud.microsoft/?ref=tools/consent", @@ -35,11 +29,14 @@ internal class EntraAppProvisioner internal static string[] GetConsentRedirectUris(string environment) { - return environment?.ToLowerInvariant() switch + var envKey = environment?.ToUpperInvariant() ?? "PROD"; + var customUris = Environment.GetEnvironmentVariable($"A365_CONSENT_REDIRECT_URIS_{envKey}"); + if (!string.IsNullOrWhiteSpace(customUris)) { - "test" or "preprod" or "ppe" => [.. TestPreProdConsentRedirectUris], - _ => [.. ProdConsentRedirectUris], - }; + return customUris.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + + return [.. ProdConsentRedirectUris]; } private readonly ILogger _logger; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index ee03bfec..f1c47e72 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -191,58 +191,46 @@ public async Task CreatePublicClientsAppAsync_ProdEnvironment_SetsPublicClientAn "consent portal. Changing these URIs breaks the admin consent flow."); } - [Theory] - [InlineData("test")] - [InlineData("preprod")] - [InlineData("ppe")] - public async Task CreatePublicClientsAppAsync_TestPreProdEnvironment_SetsTestPreProdWebConsentRedirectUris(string environment) + [Fact] + public void GetConsentRedirectUris_UsesEnvVarOverride() { - var capturedPublicClientUris = new List(); - var capturedWebUris = new List(); - _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-PublicClients", serviceTreeId: null, Arg.Any()) - .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); - _graph.UpdateAppPublicClientRedirectUrisAsync( - TenantId, - AppObjectId, - Arg.Do(uris => capturedPublicClientUris.Add(uris)), - Arg.Any()) - .Returns(Task.FromResult(true)); - _graph.UpdateAppRedirectUrisAsync( - TenantId, - AppObjectId, - Arg.Do(uris => capturedWebUris.Add(uris)), - Arg.Any()) - .Returns(Task.FromResult(true)); - - var warnings = new List(); - var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings, environment: environment); - - result.Should().NotBeNull(); - result.ClientId.Should().Be(AppClientId); - warnings.Should().BeEmpty(); - - capturedPublicClientUris.Should().ContainSingle(); - capturedPublicClientUris[0].Should().BeEquivalentTo( - new[] - { - $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", - "http://localhost:8080/callback", - "https://vscode.dev/redirect", - "http://localhost", - }, - opt => opt.WithStrictOrdering()); + const string envKey = "A365_CONSENT_REDIRECT_URIS_PREPROD"; + var prev = Environment.GetEnvironmentVariable(envKey); + try + { + Environment.SetEnvironmentVariable(envKey, "https://custom1.example.com/consent, https://custom2.example.com/consent"); + var uris = EntraAppProvisioner.GetConsentRedirectUris("preprod"); + uris.Should().BeEquivalentTo( + new[] { "https://custom1.example.com/consent", "https://custom2.example.com/consent" }, + opt => opt.WithStrictOrdering(), + because: "Non-prod consent redirect URIs are read from A365_CONSENT_REDIRECT_URIS_{ENV} " + + "to avoid leaking internal URLs in source code."); + } + finally + { + Environment.SetEnvironmentVariable(envKey, prev); + } + } - capturedWebUris.Should().ContainSingle(); - capturedWebUris[0].Should().BeEquivalentTo( - new[] - { - "https://sdf.admin.cloud.microsoft/?ref=tools/consent", - "https://ignite.admin.cloud.microsoft/?ref=tools/consent", - }, - opt => opt.WithStrictOrdering(), - because: "Test/PreProd consent redirect URIs target the sdf and ignite admin " + - "consent portals. Changing these URIs breaks the admin consent flow " + - "in non-production environments."); + [Fact] + public void GetConsentRedirectUris_FallsToProdWhenNoEnvVar() + { + const string envKey = "A365_CONSENT_REDIRECT_URIS_PREPROD"; + var prev = Environment.GetEnvironmentVariable(envKey); + try + { + Environment.SetEnvironmentVariable(envKey, null); + var uris = EntraAppProvisioner.GetConsentRedirectUris("preprod"); + uris.Should().BeEquivalentTo( + new[] { "https://admin.cloud.microsoft/?ref=tools/consent" }, + opt => opt.WithStrictOrdering(), + because: "When no env var override is set, all environments fall back to the " + + "prod consent redirect URI."); + } + finally + { + Environment.SetEnvironmentVariable(envKey, prev); + } } [Fact] From e2c762ab579bce585dd613c17a0f9059dad1bec8 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Mon, 8 Jun 2026 11:52:17 -0700 Subject: [PATCH 07/10] fix: address PR review - trim whitespace on environment parameter Trim leading/trailing whitespace from the environment string before building the env var key, preventing mismatches when A365_ENVIRONMENT contains spaces. Add test pinning the trim behavior. Co-Authored-By: Claude Opus 4.6 --- .../Services/EntraAppProvisioner.cs | 2 +- .../Services/EntraAppProvisionerTests.cs | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index a4fab1fa..b73f0adf 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -29,7 +29,7 @@ internal class EntraAppProvisioner internal static string[] GetConsentRedirectUris(string environment) { - var envKey = environment?.ToUpperInvariant() ?? "PROD"; + var envKey = environment?.Trim().ToUpperInvariant() ?? "PROD"; var customUris = Environment.GetEnvironmentVariable($"A365_CONSENT_REDIRECT_URIS_{envKey}"); if (!string.IsNullOrWhiteSpace(customUris)) { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index f1c47e72..262930bb 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -212,6 +212,26 @@ public void GetConsentRedirectUris_UsesEnvVarOverride() } } + [Fact] + public void GetConsentRedirectUris_TrimsWhitespaceFromEnvironment() + { + const string envKey = "A365_CONSENT_REDIRECT_URIS_TEST"; + var prev = Environment.GetEnvironmentVariable(envKey); + try + { + Environment.SetEnvironmentVariable(envKey, "https://trimmed.example.com/consent"); + var uris = EntraAppProvisioner.GetConsentRedirectUris(" test "); + uris.Should().BeEquivalentTo( + new[] { "https://trimmed.example.com/consent" }, + because: "Environment values with leading/trailing whitespace (common in " + + "shell/env var setups) must resolve to the correct env var key."); + } + finally + { + Environment.SetEnvironmentVariable(envKey, prev); + } + } + [Fact] public void GetConsentRedirectUris_FallsToProdWhenNoEnvVar() { From a2dfd91be7f3651c66acc3fc91ccf84c0926bcde Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Mon, 8 Jun 2026 13:21:39 -0700 Subject: [PATCH 08/10] fix: address PR review - dedup consent URIs, disable parallel test execution - Add Distinct(StringComparer.Ordinal) when appending consent URIs to prevent Graph rejecting duplicate entries - Add CollectionDefinition with DisableParallelization on EntraAppProvisionerTests since env-var tests mutate process state Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 2 ++ .../Services/EntraAppProvisionerTests.cs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index a89145a4..7a91cb73 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -791,6 +791,7 @@ private async Task UpdateA365RedirectUrisAsync( var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri) .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .Distinct(StringComparer.Ordinal) .ToArray(); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( @@ -828,6 +829,7 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri) .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .Distinct(StringComparer.Ordinal) .ToArray(); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index 262930bb..10d52cf4 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -10,12 +10,16 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; +[CollectionDefinition("EntraAppProvisionerTests", DisableParallelization = true)] +public class EntraAppProvisionerTestsCollection { } + /// /// Tests for . The provisioner absorbed the per-app creation flows /// that previously lived inline in PublishCommandExecutor and RegisterCommandExecutor. These tests /// pin every branch (success + each failure mode) so the executors can compose the provisioner /// without needing their own per-step coverage. /// +[Collection("EntraAppProvisionerTests")] public class EntraAppProvisionerTests { private const string ServerName = "mcp_TestServer"; From e49a298ac0affa328f2604327a35300c504a1f46 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Mon, 8 Jun 2026 16:42:52 -0700 Subject: [PATCH 09/10] fix: address PR review - DRY A365_ENVIRONMENT resolution into GetConsentRedirectUris Move A365_ENVIRONMENT env var fallback into GetConsentRedirectUris so callers don't need to resolve it themselves. Eliminates 4 duplicate Environment.GetEnvironmentVariable calls across EntraAppProvisioner and RegisterCommandExecutor. Non-prod environments without A365_CONSENT_REDIRECT_URIS_{ENV} fall back to the prod consent URI by design -- the env var override is required for non-prod. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 9 +++------ .../Services/EntraAppProvisioner.cs | 8 ++++---- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 7a91cb73..9b3fccc2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -788,9 +788,8 @@ private async Task UpdateA365RedirectUrisAsync( { var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); - var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri) - .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .Concat(EntraAppProvisioner.GetConsentRedirectUris()) .Distinct(StringComparer.Ordinal) .ToArray(); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); @@ -826,9 +825,8 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( { var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); - var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri) - .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) + .Concat(EntraAppProvisioner.GetConsentRedirectUris()) .Distinct(StringComparer.Ordinal) .ToArray(); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); @@ -862,8 +860,7 @@ private async Task SetConsentOnlyRedirectUrisAsync( { try { - var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; - var consentUris = EntraAppProvisioner.GetConsentRedirectUris(environment); + var consentUris = EntraAppProvisioner.GetConsentRedirectUris(); var success = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), result => !result, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index b73f0adf..0437c62e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -27,9 +27,10 @@ internal class EntraAppProvisioner "https://admin.cloud.microsoft/?ref=tools/consent", ]; - internal static string[] GetConsentRedirectUris(string environment) + internal static string[] GetConsentRedirectUris(string? environment = null) { - var envKey = environment?.Trim().ToUpperInvariant() ?? "PROD"; + var resolved = environment?.Trim() ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var envKey = resolved.ToUpperInvariant(); var customUris = Environment.GetEnvironmentVariable($"A365_CONSENT_REDIRECT_URIS_{envKey}"); if (!string.IsNullOrWhiteSpace(customUris)) { @@ -116,8 +117,7 @@ private async Task SetWebConsentRedirectUrisAsync( string tenantId, string objectId, string appName, string roleDisplay, string? environment, CancellationToken ct, List? warnings = null) { - var resolvedEnvironment = environment ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; - var consentUris = GetConsentRedirectUris(resolvedEnvironment); + var consentUris = GetConsentRedirectUris(environment); var success = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService.UpdateAppRedirectUrisAsync(tenantId, objectId, consentUris, retryCt), From 813bc2cf886ee2df1afaa3f13e57a388a505d90d Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Mon, 8 Jun 2026 16:53:31 -0700 Subject: [PATCH 10/10] fix: handle whitespace-only environment and trim A365_ENVIRONMENT Empty or whitespace-only environment parameter now falls through to A365_ENVIRONMENT (also trimmed) instead of producing an empty env var key. Co-Authored-By: Claude Opus 4.6 --- .../Services/EntraAppProvisioner.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 0437c62e..f907bd46 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -29,7 +29,10 @@ internal class EntraAppProvisioner internal static string[] GetConsentRedirectUris(string? environment = null) { - var resolved = environment?.Trim() ?? Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; + var trimmed = environment?.Trim(); + var resolved = string.IsNullOrEmpty(trimmed) + ? Environment.GetEnvironmentVariable("A365_ENVIRONMENT")?.Trim() ?? "prod" + : trimmed; var envKey = resolved.ToUpperInvariant(); var customUris = Environment.GetEnvironmentVariable($"A365_CONSENT_REDIRECT_URIS_{envKey}"); if (!string.IsNullOrWhiteSpace(customUris))