From 4350248d8fa62ef8dac6208595c3ddded1e0463f Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 6 Jul 2026 21:45:45 -0700 Subject: [PATCH] fix(oidc): carry the opt-in hint on discovered-endpoint rejections The discovered-endpoint wrapper converted every OAuthSSRFError to a bare OIDCError, so a private-resolving endpoint or trusted host got the non-public message without the allow_private_network remediation even though the same knob fixes it. Hoist the hint into a module constant and append it in both wrappers. Also name "unspecified" in the refused-even-with-opt-in message so 0.0.0.0/:: rejections read unambiguously. --- tests/test_oauth_ssrf.py | 3 ++- tests/test_oidc.py | 14 ++++++++++++++ turnstone/core/oauth_ssrf.py | 6 +++--- turnstone/core/oidc.py | 30 +++++++++++++++++------------- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/tests/test_oauth_ssrf.py b/tests/test_oauth_ssrf.py index d1551125..6eb9cf52 100644 --- a/tests/test_oauth_ssrf.py +++ b/tests/test_oauth_ssrf.py @@ -123,9 +123,10 @@ class TestValidateUrlNoSSRF: validate_url_no_ssrf("https://md.example.com", allow_http=False, allow_private=True) def test_allow_private_still_rejects_unspecified(self) -> None: + # The message names the class so 0.0.0.0/:: rejections are unambiguous. with ( patch("socket.getaddrinfo", return_value=[(2, 1, 6, "", ("0.0.0.0", 0))]), - pytest.raises(OAuthSSRFError, match="refused even with private"), + pytest.raises(OAuthSSRFError, match="unspecified"), ): validate_url_no_ssrf("https://zero.example.com", allow_http=False, allow_private=True) diff --git a/tests/test_oidc.py b/tests/test_oidc.py index ef1bdbe7..e50ba32c 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -565,6 +565,20 @@ class TestValidateDiscoveredEndpoint: trusted_endpoint_hosts=frozenset(), ) + def test_private_endpoint_hint_mentions_opt_in(self): + """A discovered endpoint resolving private carries the opt-in hint + just like the issuer does — the remediation is the same knob.""" + with ( + patch("socket.getaddrinfo", return_value=[(2, 1, 6, "", ("10.0.0.5", 0))]), + pytest.raises(OIDCError, match="allow_private_network"), + ): + validate_discovered_endpoint( + "https://idp.example.com/token", + self._issuer(), + allow_http=False, + trusted_endpoint_hosts=frozenset(), + ) + def test_rejects_http_when_issuer_is_https(self): """http:// discovered endpoint rejected when issuer is https://.""" with ( diff --git a/turnstone/core/oauth_ssrf.py b/turnstone/core/oauth_ssrf.py index aa65f9b3..ed7bdde7 100644 --- a/turnstone/core/oauth_ssrf.py +++ b/turnstone/core/oauth_ssrf.py @@ -153,9 +153,9 @@ def validate_url_no_ssrf( if allow_private: if addr.is_link_local or addr.is_multicast or addr.is_unspecified or addr.is_reserved: raise OAuthSSRFError( - f"endpoint URL resolves to a link-local/multicast/reserved " - f"address ({addr}), refused even with private addresses " - f"allowed: {url}" + f"endpoint URL resolves to a link-local/multicast/" + f"unspecified/reserved address ({addr}), refused even " + f"with private addresses allowed: {url}" ) continue raise OAuthSSRFPrivateAddressError( diff --git a/turnstone/core/oidc.py b/turnstone/core/oidc.py index 6d226c33..9f45b1d6 100644 --- a/turnstone/core/oidc.py +++ b/turnstone/core/oidc.py @@ -300,26 +300,26 @@ def load_oidc_config() -> OIDCConfig: # converting :class:`OAuthSSRFError` to :class:`OIDCError`. # --------------------------------------------------------------------------- +# Appended to every private-address rejection in this module (issuer and +# discovered endpoints alike): the login-flow URLs are operator-configured, +# so pointing the operator at the opt-in is safe here — unlike ``mcp_oauth``, +# where the URLs come from untrusted remote-server metadata and no such +# opt-in exists. +_PRIVATE_NETWORK_HINT = ( + " — to allow a self-hosted IdP on a private network, set " + "allow_private_network = true in the [oidc] section of config.toml " + "(or TURNSTONE_OIDC_ALLOW_PRIVATE_NETWORK=true)" +) + def _validate_url_no_ssrf( url: str, *, allow_http: bool, allow_private: bool = False ) -> urllib.parse.ParseResult: - """OIDC-flavoured wrapper around :func:`oauth_ssrf.validate_url_no_ssrf`. - - The private-address rejection gets the remediation hint appended: the - login-flow issuer is operator-configured, so pointing the operator at - the ``allow_private_network`` opt-in is safe here (unlike ``mcp_oauth``, - where the URLs come from untrusted remote-server metadata and no such - opt-in exists). - """ + """OIDC-flavoured wrapper around :func:`oauth_ssrf.validate_url_no_ssrf`.""" try: return _ssrf_validate_url_no_ssrf(url, allow_http=allow_http, allow_private=allow_private) except OAuthSSRFPrivateAddressError as exc: - raise OIDCError( - f"{exc} — to allow a self-hosted IdP on a private network, set " - "allow_private_network = true in the [oidc] section of config.toml " - "(or TURNSTONE_OIDC_ALLOW_PRIVATE_NETWORK=true)" - ) from exc + raise OIDCError(f"{exc}{_PRIVATE_NETWORK_HINT}") from exc except OAuthSSRFError as exc: raise OIDCError(str(exc)) from exc @@ -375,6 +375,10 @@ def validate_discovered_endpoint( trusted_endpoint_hosts=trusted_endpoint_hosts, allow_private=allow_private, ) + except OAuthSSRFPrivateAddressError as exc: + # A discovered endpoint (or trusted host) resolving private is fixed + # by the same opt-in as the issuer — carry the hint here too. + raise OIDCError(f"{exc}{_PRIVATE_NETWORK_HINT}") from exc except OAuthSSRFError as exc: raise OIDCError(str(exc)) from exc