fix(auth): parse hostname for mTLS and PSC endpoint certificate rotat… by attharva-24 · Pull Request #18153 · googleapis/google-cloud-python

attharva-24 · 2026-08-19T06:49:08Z

Fixes #18147
Follow-up to #17928

Description

This PR resolves two defects in the mTLS endpoint detection logic previously used in requests.py and urllib3.py:

Eliminates False Positives: Replaces raw URL substring search (prefix in url) with proper hostname isolation via urllib.parse.urlsplit(url).hostname. Standard non-mTLS URLs containing mtls.googleapis.com in paths or query parameters (e.g. https://storage.googleapis.com/bucket/mtls.googleapis.com or https://logging.googleapis.com/v2/entries?filter=mtls.googleapis.com) will no longer trigger unnecessary certificate rotation on 401.
Adds Private Service Connect (PSC) Support: Adds support for enterprise PSC custom mTLS domains (*.p.googleapis.com) and regional mTLS domains (*.rep.mtls.googleapis.com), ensuring certificate rotation functions correctly for PSC connections.
Centralizes Endpoint Helper: Adds _mtls_helper.is_mtls_endpoint(url) shared across both requests and urllib3 transports, with lazy evaluation on 401 status codes.

Tests

Added TestIsMtlsEndpoint unit test suite in tests/transport/test__mtls_helper.py covering standard mTLS, PSC endpoints, regional endpoints, path/query substring traps, port numbers, and edge cases.
Added integration tests in tests/transport/test_requests.py and tests/transport/test_urllib3.py verifying cert rotation is skipped on non-mTLS URLs with matching substrings and triggered on PSC URLs.

gemini-code-assist

Code Review

This pull request centralizes and improves mTLS endpoint detection by introducing the is_mtls_endpoint helper function in _mtls_helper.py, replacing previous substring-based checks in both the requests and urllib3 transports. It also adds comprehensive unit tests to verify the new endpoint detection and cert rotation logic. The review feedback highlights a potential TypeError in is_mtls_endpoint when handling bytes URLs, as calling endswith with string suffixes on a bytes hostname outside the try-except block will raise an exception. Decoding bytes inputs to str at the start of the function is recommended to ensure robust error handling.

parthea · 2026-08-19T14:40:06Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the mTLS endpoint detection logic by introducing a centralized is_mtls_endpoint helper in _mtls_helper.py and updating both requests and urllib3 transports to use it. It also adds comprehensive unit tests to verify the new helper and ensure cert rotation is skipped on non-mTLS URLs. The review feedback suggests improving the robustness of is_mtls_endpoint by handling non-string/non-bytes URL objects (such as urllib3.util.Url) to prevent them from being incorrectly classified as non-mTLS endpoints due to caught TypeErrors.

gemini-code-assist · 2026-08-19T14:42:01Z

+    if not url:
+        return False
+    if isinstance(url, bytes):
+        try:
+            url = url.decode("utf-8")
+        except (UnicodeDecodeError, AttributeError):
+            return False
+    try:
+        hostname = urlsplit(url).hostname
+    except (ValueError, TypeError, AttributeError):
+        return False


In urllib3, the url parameter passed to urlopen can be a urllib3.util.Url object (or other string-like/URL objects) rather than a plain str or bytes. Currently, passing a urllib3.util.Url object to is_mtls_endpoint will cause urlsplit(url) to raise a TypeError, which is caught and results in returning False—even if the object represents a valid mTLS endpoint.

To prevent this and ensure robust compatibility with urllib3's native URL objects, we should check if the input has a .url attribute (which urllib3.util.Url exposes as a property returning the string representation) or fall back to converting it to a string.

Suggested change

if not url:

return False

if isinstance(url, bytes):

try:

url = url.decode("utf-8")

except (UnicodeDecodeError, AttributeError):

return False

try:

hostname = urlsplit(url).hostname

except (ValueError, TypeError, AttributeError):

return False

if not url:

return False

if isinstance(url, bytes):

try:

url = url.decode("utf-8")

except (UnicodeDecodeError, AttributeError):

return False

elif not isinstance(url, str):

if hasattr(url, "url"):

url = url.url

else:

try:

url = str(url)

except Exception:

return False

try:

hostname = urlsplit(url).hostname

except (ValueError, TypeError, AttributeError):

return False

References

Adheres to the defensive programming guidelines in Section 2 of the Repository Style Guide by validating and normalizing the input type before performing operations that could raise TypeErrors. ^(link)

Do not replace historical graceful fallback behaviors (such as returning False/falling back to standard TLS) with exceptions if doing so would introduce breaking changes for downstream users and violate backwards compatibility.

@attharva-24 PTAL

Addressed in the latest commit!

is_mtls_endpoint now checks for .url on urllib3.util.Url (and other URL objects), supports bytes inputs with safe UTF-8 decoding, and falls back to string conversion. Added unit tests covering urllib3.util.Url, bytes, and str inputs across standard, PSC, and regional mTLS endpoints.

macastelaz · 2026-08-20T15:49:54Z

+    if not hostname:
+        return False
+
+    hostname = hostname.lower()


nit: I believe technically FQDNs can end with a trailing root dot (e.g. "https://pubsub.mtls.googleapis.com." is technically valid - although not expected in the wild). For complete coverage, perhaps we should also strip "." (e.g. hostname = hostname.rstrip(".").lower();

Added .rstrip(".") before matching so trailing root dots on FQDNs are stripped cleanly, along with unit test cases for standard mTLS, PSC, exact hosts, and edge cases.

6f85765

macastelaz · 2026-08-20T15:55:37Z

+class TestIsMtlsEndpoint(object):
+    @pytest.mark.parametrize(
+        "url",
+        [


This lacks examples with explicit port numbers (example: https://pubsub.mtls.googleapis.com:443/v1) and queries and fragments (e.g. "https://pubsub.mtls.googleapis.com/v1/projects?pageSize=10#frag")

Added unit test cases in TestIsMtlsEndpoint covering explicit port numbers (:443, :8443), query parameters, URL fragments, and combined port+query+fragment permutations for standard mTLS, PSC, and non-mTLS endpoints.

a9eb0be

macastelaz · 2026-08-20T15:56:02Z

+
+    @pytest.mark.parametrize(
+        "url",
+        [


Consider adding a bare PSC case (example "https://p.googleapis.com").

Additionally a case like https://[2001:db8::1]:443/mtls.googleapis.com would be good to demonstrated handling of IPv6 syntax handling

Added "p.googleapis.com" to _MTLS_EXACT_HOSTS so bare apex PSC domains are recognized as mTLS endpoints, along with unit test cases for "https://p.googleapis.com", ports, and trailing root dots.

Added IPv6 test cases (https://[2001:db8::1]:443/mtls.googleapis.com and https://[::1]:8443/mtls.googleapis.com) to confirm that bracketed IPv6 host syntax is handled properly.

9dbd745

macastelaz · 2026-08-20T15:57:48Z

+)
+
+
+def is_mtls_endpoint(url: Optional[Union[str, bytes, Any]]) -> bool:


instead of Any can you use "object"? I think ideally we'd avoid usage of Any wherever possible.

Replaced Any with object in the function signature and docstrings, and removed the unused Any import.

f20b946

macastelaz · 2026-08-20T16:01:31Z

@@ -937,9 +940,19 @@ def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self):
            # Assert mTLS check logic was SKIPPED (Inner Check was False)
            assert not mock_helper.check_parameters_for_unauthorized_response.called

-    def test_cert_rotation_skipped_on_non_mtls_url(self):
+    @pytest.mark.parametrize(


I think this is redundant - these transports are no longer responsible for mtls checks themselves and the thing under test here shouldn't be if various forms on non-mtls endpoints are detected correctly (that is already covered in the new mtls_helper tests). Instead, I'd suggest just covering one example of mtls and one example of non mtls here to cover the requests logic specifically. Same for urllib3

Looks like just this comment is pending and then I'll take one more look.

Simplified the transport test cases. One represents mTLS endpoint and one represents non-mTLS endpoint, while keeping the full endpoint permutations covered in test__mtls_helper.py.

dafe9d1

…ion (googleapis#18147) * Isolate hostname using urllib.parse.urlsplit in _mtls_helper.is_mtls_endpoint * Eliminate false positives on non-mTLS URLs containing mtls substrings in paths/queries * Add support for Private Service Connect (*.p.googleapis.com) custom mTLS endpoints * Update AuthorizedSession and AuthorizedHttp to use shared is_mtls_endpoint helper * Add comprehensive unit tests in test__mtls_helper, test_requests, and test_urllib3 Fixes googleapis#18147 Follow-up to googleapis#17928

…agments

…ions

… examples

#18189) (#18153) This reverts commit b642373.

attharva-24 requested review from a team as code owners August 19, 2026 06:49

attharva-24 assigned attharva-24 and unassigned attharva-24 Aug 19, 2026

gemini-code-assist Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/_mtls_helper.py Outdated

attharva-24 force-pushed the fix-mtls-url-check branch from e33b6c9 to f217bfc Compare August 19, 2026 06:53

gemini-code-assist Bot reviewed Aug 19, 2026

View reviewed changes

parthea assigned attharva-24 Aug 19, 2026

attharva-24 force-pushed the fix-mtls-url-check branch from f217bfc to 7fecb67 Compare August 19, 2026 19:32

macastelaz reviewed Aug 20, 2026

View reviewed changes

attharva-24 force-pushed the fix-mtls-url-check branch from 7fecb67 to 6f85765 Compare August 20, 2026 23:54

attharva-24 added 6 commits August 21, 2026 16:29

fix(auth): strip trailing root dot in hostname for FQDNs

e863710

test(auth): add test cases for explicit port numbers, queries, and fr…

c4fc118

…agments

fix(auth): support bare PSC domain and add IPv6 unit tests

07f4549

style(auth): replace Any with object in is_mtls_endpoint type annotat…

615ae8b

…ions

test(auth): simplify transport rotation tests to single mTLS/non-mTLS…

2eb5fe3

… examples

attharva-24 force-pushed the fix-mtls-url-check branch from dafe9d1 to 2eb5fe3 Compare August 21, 2026 16:30

macastelaz approved these changes Aug 21, 2026

View reviewed changes

attharva-24 merged commit b642373 into googleapis:main Aug 21, 2026
44 checks passed

release-please Bot mentioned this pull request Aug 21, 2026

chore: release main #18013

Open

attharva-24 mentioned this pull request Aug 21, 2026

Revert "fix(auth): parse hostname for mTLS and PSC endpoint certifica… #18189

Merged

macastelaz pushed a commit that referenced this pull request Aug 21, 2026

Revert "fix(auth): parse hostname for mTLS and PSC endpoint certifica… (

b42122e

#18189) (#18153) This reverts commit b642373.

		)


		def is_mtls_endpoint(url: Optional[Union[str, bytes, Any]]) -> bool:

Uh oh!

Conversation

attharva-24 commented Aug 19, 2026

Description

Tests

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

parthea commented Aug 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

attharva-24 Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

attharva-24 Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

attharva-24 Aug 21, 2026 •

edited

Loading

attharva-24 Aug 21, 2026 •

edited

Loading