Merge SNI mTLS PoP hardening (#938) into FIC branch

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>

Author: Robbie-Microsoft

docs/index.rst

@@ -198,10 +198,16 @@ Notes and requirements:
 * The ``authority`` must be **tenanted** (not ``/common`` or ``/organizations``).
 * MSAL must own the TLS transport, so a custom ``http_client`` is not supported
   together with ``mtls_proof_of_possession=True``.
+* ``azure_region`` is **optional**: omit it to use the global mTLS endpoint
+  (``mtlsauth.microsoft.com``); set it (e.g. ``westus3``) only to pin a region.
 * The existing SN/I + Bearer (client-assertion) flow is unchanged; the same
   certificate can be used either as an assertion signer (Bearer) or as the TLS
   client certificate (mtls_pop).
 * mTLS PoP currently targets the public and Azure Government (Arlington) clouds.
+* Only the mTLS-PoP path drops the signed ``client_assertion``: the certificate
+  authenticates the client through the mutual-TLS handshake instead. The classic
+  (non-mTLS) certificate path is unchanged and still sends a signed
+  ``client_assertion`` (``private_key_jwt``) to obtain an ordinary token.
 * For a Federated Identity Credential (FIC) exchange, configure the leg-2 client
   with both a ``client_assertion`` (the leg-1 token) and an
   ``mtls_binding_certificate`` sub-key; the leg-1 assertion is then sent as a

msal/application.py

@@ -1104,7 +1104,11 @@ def _get_mtls_client(self, central_authority):
         if self._mtls_is_fic_leg2:  # FIC leg 2: assertion carried as jwt-pop
             client_assertion = self.client_credential["client_assertion"]
             client_assertion_type = Client.CLIENT_ASSERTION_TYPE_JWT_POP
-        else:  # Vanilla SN/I: the TLS certificate alone authenticates the client
+        else:
+            # Vanilla SN/I: the TLS certificate alone authenticates the client, so
+            # this mTLS path sends NO signed client_assertion. (The classic, non-mTLS
+            # certificate path still signs a private_key_jwt client_assertion; only
+            # mTLS PoP replaces that assertion with the mutual-TLS handshake.)
             client_assertion = None
             client_assertion_type = None
         client = _MtlsClient(

sample/confidential_client_mtls_pop_sample.py

@@ -53,11 +53,12 @@ def _build_cert_credential():
 def _print_result_summary(result, header):
     """Print a *log-safe* summary of an MSAL token ``result``.
 
-    We deliberately never log the ``result`` dict, nor any value read out of it,
-    because on success it carries an ``access_token`` (and, for mTLS PoP, binding
-    material). Logging token responses is exactly the habit a security library
-    should not model. Instead we surface only booleans derived via comparison,
-    which reveal the *shape* of the response without leaking its contents.
+    The sensitive part of ``result`` is the ``access_token`` (a PoP/bearer
+    secret) - that is what we must never log. The ``binding_certificate`` is the
+    certificate's PUBLIC material (x5c + thumbprint), not a secret, so it is safe
+    to inspect. We still avoid dumping the whole ``result`` dict and instead
+    surface only a few derived booleans/fields, to model the habit of not
+    routinely logging token responses.
 
     On success the interesting (non-sensitive) fields are::
 

tests/test_e2e.py

@@ -454,6 +454,11 @@ def test_subject_name_issuer_authentication(self):
     # rejected for mTLS PoP with AADSTS700025. Mirrors the MSAL .NET/Java e2e config.
     _SNI_ALLOWLISTED_CLIENT_ID = "163ffef9-a313-45b4-ab2f-c7e2f5e0e23e"
     _SNI_ALLOWLISTED_AUTHORITY = "https://login.microsoftonline.com/bea21ebe-8b64-4d06-9f6d-6a889b120a7c"
+    # The mTLS Graph host (NOT plain graph.microsoft.com): only this host performs
+    # the client-certificate TLS handshake that an mtls_pop token's binding is
+    # validated against. Presenting the token here, with the binding cert on the
+    # handshake, must return 200; a 401/403 means the binding was lost.
+    _MTLS_GRAPH_RESOURCE = "https://mtlstb.graph.microsoft.com/v1.0/applications?$top=1"
 
     def _assert_pop_cache_hit(self, app, scope):
         # A second mTLS-PoP call for the same scope must be served from cache.
@@ -472,14 +477,52 @@ def _cnf_x5t_s256(access_token):
         except Exception:  # Opaque/non-JWT token - skip the offline binding check
             return None
 
-    def test_sni_mtls_pop_for_client(self):
+    def _call_graph_with_mtls_pop_token(self, token, cert_credential):
+        """Prove an ``mtls_pop`` token is genuinely usable: present it to the
+        mTLS Graph endpoint over a TLS channel bound by the SAME certificate and
+        require HTTP 200. Acquiring the token is not enough - a resource only
+        honors it when the binding certificate is on the TLS handshake and the
+        auth scheme is ``mtls_pop``; a 401/403 means the binding was lost, i.e. a
+        real regression rather than a transient failure.
+
+        Shared test infrastructure: the FIC two-leg e2e in PR #939 reuses this
+        as-is (please do not inline it), which is why it takes an explicit
+        ``cert_credential`` rather than reading ``self.app``'s certificate.
+        """
+        from msal.application import _load_mtls_cert_material
+        from msal.mtls import _create_ssl_context, _make_mtls_adapter
+        material = _load_mtls_cert_material(cert_credential)
+        session = requests.Session()
+        session.mount("https://", _make_mtls_adapter(_create_ssl_context(
+            material["cert_pem"], material["private_key_pem"])))
+        try:
+            resp = session.get(
+                self._MTLS_GRAPH_RESOURCE,
+                headers={"Authorization": "mtls_pop " + token},
+                timeout=30)
+        finally:
+            session.close()
+        self.assertEqual(
+            200, resp.status_code,
+            "mtls_pop token rejected by the resource (HTTP %s). A 401/403 means "
+            "the binding certificate was not presented on the TLS handshake, or "
+            "the 'mtls_pop' auth scheme was wrong. Body[:500]=%s" % (
+                resp.status_code, (resp.text or "")[:500]))
+        return resp
+
+    # ── Credential x Output matrix (mirrors the MSAL .NET e2e naming) ──────────
+    # Credential_<X509|Fic>_Output_<Pop|Bearer>. Task 1 covers the two X509 cells
+    # (this file); the Fic cells live in the FIC follow-up PR. snake_case per
+    # unittest, but the matrix names line up 1:1 with the sibling SDKs.
+    def test_credential_x509_output_pop(self):
         from tests.lab_config import get_client_certificate
         if not clean_env("LAB_APP_CLIENT_CERT_PFX_PATH"):
             self.skipTest("LAB_APP_CLIENT_CERT_PFX_PATH not set")
+        cert = get_client_certificate()
         self.app = msal.ConfidentialClientApplication(
             self._SNI_ALLOWLISTED_CLIENT_ID,
             authority=self._SNI_ALLOWLISTED_AUTHORITY,
-            client_credential=get_client_certificate())
+            client_credential=cert)
         result = self.app.acquire_token_for_client(
             self._MTLS_POP_SCOPE, mtls_proof_of_possession=True)
         self.assertIn("access_token", result, "mTLS PoP request failed: %s" % result)
@@ -490,23 +533,39 @@ def test_sni_mtls_pop_for_client(self):
         cnf = self._cnf_x5t_s256(result["access_token"])
         if cnf:  # When the AT is a decodable JWT, its cnf must match our cert
             self.assertEqual(binding["thumbprint_sha256"], cnf)
+        # The token must actually work at the resource, bound to our cert.
+        self._call_graph_with_mtls_pop_token(result["access_token"], cert)
         self._assert_pop_cache_hit(self.app, self._MTLS_POP_SCOPE)
 
-    def test_sni_mtls_pop_for_client_regional(self):
+    def test_credential_x509_output_bearer(self):
+        # The SAME SN/I cert, but WITHOUT mtls_proof_of_possession, must still
+        # authenticate via a signed client_assertion (private_key_jwt) and yield
+        # an ordinary Bearer token. This proves the client_assertion is dropped
+        # only on the mTLS-PoP path, never on the classic certificate path.
         from tests.lab_config import get_client_certificate
         if not clean_env("LAB_APP_CLIENT_CERT_PFX_PATH"):
             self.skipTest("LAB_APP_CLIENT_CERT_PFX_PATH not set")
         self.app = msal.ConfidentialClientApplication(
             self._SNI_ALLOWLISTED_CLIENT_ID,
             authority=self._SNI_ALLOWLISTED_AUTHORITY,
             client_credential=get_client_certificate(),
+            # Regional is correct/desirable HERE and must stay: a Bearer
+            # token_type is deterministic, so this retains live regional-endpoint
+            # E2E coverage. Region is FORBIDDEN on the mtls_pop cell, where the
+            # regional slice intermittently downgrades PoP -> Bearer (a Bearer
+            # *token*, not a temporarily_unavailable error) - which is why all
+            # sibling SDKs (.NET/Java/JS/Go) have no regional PoP cell either.
             azure_region="westus3")
-        result = self.app.acquire_token_for_client(
-            self._MTLS_POP_SCOPE, mtls_proof_of_possession=True)
-        if result.get("error") in ("invalid_request", "temporarily_unavailable"):
-            self.skipTest("Regional mTLS endpoint not reachable: %s" % result)
-        self.assertIn("access_token", result, "Regional mTLS PoP failed: %s" % result)
-        self.assertEqual("mtls_pop", result.get("token_type"))
+        result = self.app.acquire_token_for_client(self._MTLS_POP_SCOPE)
+        self.assertIn("access_token", result, "SN/I Bearer request failed: %s" % result)
+        self.assertEqual("Bearer", result.get("token_type"))
+        # Prove the cert binding is genuinely ABSENT, not merely that we skipped
+        # the flag. MSAL Python adds binding_certificate ONLY on the mtls_pop
+        # path, so assert both the SDK-internal contract (key omitted entirely)
+        # and the caller-visible one (a lookup yields None) - no cert is bound.
+        self.assertNotIn("binding_certificate", result)
+        self.assertIsNone(result.get("binding_certificate"))
+        self.assertCacheWorksForApp(result, self._MTLS_POP_SCOPE)
 
     def test_fic_two_leg_over_mtls_pop(self):
         from tests.lab_config import get_client_certificate