Add WithRequestOverMtls() API and E2E test: attested bearer via IMDSv2 mTLS flow - #6086
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end regression test in the Managed Identity IMDSv2 E2E suite to validate that when .WithMtlsProofOfPossession() is omitted (even if .WithAttestationSupport() is present), MSAL returns a standard Bearer token (no binding certificate) on an IMDSv2-capable host.
Changes:
- Added a new
[DataRow]-driven E2E test covering SAMI and UAMI (clientId) scenarios for the “no mTLS PoP requested” path. - Asserts Bearer token shape (
TokenType == "Bearer",BindingCertificate == null) and that the token was acquired from the identity provider.
|
We will need a new Bearer API to support this e2e |
|
Copilot resolve conflicts |
Conflicts resolved in commit
|
…ards - Rename public API WithMtlsBearerToken -> WithBearerOverMtls. - Rename internal flag IsMtlsBearerRequested -> PreferMsiV2 (per bgavrilMS review on AcquireTokenCommonParameters). - Add ArgumentNullException guard to WithBearerOverMtls (parity with WithMtlsProofOfPossession). - Fail fast when WithMtlsProofOfPossession and WithBearerOverMtls are combined (mutually exclusive). - Clarify the mTLS Bearer comment in ManagedIdentityAuthRequest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e65d13d3-9f55-4d28-8224-6b08ff1bd85b
Resolve conflicts: drop removed MI ClientClaims propagation (main removed it); keep PreferMsiV2 routing; Unshipped.txt keeps only WithBearerOverMtls (the rest shipped on main). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e65d13d3-9f55-4d28-8224-6b08ff1bd85b
|
Updated the branch to current
MI-focused unit tests pass locally (530 passed / 0 failed / 1 skipped, net8.0) and the E2E project compiles. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityPopExtensions.cs:90
- The PR description and example code refer to a new public API named
WithMtlsBearerToken(), but the implementation and PublicAPI entries introduceWithBearerOverMtls(). Please align the final public API name with the PR contract now (either rename the method/tests/PublicAPI toWithMtlsBearerToken, or update the PR description/example accordingly) to avoid confusion and potential follow-up breaking changes.
public static AcquireTokenForManagedIdentityParameterBuilder WithBearerOverMtls(
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:54
PreferMsiV2is now routed through the same boolean path asIsMtlsPopRequested(by OR-ing them). This means that if the VM only supports IMDSv1, the code will throwMsalError.MtlsPopTokenNotSupportedinImdsV1/MsalErrorMessage.MtlsPopTokenNotSupportedinImdsV1, which specifically mentions “mTLS Proof of Possession … private preview” and will be misleading for.WithBearerOverMtls()callers. Consider using a distinct error code/message for the mTLS-bearer-over-IMDSv2 requirement (or a more general mTLS-over-managed-identity not supported message) whenPreferMsiV2is the reason IMDSv2 is required.
AbstractManagedIdentity msi = await GetOrSelectManagedIdentitySourceAsync(requestContext, parameters.IsMtlsPopRequested || parameters.PreferMsiV2, cancellationToken).ConfigureAwait(false);
Per naming discussion with Bogdan, rename the public managed identity API WithBearerOverMtls() to WithRequestOverMtls(). Updates the method, the two mutual-exclusivity error messages, the IMDSv1-fallback log hint, the XML <see cref>, all 6 PublicAPI.Unshipped.txt files, and the E2E test (call, method name, DataRow display names, comments). The internal PreferMsiV2 flag and the confidential-client SendCertificateOverMtls 'bearer over mTLS' concept names are intentionally left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e65d13d3-9f55-4d28-8224-6b08ff1bd85b
|
Renamed the public API Scope of the rename (commit
Left intentionally unchanged: the internal Product + E2E test projects build clean (0 warn / 0 err). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityPopExtensions.cs:91
- The PR description/sample code refers to a new public API named
WithMtlsBearerToken(), but the implementation and PublicAPI entries addWithRequestOverMtls(). Please align the PR description (and any intended naming) with the actual shipped API to avoid confusion for reviewers and consumers.
/// <summary>
/// Uses the IMDSv2 attested flow (Credential Guard–issued certificate over mTLS) to acquire
/// a standard bearer token. The mTLS certificate authenticates the connection to the ESTS
/// token endpoint, but the returned token carries <c>token_type=bearer</c> and has no
/// binding certificate in the <see cref="AuthenticationResult"/>.
/// Requires Windows Credential Guard (VBS) to be enabled on the host.
/// When attestation is required, call <c>.WithAttestationSupport()</c> (from the
/// <c>Microsoft.Identity.Client.KeyAttestation</c> package) after this method.
/// </summary>
/// <param name="builder">The AcquireTokenForManagedIdentityParameterBuilder instance.</param>
/// <returns>The builder to chain .With methods.</returns>
public static AcquireTokenForManagedIdentityParameterBuilder WithRequestOverMtls(
this AcquireTokenForManagedIdentityParameterBuilder builder)
Adds a new public API
WithMtlsBearerToken()that uses the full IMDSv2 attested mTLS flow (Credential Guard–issued certificate, mTLS connection to ESTS) but requeststoken_type=bearer, returning a standard bearer token with no binding certificate. Also adds the corresponding E2E test.Changes proposed in this request
WithMtlsBearerToken()extension method toManagedIdentityPopExtensions.cs(Windows-only, mirrorsWithMtlsProofOfPossession()but sendstoken_type=bearerto ESTS instead oftoken_type=mtls_pop)IsMtlsBearerRequestedflag throughAcquireTokenCommonParameters,AcquireTokenForManagedIdentityParameters,AuthenticationRequestParameters,AcquireTokenForManagedIdentityParameterBuilder,ManagedIdentityAuthRequest, andAbstractManagedIdentityManagedIdentityClientto routeIsMtlsBearerRequestedto IMDSv2 (same asIsMtlsPopRequested)ImdsV2ManagedIdentitySource.CreateRequestAsyncto cover both PoP and ****** under a unifiedcredential_guard_not_availableerror codemtls_bearerkey component) to keep them distinct from PoP tokens and plain IMDSv1 bearer tokensPublicAPI.Unshipped.txtfor all target frameworksAcquireToken_OnImdsV2_WithAttestation_NoMtlsPoP_ReturnsBearerE2E test withAcquireToken_OnImdsV2_WithMtlsBearerToken_ReturnsBearer, which calls.WithMtlsBearerToken().WithAttestationSupport()and assertsTokenType == "Bearer",BindingCertificate == null, absence of thecnfclaim in the JWT payload, andTokenSource == IdentityProvidercredential_guard_not_availableTesting
Updated E2E test uses
.WithMtlsBearerToken()and runs on the MSALMSIV2 pool (Credential Guard / VBS enabled). Gracefully marks inconclusive if Credential Guard is unavailable. Unit test for the KeyGuard-not-available error path updated to match the new unified error code.Performance impact
None.
Documentation