diff --git a/plugins/dashboard_auth/_shared.py b/plugins/dashboard_auth/_shared.py index 3414e176aa..526321bc62 100644 --- a/plugins/dashboard_auth/_shared.py +++ b/plugins/dashboard_auth/_shared.py @@ -33,49 +33,6 @@ JSON_HEADERS = {"Accept": "application/json"} # drift seen in the field (#47815) without meaningfully extending token lifetime. DEFAULT_TOKEN_LEEWAY_SECONDS = 60.0 -# Defense against a hostile/compromised IdP serving unbounded responses to the -# dashboard auth providers: refuse to buffer past this size (issue #55121). -_OIDC_RESPONSE_BODY_LIMIT_BYTES = 1 * 1024 * 1024 -_OIDC_RESPONSE_CHUNK_BYTES = 64 * 1024 - - -def _request_limited_response(method: str, url: str, **kwargs: Any) -> httpx.Response: - """``httpx.request`` that never buffers an unbounded IdP response body. - - Content-Length is prechecked when declared; the streamed body is capped - chunk-by-chunk either way (a lying/absent header cannot bypass the bound). - Returns a fully-read ``httpx.Response`` with the same status/headers. - """ - with httpx.stream(method, url, **kwargs) as response: - declared = response.headers.get("content-length") - if declared: - try: - if int(declared) > _OIDC_RESPONSE_BODY_LIMIT_BYTES: - raise ProviderError( - f"endpoint response exceeds {_OIDC_RESPONSE_BODY_LIMIT_BYTES} bytes" - ) - except ValueError: - pass # malformed header — the chunk cap below still bounds the read - - chunks: list[bytes] = [] - total = 0 - for chunk in response.iter_bytes(chunk_size=_OIDC_RESPONSE_CHUNK_BYTES): - if not chunk: - continue - total += len(chunk) - if total > _OIDC_RESPONSE_BODY_LIMIT_BYTES: - raise ProviderError( - f"endpoint response exceeds {_OIDC_RESPONSE_BODY_LIMIT_BYTES} bytes" - ) - chunks.append(chunk) - - return httpx.Response( - status_code=response.status_code, - headers=response.headers, - content=b"".join(chunks), - request=response.request, - ) - # ---- Config / env resolution ---- @@ -213,8 +170,7 @@ def exchange_token( deliberately NOT followed: the body carries an auth code / refresh token. """ try: - response = _request_limited_response( - "POST", url, data=data, headers={**JSON_HEADERS, **(headers or {})}, timeout=TOKEN_ENDPOINT_TIMEOUT_SEC) + response = httpx.post(url, data=data, headers={**JSON_HEADERS, **(headers or {})}, timeout=TOKEN_ENDPOINT_TIMEOUT_SEC) except httpx.RequestError as exc: raise ProviderError(f"{endpoint} unreachable: {exc}") from exc if response.status_code == 400: diff --git a/plugins/dashboard_auth/self_hosted/__init__.py b/plugins/dashboard_auth/self_hosted/__init__.py index feb0abd64e..f056b973a7 100644 --- a/plugins/dashboard_auth/self_hosted/__init__.py +++ b/plugins/dashboard_auth/self_hosted/__init__.py @@ -28,7 +28,6 @@ from plugins.dashboard_auth._shared import ( TOKEN_ENDPOINT_TIMEOUT_SEC as _TOKEN_ENDPOINT_TIMEOUT_SEC, JwtOAuthProvider, SkipRegistration, - _request_limited_response, exchange_token, load_config_section, parse_json_body, @@ -138,8 +137,7 @@ class SelfHostedOIDCProvider(JwtOAuthProvider): extra_data, extra_headers = self._token_endpoint_auth(disco) data = {"token": refresh_token, "token_type_hint": "refresh_token", "client_id": self._client_id, **extra_data} try: - _request_limited_response( - "POST", endpoint, data=data, headers={**JSON_HEADERS, **extra_headers}, timeout=_TOKEN_ENDPOINT_TIMEOUT_SEC) + httpx.post(endpoint, data=data, headers={**JSON_HEADERS, **extra_headers}, timeout=_TOKEN_ENDPOINT_TIMEOUT_SEC) except Exception as exc: # noqa: BLE001 — best-effort logger.debug("self-hosted OIDC: revoke failed (ignored): %s", exc) return None @@ -215,8 +213,7 @@ class SelfHostedOIDCProvider(JwtOAuthProvider): # canonicalises .well-known; proxies upgrade http→https) and httpx defaults to # not following. The token/revocation POSTs deliberately do NOT follow # redirects (they carry an auth code / refresh token). - response = _request_limited_response( - "GET", url, headers=JSON_HEADERS, timeout=_DISCOVERY_TIMEOUT_SEC, follow_redirects=True) + response = httpx.get(url, headers=JSON_HEADERS, timeout=_DISCOVERY_TIMEOUT_SEC, follow_redirects=True) except httpx.RequestError as exc: raise ProviderError(f"OIDC discovery unreachable: {exc}") from exc if response.status_code != 200: diff --git a/tests/plugins/dashboard_auth/test_nous_provider.py b/tests/plugins/dashboard_auth/test_nous_provider.py index c60faa2fa3..6cdbf17f84 100644 --- a/tests/plugins/dashboard_auth/test_nous_provider.py +++ b/tests/plugins/dashboard_auth/test_nous_provider.py @@ -354,7 +354,7 @@ class TestCompleteLogin: "refresh_token": "rt_initial_value", }, ) - with patch("plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp): + with patch("plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp): session = provider.complete_login( code="abc", state="state-val", @@ -374,7 +374,7 @@ class TestCompleteLogin: def test_400_raises_invalid_code(self, provider): mock_resp = self._mock_post(400, {"error": "invalid_grant"}) - with patch("plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp): + with patch("plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp): with pytest.raises(InvalidCodeError, match="invalid_grant"): provider.complete_login( code="bad", state="s", code_verifier="v", @@ -384,7 +384,7 @@ class TestCompleteLogin: def test_500_raises_provider_error(self, provider): mock_resp = self._mock_post(500, "internal server error", ctype="text/plain") mock_resp.text = "internal server error" - with patch("plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp): + with patch("plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp): with pytest.raises(ProviderError, match="500"): provider.complete_login( code="x", state="s", code_verifier="v", @@ -393,7 +393,7 @@ class TestCompleteLogin: def test_missing_access_token_raises(self, provider): mock_resp = self._mock_post(200, {"token_type": "Bearer"}) - with patch("plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp): + with patch("plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp): with pytest.raises(ProviderError, match="access_token"): provider.complete_login( code="x", state="s", code_verifier="v", @@ -405,7 +405,7 @@ class TestCompleteLogin: mock_resp = self._mock_post( 200, {"access_token": access_token, "token_type": "DPoP"} ) - with patch("plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp): + with patch("plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp): with pytest.raises(ProviderError, match="token_type"): provider.complete_login( code="x", state="s", code_verifier="v", @@ -414,7 +414,7 @@ class TestCompleteLogin: def test_network_error_raises_provider_error(self, provider): with patch( - "plugins.dashboard_auth._shared._request_limited_response", + "plugins.dashboard_auth._shared.httpx.post", side_effect=httpx.ConnectError("conn refused"), ): with pytest.raises(ProviderError, match="unreachable"): @@ -525,7 +525,7 @@ class TestRefreshAndRevoke: }, ) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth._shared.httpx.post", return_value=mock_resp ) as mock_post: session = provider.refresh_session(refresh_token="rt_old_value") diff --git a/tests/plugins/dashboard_auth/test_self_hosted_provider.py b/tests/plugins/dashboard_auth/test_self_hosted_provider.py index 384197cee2..713d11c896 100644 --- a/tests/plugins/dashboard_auth/test_self_hosted_provider.py +++ b/tests/plugins/dashboard_auth/test_self_hosted_provider.py @@ -28,7 +28,6 @@ import pytest from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import rsa -import plugins.dashboard_auth._shared as shared import plugins.dashboard_auth.self_hosted as oidc_plugin from hermes_cli.dashboard_auth import ( InvalidCodeError, @@ -165,7 +164,7 @@ def _make_provider( return p -def _mock_post(status_code: int, body: Any, *, ctype: str = "application/json") -> MagicMock: +def _mock_post(status_code: int, body: Any, *, ctype: str = "application/json"): resp = MagicMock(spec=httpx.Response) resp.status_code = status_code if isinstance(body, dict): @@ -178,28 +177,6 @@ def _mock_post(status_code: int, body: Any, *, ctype: str = "application/json") return resp -class _FakeStreamResponse: - """A streaming httpx.Response stand-in yielding fixed chunks.""" - - def __init__(self, method: str, url: str, *, chunks, status_code: int = 200, - headers: Dict[str, str] | None = None) -> None: - self.status_code = status_code - self.headers = headers or {} - self.request = httpx.Request(method, url) - self.url = httpx.URL(url) - self._chunks = chunks - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc, tb): - return False - - def iter_bytes(self, chunk_size: int = 65536): - _ = chunk_size - yield from self._chunks - - # --------------------------------------------------------------------------- # Construction # --------------------------------------------------------------------------- @@ -248,7 +225,7 @@ class TestDiscovery: p = self._provider() mock_resp = self._mock_get(200, dict(_DISCOVERY_DOC)) with patch( - "plugins.dashboard_auth.self_hosted._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.get", return_value=mock_resp ) as mock_get: disco1 = p._get_discovery() disco2 = p._get_discovery() @@ -274,7 +251,7 @@ class TestDiscovery: 200, forged, url="https://attacker.example/openid-configuration" ) with patch( - "plugins.dashboard_auth.self_hosted._request_limited_response", return_value=resp + "plugins.dashboard_auth.self_hosted.httpx.get", return_value=resp ): with pytest.raises(ProviderError, match="origin"): p._fetch_discovery() @@ -285,7 +262,7 @@ class TestDiscovery: 200, dict(_DISCOVERY_DOC), url="http://auth.example.com/discovery" ) with patch( - "plugins.dashboard_auth.self_hosted._request_limited_response", return_value=resp + "plugins.dashboard_auth.self_hosted.httpx.get", return_value=resp ): with pytest.raises(ProviderError, match="origin"): p._fetch_discovery() @@ -298,7 +275,7 @@ class TestDiscovery: url="https://auth.example.com/.well-known/openid-configuration/application/o/hermes", ) with patch( - "plugins.dashboard_auth.self_hosted._request_limited_response", return_value=resp + "plugins.dashboard_auth.self_hosted.httpx.get", return_value=resp ): disco = p._fetch_discovery() assert disco["token_endpoint"] == f"{_ISSUER}/token" @@ -310,53 +287,11 @@ class TestDiscovery: 200, dict(_DISCOVERY_DOC), url="https://auth.example.com:443/x" ) with patch( - "plugins.dashboard_auth.self_hosted._request_limited_response", return_value=resp + "plugins.dashboard_auth.self_hosted.httpx.get", return_value=resp ): assert p._fetch_discovery()["issuer"] == _ISSUER - def test_discovery_rejects_oversized_response_body(self, monkeypatch): - """A discovery body larger than the cap must raise, not buffer.""" - p = self._provider() - - def fake_stream(method, url, **kwargs): - return _FakeStreamResponse( - method, url, - chunks=[b"a" * 6, b"b" * 6], - headers={"content-type": "application/json"}, - ) - - monkeypatch.setattr(shared.httpx, "stream", fake_stream) - monkeypatch.setattr(shared, "_OIDC_RESPONSE_BODY_LIMIT_BYTES", 10) - - with pytest.raises(ProviderError, match="exceeds 10 bytes"): - p._get_discovery() - - def test_discovery_rejects_oversized_content_length(self, monkeypatch): - """An oversized declared Content-Length must reject before body read.""" - p = self._provider() - read_attempted = [] - - class _ContentLengthResp(_FakeStreamResponse): - def iter_bytes(self, chunk_size=65536): - read_attempted.append(True) - yield from () - - def fake_stream(method, url, **kwargs): - return _ContentLengthResp( - method, url, - chunks=[], - headers={"content-type": "application/json", "content-length": "1048577"}, - ) - - monkeypatch.setattr(shared.httpx, "stream", fake_stream) - monkeypatch.setattr(shared, "_OIDC_RESPONSE_BODY_LIMIT_BYTES", 1048576) - - with pytest.raises(ProviderError, match="exceeds"): - p._get_discovery() - assert not read_attempted - - # --------------------------------------------------------------------------- # OIDC discovery against a REAL HTTP server that redirects (regression) # --------------------------------------------------------------------------- @@ -548,7 +483,7 @@ class TestCompleteLogin: }, ) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ): session = provider.complete_login( code="abc", @@ -571,7 +506,7 @@ class TestCompleteLogin: 200, {"id_token": id_token, "token_type": "Bearer"} ) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ): session = provider.complete_login( code="abc", @@ -586,7 +521,7 @@ class TestCompleteLogin: 200, {"access_token": "opaque", "token_type": "Bearer"} ) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ): with pytest.raises(ProviderError, match="id_token"): provider.complete_login( @@ -596,31 +531,10 @@ class TestCompleteLogin: redirect_uri="https://hermes.example/auth/callback", ) - def test_token_endpoint_rejects_oversized_response_body(self, provider, monkeypatch): - """An IDP token response larger than the cap must raise, not buffer.""" - - def fake_stream(method, url, **kwargs): - return _FakeStreamResponse( - method, url, - chunks=[b"x" * 8, b"y" * 8], - headers={"content-type": "application/json"}, - ) - - monkeypatch.setattr(shared.httpx, "stream", fake_stream) - monkeypatch.setattr(shared, "_OIDC_RESPONSE_BODY_LIMIT_BYTES", 10) - - with pytest.raises(ProviderError, match="exceeds 10 bytes"): - provider.complete_login( - code="x", - state="s", - code_verifier="v", - redirect_uri="https://hermes.example/auth/callback", - ) - def test_400_raises_invalid_code(self, provider): mock_resp = _mock_post(400, {"error": "invalid_grant"}) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ): with pytest.raises(InvalidCodeError, match="invalid_grant"): provider.complete_login( @@ -658,7 +572,7 @@ class TestConfidentialClient: id_token = _mint_id_token(rsa_keypair) mock_resp = _mock_post(200, {"id_token": id_token, **_GOOD_TOKEN_RESP_KEYS}) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ) as mock_post: provider.complete_login( code="the-code", @@ -730,7 +644,7 @@ class TestConfidentialClient: 200, {"id_token": id_token, "token_type": "Bearer", "refresh_token": "rt2"} ) with patch( - "plugins.dashboard_auth._shared._request_limited_response", return_value=mock_resp + "plugins.dashboard_auth.self_hosted.httpx.post", return_value=mock_resp ) as mock_post: provider.refresh_session(refresh_token="rt_old") _, kwargs = mock_post.call_args