Skip to content

Do not carry authority port to global instance discovery - #6155

Merged
Bogdan Gavril (bgavrilMS) merged 3 commits into
mainfrom
fix/6154-global-discovery-port
Sep 17, 2026
Merged

Bogdan Gavril (bgavrilMS) merged 3 commits into
mainfrom
fix/6154-global-discovery-port

Conversation

@bgavrilMS

@bgavrilMS Bogdan Gavril (bgavrilMS) commented Jul 31, 2026 •

Copy link
Copy Markdown
Member

Summary

  • use an authority's custom port for instance discovery only when discovery targets that same host
  • use the default HTTPS port when an unknown authority is redirected to the global discovery host
  • preserve the custom port on authorization and token endpoints
  • update the custom-port regression coverage and shared discovery mock behavior

Fixes #6154

Testing

  • dotnet build src\client\Microsoft.Identity.Client\Microsoft.Identity.Client.csproj --framework net8.0 --no-restore --verbosity quiet

Review follow-up

  • Added a direct network-provider test proving a custom port is retained when discovery uses the authority host.

  • Local targeted tests were attempted on net8.0 and net48. Execution was blocked by an MSTest adapter get_DataRow implementation error on net8.0 and a missing System.Memory assembly on net48. Cleaning net8.0 outputs and restoring dependencies did not resolve the net8.0 runner error. No passing test result is claimed.

  • Added a negative regression test with no custom discovery endpoint supplied to MSAL or the mock helper. For https://localhost:5215/common/, it explicitly asserts the discovery host, port 443, and full endpoint path.

  • Re-ran the three targeted custom-port tests on net8.0 and net48; execution remains blocked by the runner errors noted above.

assistance: agentic-cli
type: bug
agent-tool: copilot-cli
agent-model: gpt-6-astra
work-item: AB#n/a

Copilot AI lite review requested due to automatic review settings July 31, 2026 14:41
@bgavrilMS
Bogdan Gavril (bgavrilMS) requested a review from a team as a code owner July 31, 2026 14:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes instance discovery URL construction so that a custom authority port is not propagated when MSAL redirects instance discovery to the global discovery host, while still preserving the custom port for authorization/token endpoints.

Changes:

  • Update instance discovery endpoint computation to only apply the authority’s custom port when discovery targets the same host.
  • Align the shared instance discovery mock helper with the new endpoint behavior.
  • Update/rename the custom-port regression unit test to validate “no port on global discovery” behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/Microsoft.Identity.Test.Unit/CoreTests/InstanceTests/AadAuthorityTests.cs Updates the regression test to ensure custom port is preserved for authority usage but not applied to global instance discovery.
src/client/Microsoft.Identity.Lab.Api/Http/MockHttpManagerExtensions.cs Adjusts discovery mock URL construction to mirror the new “port only when same host” logic.
src/client/Microsoft.Identity.Client/Instance/Discovery/NetworkMetadataProvider.cs Fixes production instance discovery endpoint creation to avoid carrying the authority port to the global discovery host.

Copilot AI review requested due to automatic review settings September 15, 2026 13:16
@bgavrilMS Bogdan Gavril (bgavrilMS) added agentic-cli Terminal AI agent drove the bulk of the edits bug labels Sep 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Targeted test execution was blocked, requiring final human review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 15, 2026 15:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Targeted tests were blocked by runner and dependency errors, requiring final human review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 15, 2026 17:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Use a custom authority port only when instance discovery targets the same host. Preserve the port on the authority itself for authorization and token requests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8fd80bfc-67a0-4961-86f7-e911aaebfc13
Add direct network-provider regression coverage requested in PR review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 523c427f-a9a4-435d-a330-be66fc474897
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 073961ec-5315-4e60-a0c0-fd264fb0c055
Copilot AI review requested due to automatic review settings September 17, 2026 14:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Targeted regression tests remain blocked by test-runner and dependency errors.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@bgavrilMS
Bogdan Gavril (bgavrilMS) merged commit 05e4829 into main Sep 17, 2026
19 checks passed
@bgavrilMS
Bogdan Gavril (bgavrilMS) deleted the fix/6154-global-discovery-port branch September 17, 2026 16:46
This was referenced Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentic-cli Terminal AI agent drove the bulk of the edits bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not carry authority port to global instance discovery endpoint

4 participants