Add Bearer-over-mTLS (sendCertificateOverMtls) for confidential clients - #1054
Draft
Robbie-Microsoft wants to merge 3 commits into
Draft
Add Bearer-over-mTLS (sendCertificateOverMtls) for confidential clients#1054Robbie-Microsoft wants to merge 3 commits into
Robbie-Microsoft wants to merge 3 commits into
Conversation
Present the app's SN/I certificate as the client certificate on the mTLS handshake to the token endpoint and receive a plain (unbound) Bearer token, ported from MSAL.NET's CertificateOptions.SendCertificateOverMtls (.NET PRs #5849 client-credentials, #6009 OBO/refresh/auth-code). This is distinct from Task-1 mTLS Proof-of-Possession: mtls_pop binds the token to the certificate (token_type=mtls_pop, thumbprint-fenced cache); Bearer-over-mTLS uses the same mTLS transport but returns an ordinary Bearer token that is NOT bound to the certificate. - New app-level ConfidentialClientApplication.Builder.sendCertificateOverMtls(boolean) (default false); getter on ConfidentialClientApplication + IConfidentialClientApplication (mirrors the sendX5c pattern). - build() fails fast with MsalClientException (CERTIFICATE_REQUIRED_FOR_MTLS) when the flag is set without a certificate credential (mirrors .NET InvalidCredentialMaterial). - Generalize TokenRequestExecutor routing from isMtlsProofOfPossession() to usesMtlsTransport() = mtls_pop OR Bearer-over-mTLS. The Bearer branch routes to the mTLS endpoint and forces the x5c chain on the client_assertion (regardless of app.sendX5c()), but keeps token_type Bearer (no token_type=mtls_pop / req_cnf) and produces a plain Bearer cache entry (not thumbprint-fenced). A per-request mtlsProofOfPossession() always takes precedence over the app-level flag. - Honored by every confidential flow (client credentials, on-behalf-of, refresh token, authorization code) by reading the flag off the application rather than a request cast. Tests: - BearerOverMtlsTest (13 unit cells via mockConstruction(DefaultHttpClient)): config (default/stored/non-cert fail-fast); client credentials (global mtlsauth + Bearer + client_assertion with forced x5c and no mtls_pop/req_cnf; regional; per-request mtls_pop wins; plain-Bearer cache); user flows (OBO/refresh/auth-code route to mtlsauth with the correct grant + forced-x5c assertion; OBO 2nd call served from cache; OBO without the flag uses the regular login endpoint). - MtlsPopIT: live Bearer-over-mTLS client-credentials cell + cache-hit (SN/I-allow-listed app), and a skip-gated OBO cell documenting the user-flow allow-listing gap (AADSTS700027 / AADSTS392189). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…all cache Lock the Bearer-over-mTLS 2nd-call cache-serve seam deterministically in CI. The existing OBO 2nd-call test uses instanceDiscovery(false), which skips the metadata resolution path where .NET's SecondCallDoesNotCrash regression lived. This adds a variant with instanceDiscovery(true) (seeding the login-host metadata to avoid a live IMDS call) that asserts the 2nd call is served from cache without crashing and that the Bearer AT is cached under the login host, never the rewritten mtlsauth host. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per cross-SDK parity review: name the gate (AadInstanceDiscoveryProvider .shouldUseRegionalEndpoint routes regionally only for ClientCredentialRequest, so user flows fall back to the global endpoint). The OBO+region cell locks that Bearer-over-mTLS does not accidentally introduce regional routing on a user flow msal4j does not support there. Comment-only; behavior unchanged (14/14 green). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Hi, I have only one problem. We are adding and adding more functionality in our tokenrequestexecutor. This will lead to a massive degeneration of our code in the future when everybody does that. How about we do a refactoring after this PR and use the composite pattern to have a functionality based class like mTLS that add functionality to an object? @Robbie-Microsoft How is your opinion on that? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bearer-over-mTLS (
sendCertificateOverMtls)Ports MSAL.NET's
CertificateOptions.SendCertificateOverMtls(.NET PRs #5849 client-credentials, #6009 OBO/refresh/auth-code).What it is: present the app's SN/I certificate as the client certificate on the mTLS handshake to the token endpoint, route to the mTLS endpoint (
mtlsauth.*), and receive a plain Bearer access token. The certificate authenticates the transport; the token is not bound to it.How it differs from existing surfaces:
token_type=mtls_pop) and fences the cache by certificate thumbprint.private_key_jwtBearer path: that signs an assertion to the regular token endpoint; the cert is never on the TLS handshake. Bearer-over-mTLS = cert on the handshake → Bearer from the mTLS endpoint.API
ConfidentialClientApplication.Builder.sendCertificateOverMtls(boolean)— app-level, defaultsfalse(zero behavior change when unset).build()fails fast withMsalClientException(AuthenticationErrorCode.CERTIFICATE_REQUIRED_FOR_MTLS) if the flag is set without a certificate credential.Behavior
token_type=Bearer(notoken_type=mtls_pop, noreq_cnf); forces the x5c chain on theclient_assertion(regardless ofapp.sendX5c()) so ESTS can do SN/I subject+issuer matching over the mTLS channel; produces a plain (not thumbprint-fenced) Bearer cache entry..mtlsProofOfPossession()always takes precedence over the app-level flag.Tests
BearerOverMtlsTest, 13 cells viamockConstruction(DefaultHttpClient)): config (default / stored / non-cert fail-fast); client credentials (globalmtlsauth+ Bearer +client_assertionwith forced x5c and nomtls_pop/req_cnf; regional; per-requestmtls_popwins; plain-Bearer cache); user flows (OBO / refresh / auth-code route tomtlsauthwith the correct grant + forced-x5c assertion; OBO 2nd call served from cache; OBO without the flag uses the regular login endpoint).MtlsPopIT): live client-credentials Bearer-over-mTLS cell + cache-hit (SN/I-allow-listed app); a skip-gated OBO cell documenting the user-flow allow-listing gap (AADSTS700027 / AADSTS392189). The token-endpoint wire contract is asserted at the unit level because the mTLSDefaultHttpClientis constructed internally (no factory injection point).PR topology
Child branch off
rginsburg/sni-mtls-pop(#1040), targeting the SNI branch — a sibling to the FIC follow-up #1041, not stacked on it. Retarget todevafter #1040 merges. Does not touch the FIC test or its skip-hatch.Draft until #1040 merges to
dev.