fix(security): pin Service Fabric managed identity TLS

[Nilesh Choudhary (4gust)]

Derives an isolated Requests session for Service Fabric, enforces HTTPS, validates the configured certificate thumbprint before transmitting the Secret header, and fails closed for unsupported custom clients or adapters. Includes self-contained TLS validation coverage.

[Copilot]

Pull request overview

This PR hardens the Service Fabric managed identity flow by isolating transport settings, enforcing HTTPS, and validating the configured Service Fabric certificate thumbprint before sending the Secret header.

Changes:

• Enforce https:// endpoints for Service Fabric managed identity and normalize/validate IDENTITY_SERVER_THUMBPRINT.
• Derive a dedicated requests.Session for Service Fabric with a pinned-certificate transport and fail closed for unsupported custom clients/adapters.
• Add tests that stand up a local TLS endpoint and validate thumbprint pinning and “secret not sent on failure” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
msal/managed_identity.py	Adds Service Fabric HTTPS enforcement, thumbprint normalization, pinned transport via a derived requests.Session, and fail-closed checks for custom clients/adapters.
tests/test_mi.py	Updates existing Service Fabric tests to account for derived-session behavior and adds TLS pinning validation tests with a local HTTPS server.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (2)

msal/managed_identity.py:725

• The comment in cert_verify contradicts the implementation: this adapter always forces verify=False (disabling CA/hostname verification) and relies on thumbprint pinning in connect(). Please update the comment to reflect the actual trust model so readers don’t assume CA validation is happening here.

    def cert_verify(self, conn, url, verify, cert):
        # The exact Service Fabric certificate thumbprint is the trust anchor.
        # Do not inherit caller-provided verify=False or a custom CA configuration.
        super(_ServiceFabricHTTPAdapter, self).cert_verify(
            conn, url, verify=False, cert=cert)

msal/managed_identity.py:765

• _create_service_fabric_http_client copies connection pool settings by reaching into HTTPAdapter private attributes (_pool_connections/_pool_maxsize/_pool_block). Those are not part of requests’ public API and can break across requests versions. Consider using getattr() with safe defaults (or omit these kwargs) to avoid AttributeError and reduce coupling to internals.

        max_retries=copy.deepcopy(source_adapter.max_retries),
        pool_connections=source_adapter._pool_connections,
        pool_maxsize=source_adapter._pool_maxsize,
        pool_block=source_adapter._pool_block,
    ))

[Copilot]

Pull request overview

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

Suppressed comments (2)

msal/managed_identity.py:676

• getpeercert(binary_form=True) can return None when the peer doesn't provide a certificate. As written, that would raise a TypeError in hashlib.sha1(...) (instead of a consistent TLS verification error), which can leak an unexpected exception type to callers.

        certificate = self.sock.getpeercert(binary_form=True)
        actual_thumbprint = hashlib.sha1(certificate).hexdigest()
        if not hmac.compare_digest(actual_thumbprint, self._server_thumbprint):

tests/test_mi.py:511

• The HTTPS test server thread is non-daemon; if a failure occurs before tearDown runs (or shutdown() blocks unexpectedly), it can hang the test process. Making it a daemon thread avoids a hard hang while keeping the explicit shutdown()/join() cleanup.

        self.server_thread = threading.Thread(target=self.server.serve_forever)