feat: add web consent redirect URIs to BYO Entra apps#450
Open
ragurubhaarath wants to merge 6 commits into
Open
feat: add web consent redirect URIs to BYO Entra apps#450ragurubhaarath wants to merge 6 commits into
ragurubhaarath wants to merge 6 commits into
Conversation
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 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Entra Public Clients app provisioning logic to also set environment-specific consent redirect URIs on the Web platform, in addition to the existing publicClient canonical redirect URIs, and updates unit tests accordingly.
Changes:
- Add env-to-consent-redirect-URI mapping and patch Web platform redirect URIs during Public Clients app creation.
- Expand unit tests to validate both publicClient and web redirect URI updates across prod vs test/preprod/ppe.
- Adjust publish flow call site to account for the new optional
environmentparameter insertion (via namedct:).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs | Adds environment-aware consent redirect URI selection and applies it via a second Graph PATCH to the app’s web redirect URIs. |
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs | Updates tests to assert separate PATCH calls for publicClient vs web redirect URIs across environments and failure modes. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs | Updates the provisioning call site to use named ct: after inserting the new optional method parameter. |
…Clients 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 <noreply@anthropic.com>
…irect 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 <noreply@anthropic.com>
Comment on lines
+122
to
+137
| 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); | ||
| } |
Comment on lines
319
to
+320
| var publicClients = await provisioner.CreatePublicClientsAppAsync( | ||
| input.ServerName, tenantId, serviceTreeId: null, warnings, ct); | ||
| input.ServerName, tenantId, serviceTreeId: null, warnings, ct: ct); |
Comment on lines
+183
to
+189
| capturedWebUris.Should().ContainSingle(); | ||
| capturedWebUris[0].Should().BeEquivalentTo( | ||
| new[] | ||
| { | ||
| "https://admin.cloud.microsoft/?ref=tools/consent", | ||
| }, | ||
| opt => opt.WithStrictOrdering()); |
Comment on lines
+233
to
+240
| 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()); |
Comment on lines
+786
to
+789
| var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; | ||
| var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri) | ||
| .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) | ||
| .ToArray(); |
Comment on lines
+823
to
+826
| var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; | ||
| var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri) | ||
| .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) | ||
| .ToArray(); |
Comment on lines
+282
to
299
| [Fact] | ||
| public async Task CreatePublicClientsAppAsync_WhenWebRedirectUriUpdateReturnsFalse_AppendsWarning() | ||
| { | ||
| _graph.CreateEntraAppAsync(TenantId, Arg.Any<string>(), serviceTreeId: Arg.Any<string?>(), Arg.Any<CancellationToken>()) | ||
| .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); | ||
| _graph.UpdateAppPublicClientRedirectUrisAsync( | ||
| TenantId, AppObjectId, Arg.Any<string[]>(), Arg.Any<CancellationToken>()) | ||
| .Returns(Task.FromResult(true)); | ||
| _graph.UpdateAppRedirectUrisAsync( | ||
| TenantId, AppObjectId, Arg.Any<string[]>(), Arg.Any<CancellationToken>()) | ||
| .Returns(Task.FromResult(false)); | ||
|
|
||
| var warnings = new List<string>(); | ||
| 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'."); | ||
| } |
ragurubhaarath
changed the title
…thout 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 <noreply@anthropic.com>
pronak657
previously approved these changes
Jun 6, 2026
…rage - 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 <noreply@anthropic.com>
Comment on lines
+36
to
+42
| internal static string[] GetConsentRedirectUris(string environment) | ||
| { | ||
| return environment?.ToLowerInvariant() switch | ||
| { | ||
| "test" or "preprod" or "ppe" => [.. TestPreProdConsentRedirectUris], | ||
| _ => [.. ProdConsentRedirectUris], | ||
| }; |
Comment on lines
+225
to
+233
| 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()); |
Comment on lines
+791
to
+794
| var environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; | ||
| var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri) | ||
| .Concat(EntraAppProvisioner.GetConsentRedirectUris(environment)) | ||
| .ToArray(); |
…aking 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-PublicClientsEntra app created duringdevelop-mcp publishanddevelop-mcp register-external-mcp-serverflowshttps://sdf.admin.cloud.microsoft/?ref=tools/consentandhttps://ignite.admin.cloud.microsoft/?ref=tools/consenthttps://admin.cloud.microsoft/?ref=tools/consentA365_ENVIRONMENTenv var, or defaults toprodTest plan
register-external-mcp-serveragainst PreProd — confirmed publicClient URIs (4) and web URIs (2) are set separately via two PATCH calls🤖 Generated with Claude Code