Skip to content

MSAL MI: restore classic fallback behavior unless GetManagedIdentitySourceAsync() is explicitly invoked#5815

Merged
gladjohn merged 6 commits intomainfrom
gladjohn/probe_classic
Mar 9, 2026
Merged

MSAL MI: restore classic fallback behavior unless GetManagedIdentitySourceAsync() is explicitly invoked#5815
gladjohn merged 6 commits intomainfrom
gladjohn/probe_classic

Conversation

@gladjohn
Copy link
Copy Markdown
Contributor

@gladjohn gladjohn commented Mar 6, 2026

Fixes #5812 #5651

Changes proposed in this request
This pull request refactors and improves the managed identity source discovery and caching logic in the MSAL library. The changes clarify the distinction between explicit source discovery and implicit selection, ensure correct caching of discovery results (including "NoneFound"), and improve error handling for IMDSv2 scenarios. Several tests are updated to reflect the new discovery order and caching behavior. The most important changes are grouped below by theme.

Managed Identity Source Discovery & Caching

  • Introduced explicit caching for managed identity source discovery results, including handling for "NoneFound" and separation from implicit source selection. Added s_isSourceDiscoveryCached and s_cachedSourceResult fields and updated logic in ManagedIdentityClient to use them. (src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs) [1] [2]
  • Refactored GetOrSelectManagedIdentitySourceAsync to use cached discovery results when available, otherwise select source based on environment variables and default to IMDS without probing. Explicit discovery now probes IMDSv1 first, then IMDSv2, and caches the result. (src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs) [1] [2] [3]
  • Updated ManagedIdentityApplication.GetManagedIdentitySourceAsync to use the new caching mechanism and removed unnecessary parameters. (src/client/Microsoft.Identity.Client/ManagedIdentityApplication.cs)

IMDSv2 Error Handling

  • Improved error handling for IMDSv2 CSR endpoint: a 404 from the endpoint now throws a client exception indicating that mTLS PoP is not supported on IMDSv1-only hosts, providing clearer feedback to callers. (src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs)

Test Updates

  • Updated unit and E2E tests to reflect new discovery logic, including correct probe order (IMDSv1 first), removal of unnecessary environment variable checks, and validation of cached discovery results. (tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs, tests/Microsoft.Identity.Test.E2e/ManagedIdentityImdsV2Tests.cs) [1] [2]
  • Removed redundant IMDSv1 probe mocks from unit tests to align with the new discovery and caching flow. (tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsTests.cs) [1] [2] [3] [4] [5] [6] [7]

These changes make the managed identity source selection more robust, predictable, and easier to reason about, and improve developer experience when using mTLS PoP and IMDSv2.

Testing
unit, integration, and e2e

Performance impact
none

Documentation

  • All relevant documentation is updated.

@gladjohn gladjohn requested a review from a team as a code owner March 6, 2026 02:48
Copy link
Copy Markdown
Member

@bgavrilMS bgavrilMS left a comment

Choose a reason for hiding this comment

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

Single responsability principle

Copilot AI review requested due to automatic review settings March 6, 2026 13:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors managed identity (MI) source selection to restore “classic” fallback behavior by defaulting to IMDS without probing, while making IMDS probing an explicit opt-in via GetManagedIdentitySourceAsync() and caching the explicit discovery result (including “NoneFound”).

Changes:

  • Split implicit MI source selection (env-var detection + default-to-IMDS) from explicit discovery (IMDS probing + caching).
  • Updated IMDSv2 CSR error handling to translate CSR-endpoint 404s into a clearer client-side failure mode.
  • Adjusted unit/E2E tests to align with the new probe order (IMDSv1 first) and caching semantics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs Introduces explicit discovery result caching and removes IMDS probing from the default token path.
src/client/Microsoft.Identity.Client/ManagedIdentityApplication.cs Routes public GetManagedIdentitySourceAsync through the new explicit discovery API/caching behavior.
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs Adds special-case handling for CSR endpoint 404 to produce a client exception.
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ManagedIdentityTests.cs Updates unit tests to match new discovery/probe behavior and cached “NoneFound” semantics.
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs Updates probe order expectations and adds coverage for mTLS PoP without prior discovery.
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsTests.cs Removes now-unneeded probe mocks due to default path no longer probing.
tests/Microsoft.Identity.Test.E2e/ManagedIdentityImdsV2Tests.cs Adjusts E2E behavior around attestation prerequisites.

@gladjohn
Copy link
Copy Markdown
Contributor Author

gladjohn commented Mar 6, 2026

Single responsability principle

Implemented the single responsibility feedback in db1dd6f (“single responsibility”):

  • Removed the redundant s_isSourceDiscoveryCached flag (use s_cachedSourceResult == null sentinel instead)
  • Made s_cachedSourceResult private
  • Removed cache-handling logic from ManagedIdentityApplication (it now delegates to ManagedIdentityClient)

This keeps discovery/caching contained to ManagedIdentityClient and avoids leaking cache mechanics into the application layer.

@gladjohn gladjohn requested a review from bgavrilMS March 6, 2026 13:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

@gladjohn gladjohn self-assigned this Mar 9, 2026
Copilot AI review requested due to automatic review settings March 9, 2026 16:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

@gladjohn gladjohn merged commit 282a9e6 into main Mar 9, 2026
12 checks passed
@gladjohn gladjohn deleted the gladjohn/probe_classic branch March 9, 2026 17:21
This was referenced Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MSAL MI: restore classic fallback behavior unless GetManagedIdentitySourceAsync() is explicitly invoked

4 participants