revert: dashboard-auth token exchange breaks on gzip IdP responses (partial revert of #133938)
Reverts the plugins/dashboard_auth half ofca38caeea0. _request_limited_response streamed the body via iter_bytes() (already gzip-decoded by httpx) and rebuilt an httpx.Response with the ORIGINAL headers, Content-Encoding: gzip included, so .json() tried to gunzip plaintext. Every Nous Portal dashboard login failed with 503 "Provider unreachable: Portal token endpoint unreachable: Error -3 while decompressing data: incorrect header check". Self-hosted OIDC discovery/token/revocation used the same helper. The urllib half (gateway health probe, ElevenLabs voices) is kept: urllib does not decode Content-Encoding, andc061068b26builds on its _read_dashboard_json_response helper.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user