Skip to content

Complete mTLS E2E parity and combined transport integration - #976

Open
Robbie-Microsoft wants to merge 72 commits into
devfrom
robbie-microsoft-comprehensive-mtls-e2e-parity
Open

Robbie-Microsoft wants to merge 72 commits into
devfrom
robbie-microsoft-comprehensive-mtls-e2e-parity

Conversation

@Robbie-Microsoft

@Robbie-Microsoft Robbie-Microsoft commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Scope and dependencies

Combined-head follow-up for #938, #939, #947, and #974. Do not merge before those feature dependencies land. The initial diff against dev necessarily includes their inherited production changes. Their exact approved heads were merged only into this branch; the original four branches/PRs remain untouched. These are ordinary dependencies, not a native GitHub stack.

All new parity tests, fixtures, and test documentation stay in this PR. No redistribution is authorized. After the dependencies land, a later rebase onto dev can remove inherited feature changes from the diff; that operation is not performed here.

Changes and explicit approvals

  • Inventory 21 mandatory live certificate/FIC methods: global Graph PoP success and three same-token rejection controls, regional Key Vault PoP, both live cache orders, Bearer-over-mTLS, and global/regional two-leg FIC.
  • Record real default/custom certificate transport without replacing network responses. OBO requires HTTP 400 / invalid_request / AADSTS50027; refresh/auth-code captures require HTTP 400 / invalid_grant. Endpoint, signed assertion/type, grant-specific fields, and flag-off controls remain mandatory.
  • Exercise actual create_mtls_client(binding_context) pipelines for successful client credentials and supported PoP/FIC combinations, preserving middleware and caller-owned cleanup. Offline guards and loopback evidence are separate from live results.
  • Include the explicitly user-approved narrow transport integration: share the existing certificate-scoped factory and endpoint protections with Bearer-over-mTLS while preserving PoP and FIC jwt-pop. This is E2E parity plus necessary transport integration and approved review fixes, not tests-only.
  • Apply separately approved safety fixes to the two inherited samples: static/boolean diagnostics instead of dynamic token-result logging, with output regression tests. Set an explicit TLS 1.2 minimum in the loopback fixture. No scanner gate is suppressed.
  • Hide token-bearing test-helper traceback frames for cache, FIC, Graph, and recorder failures. Offline regressions render actual pytest tracebacks with argument display enabled and check complete tokens and truncated fragments. Live assertions and transport behavior are unchanged.
  • Apply the separately user-approved cache-write scaling fix in this PR: a private, transient identity-to-storage-location index replaces per-write scans. Reconcile once when the cache or either AT bucket is replaced; maintain locations on writes/removals, including ordinary Bearer/SSH entries sharing those buckets. Preserve native legacy renewal, certificate/scheme case semantics, released SSH layout, and non-reentrant persistence hooks. No public API or persisted-format redesign is added.

The combined baseline is d9f87b1; transport integration/regressions are isolated in 6b0694c; parity tests/docs in f5435b4; approved sample/fixture safety fixes in fea35cf; evidence-based test corrections in 7f98208; test-only traceback fixes in 1208c07; and the separately approved cache fix/regressions in 0cd290d. The privacy and cache fixes were implemented together without waiting for intermediate-head CI; the final combined head below is the completion gate.

Verified final-head evidence

Exact pushed and tested head: 0cd290d14ed194a493bf2f3653cf795880bcdc10.

Official 1709627 and SDL 1709626 both succeeded with that exact sourceVersion. All six unit jobs passed 594 tests each. All 12 selected privacy/cache/reload/persistence regression methods also passed individually on all six versions: 72 regression executions. Every required live method was verified individually against the exact inventory: 126 passed executions, zero required skips.

Python E2E test-run ID Required live cases
3.9 6627761 21 passed
3.10 6627757 21 passed
3.11 6627767 21 passed
3.12 6627764 21 passed
3.13 6627766 21 passed
3.14 6627765 21 passed

Final-head CodeQL aggregate succeeded with zero annotations. Packaging, CLA, both analyzers, and all reported final-head checks are green. Unrelated legacy skips remain outside the required live inventory.

Combined local cache/application/recorder/lab/transport coverage passed 327 tests / 254 subtests. Before the fixes, actual pytest rendering reproduced the privacy leaks and deterministic counters reproduced quadratic writes. After one-time reconciliation, 60 mixed inserts/renewals use 90 identity operations and zero bucket visits at 0/100/1,000/10,000 cached entries. Fresh 100/200/400-entry populations use 100/200/400 identity operations and zero visits. Existing compatibility, reload, persistence, and removal tests remain, with additional bucket/slot replacement coverage. The index adds only transient in-memory metadata; serialization remains unchanged.

The pinned reference-to-Python mapping enumerates all 19 applicable active .NET/Go cases and their assertions. Resource PoP JWT/cnf checks are unconditional. Reference FIC exchange assertions are explicitly opaque: requested scope, PoP type, certificate continuity, exact unchanged jwt-pop handoff, and successful real redemption remain required. Audience allowlists contain only verified URI/application-ID aliases. Service-representation and precise OBO expectations were corrected without accepting arbitrary OAuth/network failures.

Explicit exclusions

IMDSv2; non-exportable/VBS/provider-backed keys; managed identity; new SDK public APIs/features or lab provisioning; intentionally disabled reference successful user acquisitions; .NET-only callback options without a Python equivalent; and unrelated legacy E2E skips. This does not claim repository-wide skip elimination.

Robbie-Microsoft and others added 30 commits July 7, 2026 18:26
Allow a confidential-client app configured with a Subject Name + Issuer
(SN/I) certificate to obtain an mTLS-bound PoP access token from Entra ID,
using the same certificate as the client TLS certificate in the mutual-TLS
handshake to the token endpoint (token_type=mtls_pop, cnf/x5t#S256 binding).

- Add mtls_proof_of_possession kwarg to acquire_token_for_client, returning
  a binding_certificate (public x5c + sha256 thumbprint) on success
- Add mTLS client-cert transport (msal/mtls.py) with endpoint transform and
  sovereign-cloud / tenanted-authority / custom-http-client guardrails
- Isolate mtls_pop tokens in cache via key_id (Bearer unchanged)
- Add token-type telemetry, docs, and a confidential-client mTLS PoP sample

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The two SN/I-over-mTLS-PoP e2e tests used the generic lab app
(LAB_APP_CLIENT_ID) with the microsoft.onmicrosoft.com authority. That
combination works for Bearer SN/I but is not ESTS allow-listed for mTLS
PoP, so the token request fails with AADSTS700025. Point both tests at
the SN/I-allow-listed app 163ffef9-a313-45b4-ab2f-c7e2f5e0e23e in the
MSI-team tenant, mirroring the MSAL .NET/Java e2e config. The lab SN/I
cert and the MS Graph resource are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Gate the ext_cache_key and key_id cache-isolation filters in
TokenCache.search() on a non-empty target. Scoped token retrieval keeps
the mtls_pop / FMI isolation intact, but broad target-less searches (used
by _sign_out and remove_account) now enumerate key-bound access tokens so
they are removed from the cache instead of being left behind.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ion build

Harden the mTLS Proof-of-Possession path with three fixes surfaced by review.

Fail closed on a token_type downgrade. A cert-bound request is driven by our
request flag, not the response, so if ESTS answers an mtls_pop request with a
non-cert-bound token we previously still attached binding_certificate and cached
it as bound. Now acquire_token_for_client returns token_type_mismatch when the
response token_type is not "mtls_pop", and _MtlsClient only re-injects key_id
(the cache binding) when the response is actually mtls_pop. A downgraded token
therefore caches as an ordinary token that the cert-bound silent query can never
match, so the flow re-fetches and fails closed every time. Mirrors MSAL .NET/Go.

Honor verify in the mTLS transport. A custom ssl_context bypasses requests' own
verify handling, so verify was silently ignored: verify=False raised a cryptic
ValueError and the default used the system store instead of certifi. The context
builder now matches requests' semantics (certifi default, CA file/dir path, or
disabled) so verify behaves as documented.

Serialize first session build. Wrap the lazy session construction in a
double-checked lock so a cold-start burst on a multi-threaded confidential
client no longer builds N transports (each writing the private key to a temp
file) and orphans all but one.

Also drop stray blank lines that had crept into the acquire_token_for_client
mTLS setup block.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two review fixes for the mTLS PoP work.

Cache isolation (empty-scopes leak): the key_id and ext_cache_key
isolation filters in TokenCache.search() were gated on `target`, so an
empty-scopes for-use lookup (e.g. acquire_token_for_client([])) was
target-less and bypassed isolation -- returning a key-bound (mtls_pop)
or FMI-bound AT the caller never asked for. Decouple "removal intent"
from "target-less" via a keyword-only for_removal flag: isolation is now
unconditional for for-use lookups and only the two removal callers
(_sign_out, remove_tokens_for_client) opt out with for_removal=True to
enumerate and delete those ATs. Mirrors MSAL .NET, where token-type
isolation is never gated on scope state.

mTLS transport verify=None: _MtlsHttpClient forced CERT_NONE onto a
verifying ssl_context when verify was None, raising ValueError. Normalize
None to True (the safe default for a security library, verifying via
certifi) so session.verify and the ssl_context agree.

Adds regression tests for both empty-scope leak paths (key-bound and
ext-bound) and updates the removal-enumeration test to opt in via
for_removal=True.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Four small, low-risk fixes from the code review:

- application.py `_private_key_to_unencrypted_pem`: pass an explicit
  `backend=default_backend()` to `load_pem_private_key` (required on the
  supported `cryptography` floor, 2.5) and wrap the call so a bad or
  encrypted-without-passphrase key surfaces a clear, actionable ValueError
  instead of a cryptic low-level error. Mirrors the sibling
  `_load_private_key_from_pem_str`.

- token_cache.py: coerce `key_id` to a str via a new `_key_id_to_str`
  helper before building the AccessToken cache key, so a non-str key_id
  never raises TypeError at the `"-" + key_id` concatenation. The normal
  ASCII-str path is byte-identical, so existing mtls_pop cache entries are
  unaffected. Defensive only: current callers already pass an ASCII str.

- test_application.py: hoist `import os` to the top of the module and drop
  the mid-file `import os as _os` alias.

Adds regression tests: bytes/other key_id coercion (test_token_cache.py)
and the clearer private-key load errors (test_mtls_transport.py).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Stacked on top of the SN/I mTLS PoP base support. Enables a Federated
Identity Credential (FIC) two-leg exchange over mutual-TLS: a leg-1
cert-bound mtls_pop token is presented as a jwt-pop client assertion over
an mTLS connection bound by the same certificate to obtain the final token.

- Detect the client_credential mtls_binding_certificate sub-key (FIC leg 2)
  and carry the leg-1 assertion as a jwt-pop client assertion over mTLS
- Route every FIC leg-2 request over the mTLS endpoint, so even a Bearer
  token travels the cert-bound connection (implicit Bearer-over-mTLS)
- Add unit + E2E coverage, docs, and a two-leg sample

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A confidential client configured for a Federated Identity Credential (FIC)
leg-2 exchange (a client_assertion plus an mtls_binding_certificate) is only
meaningful through acquire_token_for_client, which presents the binding
certificate over mutual-TLS and carries the leg-1 assertion as jwt-pop. If such
an app is instead used with any other flow (authorization code, on-behalf-of,
refresh token, username/password, or user-FIC), MSAL would send that cert-bound
leg-1 assertion as an ordinary jwt-bearer over a non-mTLS connection. ESTS
rejects it, but with a confusing server error.

Add a small _reject_if_mtls_binding_cert guard and call it at the top of those
flows so the misuse fails fast with a clear ValueError that points the caller to
acquire_token_for_client. The guard is a no-op for every non-FIC-leg-2 app, so
public clients and ordinary confidential clients are unaffected.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The _reject_if_mtls_binding_cert guard already covered the six
non-client-credential acquisition flows, but not acquire_token_silent /
acquire_token_silent_with_error. With a shared token cache holding user
refresh tokens, a FIC leg-2 app (client_assertion + mtls_binding_certificate)
could silently redeem one of those RTs, putting the cert-bound leg-1
assertion on a non-mTLS jwt-bearer request -- which ESTS rejects with a
confusing invalid_client rather than a clear message.

Add the guard to both public silent entry points. Both call the private
_acquire_token_silent_with_error directly (not each other), and
acquire_token_for_client also uses that private helper, so the guard lives
on the two public wrappers -- never on the private method -- to keep the
legitimate mTLS client-credential flow working. No-op for normal apps.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Port the msal-dotnet PR #6100 rigor to the SN/I-over-mTLS-PoP e2e tests:

- Add a reusable _call_graph_with_mtls_pop_token(token, cert) helper that
  presents the acquired token to the mTLS Graph host
  (mtlstb.graph.microsoft.com) over a TLS channel bound by the SAME
  certificate and asserts HTTP 200. Acquiring a token is not proof it works;
  a 401/403 (binding lost / wrong scheme) is a regression. The helper reuses
  msal.mtls._create_ssl_context/_make_mtls_adapter and is shared with the FIC
  e2e (follow-up PR).
- Rename the two PoP tests to the cross-SDK Credential_<X509|Fic>_Output_<Pop|
  Bearer> matrix (snake_case): test_credential_x509_output_pop (+_regional),
  both now call the resource-call helper.
- Add test_credential_x509_output_bearer: the same SN/I cert WITHOUT the flag
  must still authenticate via a signed client_assertion and yield an ordinary
  token (no binding_certificate) - the client_assertion is dropped only on the
  mTLS path.
- Regional test: skip only on temporarily_unavailable. invalid_request is a
  400 config error (a real regression), so it now fails loudly instead of
  being masked as "endpoint not reachable".
- Docs/sample/code comments: azure_region is optional (global mtlsauth
  fallback); clarify the classic cert path still signs a private_key_jwt
  client_assertion while only mTLS PoP omits it; tighten the sample's
  log-safety note (binding_certificate x5c is public, not a secret).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per cross-SDK review feedback: make the Bearer contrast explicit. On the
non-mTLS path MSAL Python omits binding_certificate entirely, so assert both
that the key is absent and that a lookup yields None - proving the cert
binding is genuinely gone, not merely that the mtls_proof_of_possession flag
was skipped.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per cross-SDK review: document that the FIC two-leg e2e (PR #939) reuses this
helper as-is, so it is not inlined later. Explains why it takes an explicit
cert_credential instead of reading self.app's cert.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Align with the region-split rule shared across the sibling SDKs (merged in
MSAL .NET, matched by Java/JS/Go): the regional slice intermittently
downgrades an mtls_pop request to Bearer by returning a Bearer *token* (not a
temporarily_unavailable error), so a regional PoP cell either fails its
token_type assert or forces us to mask a real downgrade - both wrong.

- DELETE test_credential_x509_output_pop_regional. The {region}.mtlsauth host
  rewrite stays covered by the mtls transport/endpoint unit tests, so no unit
  coverage is lost.
- Make test_credential_x509_output_bearer REGIONAL (azure_region="westus3").
  A Bearer token_type is deterministic, so this safely RETAINS live
  regional-endpoint E2E coverage. Comment explains region is correct here but
  forbidden on the PoP cell.
- test_credential_x509_output_pop stays GLOBAL (Graph scope, resource-call
  200, cache-hit) - unchanged.

Result mirrors MSAL .NET: pop (global) / bearer (regional westus3).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bring the Task-1 SNI hardening from rginsburg/sni-mtls-pop (tip b9233c6)
into the FIC follow-up branch: the reusable _call_graph_with_mtls_pop_token
resource-call e2e helper, the cross-SDK naming matrix, the loud-fail on
invalid_request, and the region split (drop the regional mtls_pop cell that
intermittently downgrades to Bearer; make the Bearer cell regional westus3;
keep the pop cell global).

Conflicts resolved by keeping both sides:
- docs/index.rst: retain the SNI note that only the mTLS-PoP path drops the
  signed client_assertion AND the FIC leg-2 configuration note.
- msal/application.py: retain the FIC leg-2 jwt-pop assertion branch AND fold
  the SNI clarifying comment into the vanilla SN/I else path.

The FIC e2e test and its leg-1 skip-hatch are left untouched; porting the
resource-call helper into the FIC e2e is Task 2.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bring the SNI mTLS PoP branch current with base dev (which had advanced with
PRs #911, #937, #940, #942), resolving the acquire_token_for_client overlap
between this branch's mtls_proof_of_possession feature and dev's new
forwarded_client_claims parameter (#937).

Conflicts resolved by integrating BOTH sides:
- msal/application.py acquire_token_for_client(): keep both the
  mtls_proof_of_possession and forwarded_client_claims parameters, both request
  setup blocks, and the fail-closed mtls_pop downgrade/binding_certificate
  post-processing (result = _clean_up(...) form).
- msal/application.py _acquire_token_for_client(): keep the is_mtls_pop client
  selection AND the client_claims merge into the OAuth claims parameter; the
  single popped request_data carries token_type/key_id and client_claims on the
  wire.
- tests/test_token_cache.py: union the imports (_key_id_to_str from this branch
  plus _parse_claims_or_raise/_merge_claims from dev).

No Task-1 SNI hardening dropped: the mTLS PoP feature, the
test_credential_x509_output_{pop,bearer} matrix, the region split (pop GLOBAL /
bearer REGIONAL westus3, no regional pop cell), the _call_graph helper, and the
downgrade fail-closed all remain. No escape hatch reintroduced.

Verified: py_compile clean; pytest collects 196 tests (e2e matrix + upstream
suites); 17 mtls_pop + forwarded_client_claims unit tests pass; 49 token_cache
tests pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bring the FIC branch current with base dev by merging the resync'd SNI branch
(rginsburg/sni-mtls-pop @ 4ec592f, which merged origin/dev), so #939 also
carries PRs #911, #937, #940, #942.

Single conflict in msal/application.py acquire_token_for_client(), resolved by
integrating BOTH sides: keep the FIC-aware mTLS condition
(mtls_proof_of_possession or self._mtls_is_fic_leg2) and its comment from this
branch, AND add dev's forwarded_client_claims request-setup block above it. The
private _acquire_token_for_client() auto-merged correctly, combining the
FIC/mtls client selection (is_mtls_pop or self._mtls_is_fic_leg2) with dev's
client_claims -> OAuth claims merge.

The FIC e2e test test_fic_two_leg_over_mtls_pop and its leg-1 skip-hatch are
left UNTOUCHED (de-hatch + resource-call port + Credential_Fic_Output_* naming
remain Task 2). Region split, x509 pop/bearer matrix, and downgrade fail-closed
are all preserved.

Verified: py_compile clean; e2e matrix (pop/bearer) + FIC test collect; 22
mtls_pop + forwarded_client_claims unit tests pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Introduce an app-level opt-in `send_certificate_over_mtls` on the
confidential-client `client_credential` dict. When set, MSAL presents the
SN/I certificate on the TLS handshake to the [region.]mtlsauth.* endpoint and
signs a private_key_jwt client_assertion (x5c forced), but requests a plain
Bearer token (no token_type=mtls_pop / no key_id / no req_cnf). The result is
an ordinary, non-cert-bound Bearer access token.

- Validate the flag at construction: it requires a certificate credential,
  else ValueError naming the flag.
- Add _get_mtls_bearer_client(): reuses the Task-1 mTLS transport
  (msal/mtls.py) + the login.*->[region.]mtlsauth.* endpoint transform, adds a
  signed x5c-forced assertion, and caches a plain (non-key_id-fenced) token.
- Route acquire_token_for_client() to it when the flag is set and the request
  is not mtls_pop. A per-request mtls_proof_of_possession=True always wins.
- Tests (RED-first) in TestSendCertificateOverMtls: config/builder (default
  false, stored true, flag + non-cert credential raises) and CC behavior
  (global/regional mtlsauth endpoint, client_assertion + jwt-bearer, x5c
  forced, no PoP wire markers, plain-Bearer result, mtls_pop precedence).

Ports MSAL.NET CertificateOptions.SendCertificateOverMtls (client-credentials
leg). OBO/refresh/auth-code coverage follows.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extend the app-level send_certificate_over_mtls opt-in beyond
client-credentials to every confidential-client user flow: on-behalf-of,
refresh-token, and authorization-code.

Add _client_for_confidential_request(): when the flag is set it returns the
Bearer-over-mTLS client (SN/I cert on the TLS handshake, routed to the
[region.]mtlsauth.microsoft.com endpoint, private_key_jwt client_assertion
with the x5c chain forced, plain Bearer token back), otherwise the regular
client. Swap it in at the three user-flow call sites. Client-credentials
routing stays in _acquire_token_for_client, where a per-request
mtls_proof_of_possession still wins over the app-level flag.

Tests (tests/test_application.py, TestSendCertificateOverMtls): OBO / refresh
/ auth-code with the flag hit the global mtlsauth endpoint, carry
client_assertion + client_assertion_type=jwt-bearer with no key_id/req_cnf,
and use the correct grant (on_behalf_of / refresh_token / authorization_code);
OBO with the flag + a configured region hits the regional mtlsauth host; OBO
without the flag stays on the regular login endpoint (negative control).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Guard the .NET MtlsBearerUserFlowTests second-call regression, adapted to
msal-python's cache seam: a 2nd Bearer-over-mTLS client-credentials call is
served from cache with no 2nd network round-trip and no crash on
region/instance-metadata resolution, returning the cached plain Bearer AT
(token_type=Bearer, NOT cert-bound).

In .NET the 2nd call crashes because the AT is cached under the mtlsauth.*
host and instance-metadata discovery rejects that host. msal-python
structurally avoids this by decoupling the cache/metadata authority (login.*,
where the plain Bearer AT lives) from the transport endpoint (mtlsauth.*): the
2nd lookup resolves login.* -- the normal, always-safe path -- so the cached
Bearer AT is returned without feeding the mtlsauth.* host into discovery.

Only client-credentials auto-serves from cache in msal-python (OBO, refresh,
and auth-code have no silent-first lookup), so CC is the flow that exercises
this regression.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover the send_certificate_over_mtls feature in tests/test_e2e.py, mirroring
MSAL .NET's live + capture split and reusing the SNI allow-listed app/authority
constants already in PublicCloudScenariosTestCase.

- test_credential_x509_send_certificate_over_mtls_output_bearer: LIVE
  client-credentials acquisition (allow-listed app, westus3, Key Vault resource)
  that presents the SN/I cert on the TLS handshake to the mTLS endpoint and
  asserts a PLAIN Bearer token back (token_type=Bearer, binding_certificate
  absent) plus a normal flagless cache hit -- the caller-visible proof that the
  AT is an ordinary Bearer entry, not a key-id-fenced mtls_pop entry.
- test_send_certificate_over_mtls_user_flows_capture_request: request-capture
  for OBO, refresh-token, and auth-code over the real lab SN/I certificate --
  asserts each builds the mtlsauth endpoint + a private_key_jwt client_assertion
  + the flow's own grant with none of the mtls_pop markers, without needing
  those (non-mTLS-enabled) apps allow-listed.
- test_send_certificate_over_mtls_user_flows_live_acquire: skip-gated live
  OBO/refresh placeholder documenting the pending server-side app
  mTLS-enablement (AADSTS700027 / AADSTS392189), mirroring MSAL .NET's [Ignore].

All three are gated (lab cert / explicit skip), collect cleanly, and skip
locally; the live and capture cells run in the lab pipeline.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a docstring admonition to the client_credential parameter describing the
Bearer-over-mTLS opt-in: set send_certificate_over_mtls=True on the certificate
credential dictionary to present the cert on the mTLS handshake and receive a
plain (non-cert-bound) Bearer token. Notes that it covers all four confidential
flows, defaults to False, requires a certificate credential, and that a
per-request mtls_proof_of_possession=True always takes precedence.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a negative assertion to test_cc_second_call_is_bearer_cache_hit that the
cached plain Bearer AT is stored under the ORIGINAL login.* host and its
environment never contains "mtlsauth". This is the assertion that would fail if
the cache seam were ever switched to the .NET-style mtlsauth.* environment
(Option A), which would silently break msal-python's login.*-keyed cache lookup
and defeat the 2nd-call cache hit. Also assert result1 == result2 (the same
cached AT is returned) alongside the existing exactly-one-network-POST guard.

Mirrors the cross-SDK finding that a 2nd-call cache test can pass without
actually proving the cache environment; this makes the env dependence explicit
and deterministic.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

# Conflicts:
#	msal/sku.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Robbie-Microsoft and others added 12 commits October 23, 2026 15:15
Adapt FIC scenarios to the shared certificate configuration and preserve global PoP and regional Bearer parity.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Run all five required certificate scenarios on tagged releases and verify missing credentials are errors, not skips. Preserve the supported live and request-capture coverage boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Retain the FIC-aware response validator and user-flow guard alongside the Bearer-over-mTLS client. Preserve both feature documentation and sample entry points.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve FIC jwt-pop assertions and response validation inside the certificate-scoped factory path. Retain both Bearer and PoP state and guards; use the renamed custom-client state in the inherited Bearer path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Apply the approved sibling-feature integration: Bearer-over-mTLS uses the same certificate-scoped custom factory and endpoint protections as PoP and FIC. Retain one owned pipeline per application, jwt-pop FIC assertions, cache isolation, and caller cleanup ownership.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add strict Graph controls, regional PoP, both live cache orders, real default/custom grant-rejection recording, and two-leg FIC factory evidence. Inventory the 21 mandatory live methods and map pinned .NET/Go assertions in the test setup documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 21:10
@Robbie-Microsoft
Robbie-Microsoft requested a review from a team as a code owner October 23, 2026 21:10
Comment thread sample/confidential_client_mtls_http_client_sample.py Fixed
Comment thread sample/confidential_client_mtls_pop_sample.py Fixed
Comment thread tests/test_mtls_http_client.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Bound-token writes scan the full shared cache under lock, causing quadratic cache population and slower renewals.

1 open issue
What changed in this PR

Completes mTLS end-to-end parity coverage and integrates certificate-scoped transport across PoP, Bearer-over-mTLS, and two-leg FIC flows. The combined diff includes inherited feature changes; dependency merges and credentialed validation remain pending.

Changes:

  • Adds 21 required live certificate/FIC scenarios, transport recording, and offline failure guards.
  • Shares protected certificate-aware pipelines while preserving token binding and cache isolation.
  • Expands transport, cache compatibility, samples, and setup documentation.
File Description
tests/​test_token_cache.py Tests binding isolation, migration, and released-cache compatibility.
tests/​test_optional_thumbprint.py Updates certificate mock return values.
tests/​test_mtls_transport.py Tests endpoints, certificate chains, and TLS settings.
tests/​test_mtls_http_client.py Tests custom factories, isolation, and loopback TLS.
tests/​test_mtls_e2e_helpers.py Validates recording and scenario failure guards.
tests/​test_lab_config.py Checks credentials and required live-test inventory.
tests/​test_e2e.py Adds required live certificate and FIC scenarios.
tests/​README.md Documents setup, required coverage, and pinned parity mapping.
tests/​mtls_e2e_helpers.py Adds real-transport recorders and certificate fixtures.
tests/​lab_config.py Validates required PFX credentials.
sample/​confidential_client_mtls_pop_sample.py Demonstrates PoP, Bearer-over-mTLS, and FIC.
sample/​confidential_client_mtls_http_client_sample.py Demonstrates caller-owned certificate-aware pipelines.
msal/​token_cache.py Separates mTLS tokens and migrates bound-token layouts.
msal/​telemetry.py Adds mTLS token-type telemetry.
msal/​oauth2cli/​oauth2.py Adds the jwt-pop assertion type.
msal/​mtls.py Implements binding contexts and protected TLS transport.
msal/​__init__.py Exports MtlsBindingContext.
docs/​index.rst Documents mTLS usage and custom transport contracts.
azure-pipelines.yml Adds PR triggering for the mTLS base branch.
.Pipelines/​template-pipeline-stages.yml Updates comments about missing-credential failures.
.Pipelines/​CI-AND-RELEASE-PIPELINES.md Documents revised E2E failure behavior.

Review effort: Balanced

Comment thread msal/token_cache.py Outdated
Robbie-Microsoft and others added 2 commits October 23, 2026 17:32
Apply the explicitly approved sample logging safety fixes without emitting dynamic token-result fields. Add output guards and require TLS 1.2 or newer in the loopback fixture; retain the CodeQL gate unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require the observed OBO invalid_request/AADSTS50027 rejection. Accept verified resource application-ID audience aliases and treat exchange-scope FIC assertions as opaque as the pinned references do, preserving strict resource JWT binding and real redemption assertions. Guard sensitive helper arguments from pytest tracebacks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 21:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Live-test failure tracebacks can expose token material through unhidden helper arguments.

2 open issues

Review effort: Balanced

Comment thread tests/test_e2e.py
Cover cache, FIC, Graph, recorder POST and recorder setup failures with actual pytest argument rendering, including truncated-token checks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Explicit cert=None is validated inconsistently, making acquisition behavior depend on cache state and transport choice.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Reject explicit cert=None before cache lookup

msal/​mtls.py:298

An explicit cert=None passes this pre-cache check, but _MtlsHttpClient._request() rejects any cert keyword. The same PoP/FIC acquisition can therefore succeed from cache and raise ValueError after the token expires, while a custom pipeline may accept it. Check keyword presence here to match the owned transport, and add cert=None to the pre-cache validation regression cases.

1 resolved since last review

Review effort: Balanced

Reconcile storage locations once per loaded cache or replaced bucket, maintain the transient index on writes and removals, and preserve legacy renewal and serialized formats.

Add deterministic bucket-visit and identity-operation regressions, retain compatibility and persistence coverage, and exercise reload and shared SSH-slot replacement.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 22:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Sensitive values can still appear in failure tracebacks, and the cache-runtime optimization needs scope, approval, and verification reconciliation.

1 open issue
3 previously missed issues

In code that hasn't changed since last review

Medium severity Prevent sensitive result leakage in FIC callback traceback

sample/​confidential_client_mtls_pop_sample.py:181

The FIC leg-1 failure path still includes an unchecked error value in an uncaught exception, so a sensitive result value can appear in the traceback despite the safe summary output. Use a static exception message here and extend the sample-output regression to cover this callback failure.

Medium severity Suppress credential exposure in default-recorder failures

tests/​mtls_e2e_helpers.py:163

When default-recorder validation raises inside Session.send, the enclosing Requests frames still hold raw form arguments containing the client assertion and possibly a refresh token. Hiding the helper frames does not prevent argument-enabled pytest tracebacks from exposing those credentials. Report validation failures after the HTTP stack unwinds, without the original traceback, and add a default-recorder failure regression.

Medium severity Suppress access-token exposure in transport failure tracebacks

tests/​test_e2e.py:539

A Graph connection, TLS, or timeout failure propagates through Requests frames whose headers argument contains the access token; this helper's __tracebackhide__ does not hide those frames. Catch RequestException and raise a static failure with suppressed exception chaining, and add transport-failure traceback coverage. The same issue affects the rejection-control request at tests/test_e2e.py:536–539.

1 resolved since last review

Review effort: Balanced

Comment thread msal/token_cache.py

This branch has not been deployed

No deployments
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.

3 participants