Add mTLS PoP support for WithCertificate(() => x509) (DynamicCertificateClientCredential)

[gladjohn]

What does this PR do?

Adds mTLS Proof-of-Possession support for the dynamic certificate credential (WithCertificate(() => x509)). Previously, calling .WithMtlsProofOfPossession() with a dynamic certificate provider threw MtlsCertificateNotProvided at preflight because MtlsPopParametersInitializer didn't handle DynamicCertificateClientCredential.

The provider is invoked exactly once per mTLS token request.

Fixes #5943

How the single invocation works

1. Preflight (MtlsPopParametersInitializer.InitExplicitMtlsPopAsync) invokes the credential's GetCredentialMaterialAsync(Mode == Mtls) polymorphically (via ResolveMtlsMaterialAsync) to resolve the certificate, builds the cnf claim, validates AAD region/tenant constraints, and stashes the resolved cert on p.MtlsCertificate.
2. Credential material resolution (CredentialMaterialResolver.ResolveAsync) sees that requestParams.MtlsCertificate is set and the credential is a CertificateAndClaimsClientCredential, and short-circuits to (empty params, cert) without re-invoking the credential — avoiding a second provider delegate call at runtime.

The implicit Bearer-over-mTLS path (SendCertificateOverMtls = true) goes through the same polymorphic resolve and benefits from the same single-invocation guarantee.

Changes

MtlsPopParametersInitializer.cs

• InitExplicitMtlsPopAsync resolves the cert polymorphically via ResolveMtlsMaterialAsync (no concrete-credential downcasts) and translates MsalError.InvalidCredentialMaterial to MtlsCertificateNotProvided to preserve the public mTLS PoP error-code contract.
• TryInitImplicitBearerOverMtlsAsync (SendCertificateOverMtls = true path) uses the same polymorphic resolver. The Case 1 comment now correctly describes the contract: ConfidentialClientApplicationBuilder.Validate already rejects the non-cert pairing at construction time, so the polymorphic resolve is guaranteed to succeed.
• BuildPreflightContext extracted as a single chokepoint for CredentialContext.Create(...), reused across the preflight paths.
• Guarded TenantId = AuthorityInfo.GetFirstPathSegment(...) behind AuthorityType == Aad so non-AAD authorities (ADFS, DSTS, Generic, MI) no longer throw InvalidOperationException at preflight; TenantId is left null and runtime resolution fills it in.
• <remarks> on InitExplicitMtlsPopAsync now covers all four GetCredentialMaterialAsync(Mtls) outcomes, including the WithClientClaims direct-throw path.

CertificateAndClaimsClientCredential.cs

• GetCredentialMaterialAsync(Mode == Mtls) returns (empty params, cert) from a single provider invocation. The comment above the cert resolution call spells out the invariant for subclass authors: mTLS-mode output must remain (empty, cert), because CredentialMaterialResolver.ResolveAsync short-circuits this method at runtime when a preflight cert is present — overriding this method alone will be silently bypassed.
• _claimsToSign is not null guard inside the mTLS branch (existing WithClientClaims rejection) keeps the MsalError.MtlsCertificateNotProvided error code and now throws with MsalErrorMessage.MtlsNotSupportedWithClientClaimsMessage. The guard fires on every mTLS-mode resolution path — explicit PoP and implicit Bearer-over-mTLS (SendCertificateOverMtls = true) — so the message names both transports rather than only Proof-of-Possession.

CredentialMaterialResolver.cs

• ResolveAsync short-circuits to (empty, requestParams.MtlsCertificate) when the credential is a CertificateAndClaimsClientCredential and the preflight has already resolved a cert. The block is flanked by a documented invariant: subclasses that need different mTLS-mode behaviour must change this short-circuit too — overriding the method alone will be silently bypassed.

MsalErrorMessage.cs

• New constant MtlsNotSupportedWithClientClaimsMessage describing the WithClientClaims + mTLS conflict for both PoP and Bearer-over-mTLS transports.

MtlsPopTests.cs

• MtlsPop_WithDynamicCertificate_WithoutRegion_UsesGlobalMtlsEndpointAsync — dynamic cert + mTLS PoP without region falls through to the global mTLS endpoint, matching the static-cert behavior validated by MtlsPop_WithoutRegion_UsesGlobalMtlsEndpoint.
• MtlsPop_WithDynamicCertificate_NullFromProvider_ThrowsAsync — null from provider throws MtlsCertificateNotProvided.
• MtlsPop_WithDynamicCertificate_SuccessAsync — end-to-end success with Assert.AreEqual(1, providerCallCount, ...) locking in the single-invocation invariant. If the resolver short-circuit is ever broken, this test fails with a self-describing message.
• MtlsPopWithoutCertificateWithClientClaimsAsync — tightened with StringAssert.Contains(ex.Message, "WithClientClaims") so a future error-message refactor cannot silently revert the WithClientClaims diagnostic.
• SendCertificateOverMtls_WithClientClaims_ThrowsClearMessageAsync — new regression test covering the previously-uncovered SendCertificateOverMtls = true + WithClientClaims combo (no explicit .WithMtlsProofOfPossession()). Asserts the message names both transports and the WithClientClaims API.

Design decisions

• Guard lives in CertificateAndClaimsClientCredential.GetCredentialMaterialAsync, the common base for both static (CertificateClientCredential) and dynamic (DynamicCertificateClientCredential) certificate credentials. Subclasses pass claimsToSign: null, so the guard only fires for the WithClientClaims(cert, claims) configuration.
• Single provider invocation is enforced by test, not convention. The providerCallCount == 1 assertion in MtlsPop_WithDynamicCertificate_SuccessAsync is the contract — any regression that re-introduces a second provider call fails this test loudly.
• Single-invocation contract: Option 2 (resolver short-circuit + documented invariant) rather than Option 1 (explicit CredentialContext.PreResolvedCertificate field). Option 2 is the smaller change and the invariant is enforced by test plus comments on both sides; Option 1 would add a context field used by exactly one credential type.
• No new public API surface. No PublicAPI.Unshipped.txt updates required.

Tests

• Build: 0 warnings, 0 errors across netstandard2.0, net472, net462, net8.0.
• Full unit test suite (Microsoft.Identity.Test.Unit, net8.0): 2119/2119 passing, 19 skipped (unrelated platform-gated).

[Copilot]

Pull request overview

Adds mTLS Proof-of-Possession (PoP) support for dynamic certificate credentials (WithCertificate(() => x509)) by wiring dynamic cert resolution into the mTLS PoP preflight path and extending unit coverage for these scenarios.

Changes:

• Extend MtlsPopParametersInitializer to handle DynamicCertificateClientCredential during explicit mTLS PoP initialization.
• Refactor certificate-provider validation in CertificateAndClaimsClientCredential and add a preflight resolution helper.
• Add unit tests covering dynamic certificate + mTLS PoP success and failure cases.

Reviewed changes

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

File	Description
tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs	Adds unit tests for dynamic certificate + mTLS PoP (region missing, null provider result, success path).
src/client/Microsoft.Identity.Client/Internal/ClientCredential/CertificateAndClaimsClientCredential.cs	Extracts shared certificate validation and introduces a preflight certificate resolution method.
src/client/Microsoft.Identity.Client/ApiConfig/Parameters/MtlsPopParametersInitializer.cs	Adds dynamic cert handling for explicit mTLS PoP preflight and expands callback options (Authority/TenantId).

[bgavrilMS]

When will this go in @gladjohn ?

[gladjohn]

When will this go in @gladjohn ?

@bgavrilMS - ready for review. Saw you closed #5886 as not_planned, so I pivoted this PR to honor the #5943 principle directly instead of deferring: the certificate provider is now invoked exactly once per mTLS PoP token request.

[Copilot]

Pull request overview

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

[gladjohn]

@bgavrilMS @neha-bhargava would appreciate some reviews and feedback on this one

[bgavrilMS]

@Robbie-Microsoft - pls help review this.

[bgavrilMS]

Try to solve so that no upcasting is needed.

[Robbie-Microsoft]

Thorough review pass — most of this PR is in good shape, but there are a few items worth tightening before merge. Pasting them inline plus one PR-description issue here:

1) PR description does not match the implementation.

The "Changes / CredentialContext.cs" bullet says:

Added PreResolvedCertificate init-only property. When set and Mode == Mtls, credential implementations MUST reuse this certificate instead of invoking their provider delegate again.

This property does not exist in CredentialContext.cs in this PR (zero references in the repo). The actual mechanism is the resolver short-circuiting on requestParams.MtlsCertificate directly in CredentialMaterialResolver.ResolveAsync — see inline comment there. The credential layer never sees the pre-resolved cert; only the resolver does.

Similarly the "Design decisions" bullet says:

Matches DynamicCertificateClientCredential specifically, not the base class CertificateAndClaimsClientCredential, to avoid unintentionally changing behavior for WithClientClaims(...).

…but the new _claimsToSign is not null guard is in fact in the base class CertificateAndClaimsClientCredential.GetCredentialMaterialAsync. Public-API behavior for WithClientClaims(cert, claims) + WithMtlsProofOfPossession() is preserved (still throws MtlsCertificateNotProvided, as locked in by MtlsPopWithoutCertificateWithClientClaimsAsync), so this isn't a functional regression — but the description should be rewritten to describe what was actually built. Reviewers and release-note authors will otherwise miss the actual contract.

Nothing here blocks merging on functionality grounds — public-API behavior is preserved end-to-end and the new dynamic-cert tests are good. The four items below are the cleanups I'd want before this lands.

stale