Skip to content

Conversation

@gladjohn
Copy link
Contributor

Fixes #- Mark API as experimental

Changes proposed in this request
This pull request refactors how client assertion delegates are registered in ConfidentialClientApplicationBuilder and updates related unit tests to enable experimental features required for these APIs. The main change is the introduction of an internal helper method for setting client assertion providers, which improves code clarity and enforces experimental feature validation. The unit tests are updated to explicitly enable experimental features to ensure correct test coverage.

Refactoring of client assertion registration:

  • Added a new internal method WithClientAssertionInternal to encapsulate the logic for setting client assertion providers, and updated all public WithClientAssertion overloads to use this method. This also adds validation for the use of experimental features. (src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs) [1] [2] [3] [4]

Test updates for experimental feature usage:

  • Updated all relevant unit tests to call .WithExperimentalFeatures(true) when using WithClientAssertion, ensuring that tests reflect the required usage of experimental APIs. (tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientAssertionTests.cs, tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Testing
Have unit, integration tests

Performance impact
none

Documentation

  • All relevant documentation is updated.

@gladjohn gladjohn requested a review from a team as a code owner October 24, 2025 19:41
public ConfidentialClientApplicationBuilder WithClientAssertion(Func<AssertionRequestOptions,
CancellationToken, Task<ClientSignedAssertion>> clientSignedAssertionProvider)
{
ValidateUseOfExperimentalFeature();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have telemetry that this does not break anyone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mTLS POP is an internal only feature, so we should not be breaking anyone here.

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be limited to MTLS though is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good point @neha-bhargava and @trwalke. I will let @bgavrilMS know about this.

Copy link
Contributor

@neha-bhargava neha-bhargava left a comment

Choose a reason for hiding this comment

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

Approve with comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants