mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(dashboard-auth): follow redirects on self-hosted OIDC discovery (#53399)
The self-hosted OIDC provider fetched the discovery document with a bare httpx.get(). httpx defaults to follow_redirects=False (unlike curl -L or the requests library), so when an IDP answers GET /.well-known/openid-configuration with a 3xx — Authentik canonicalises the .well-known path, and any IDP behind a reverse proxy doing an http→https upgrade redirects too — the bare redirect (empty body) tripped the status != 200 guard and raised 'OIDC discovery returned 302', which routes.py maps to the provider_unreachable audit event and a 503. The browser surfaced 'Auth provider self-hosted unreachable'. The user's smoking gun (curl -o writing zero bytes from inside the container) is exactly a redirect with no body — the same wall the code hit. Add follow_redirects=True to the discovery GET only. It's safe: the issuer-pin check and _require_https_or_loopback still validate the resolved document and every endpoint, so a redirect can't smuggle in a bad issuer or a cleartext endpoint. The token/revocation POSTs deliberately keep the no-follow default (they carry an auth code / refresh token and the endpoint is already the canonical absolute URL). Existing discovery tests mocked httpx.get with a canned 200 and never exercised a real 3xx. Add a regression test that runs a real loopback server returning a 302 on the .well-known path — fails without the fix (ProviderError: discovery returned 302), passes with it.
This commit is contained in:
parent
dd0e4ab81a
commit
fbf748b282
2 changed files with 104 additions and 0 deletions
|
|
@ -419,10 +419,26 @@ class SelfHostedOIDCProvider(DashboardAuthProvider):
|
|||
def _fetch_discovery(self) -> Dict[str, Any]:
|
||||
url = self._discovery_url()
|
||||
try:
|
||||
# follow_redirects=True: many IDPs answer the discovery GET with a
|
||||
# 3xx rather than a direct 200 — Authentik canonicalises the
|
||||
# ``.well-known`` path, and any IDP behind a reverse proxy doing an
|
||||
# http→https upgrade redirects too. httpx (unlike curl -L or the
|
||||
# requests library) defaults to follow_redirects=False, so without
|
||||
# this the redirect comes back as a bare 3xx with an empty body and
|
||||
# the ``status != 200`` check below raises "discovery returned 302"
|
||||
# → provider_unreachable → 503. Following the redirect is safe: the
|
||||
# issuer-pin check and _require_https_or_loopback below still
|
||||
# validate the *resolved* document and every endpoint in it, so a
|
||||
# redirect to a hostile location can't smuggle in a bad issuer or a
|
||||
# cleartext endpoint. (The token/revocation POSTs deliberately do
|
||||
# NOT follow redirects — see _exchange — because they carry an auth
|
||||
# code / refresh token and the endpoint is already the canonical
|
||||
# absolute URL resolved here.)
|
||||
response = httpx.get(
|
||||
url,
|
||||
headers={"Accept": "application/json"},
|
||||
timeout=_DISCOVERY_TIMEOUT_SEC,
|
||||
follow_redirects=True,
|
||||
)
|
||||
except httpx.RequestError as exc:
|
||||
raise ProviderError(f"OIDC discovery unreachable: {exc}") from exc
|
||||
|
|
|
|||
|
|
@ -316,6 +316,94 @@ class TestDiscovery:
|
|||
p._get_discovery()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OIDC discovery against a REAL HTTP server that redirects (regression)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDiscoveryRealRedirect:
|
||||
"""Discovery must follow a 3xx on the .well-known GET.
|
||||
|
||||
The rest of the discovery suite mocks ``httpx.get`` with a canned 200, so
|
||||
it cannot see httpx's ``follow_redirects=False`` default. Many real IDPs
|
||||
answer the discovery GET with a redirect rather than a direct 200 —
|
||||
Authentik canonicalises the ``.well-known`` path, and any IDP behind a
|
||||
reverse proxy doing http→https upgrade redirects too. Before the fix the
|
||||
bare 3xx (empty body) tripped the ``status != 200`` guard and surfaced as
|
||||
``provider_unreachable`` → HTTP 503 (the symptom in the user report:
|
||||
``curl -o`` writing zero bytes is exactly a redirect with no body).
|
||||
|
||||
This exercises the real httpx transport against a loopback server, so it
|
||||
fails without ``follow_redirects=True`` and passes with it — a behaviour
|
||||
contract, not a mock-shaped snapshot.
|
||||
"""
|
||||
|
||||
def _serve(self, handler_cls):
|
||||
import http.server
|
||||
import socketserver
|
||||
import threading
|
||||
|
||||
# Bind :0 so the OS picks a free port (parallel-runner safe).
|
||||
httpd = socketserver.TCPServer(("127.0.0.1", 0), handler_cls)
|
||||
port = httpd.server_address[1]
|
||||
thread = threading.Thread(target=httpd.serve_forever, daemon=True)
|
||||
thread.start()
|
||||
return httpd, port
|
||||
|
||||
def test_discovery_follows_redirect_to_json(self):
|
||||
import http.server
|
||||
|
||||
holder: Dict[str, Any] = {}
|
||||
|
||||
class Handler(http.server.BaseHTTPRequestHandler):
|
||||
def log_message(self, format, *args): # silence test-server logging
|
||||
pass
|
||||
|
||||
def do_GET(self):
|
||||
issuer = holder["issuer"]
|
||||
if self.path == "/.well-known/openid-configuration":
|
||||
# 302 with an EMPTY body — the failing shape.
|
||||
self.send_response(302)
|
||||
self.send_header(
|
||||
"Location", "/canonical/openid-configuration"
|
||||
)
|
||||
self.end_headers()
|
||||
return
|
||||
if self.path == "/canonical/openid-configuration":
|
||||
body = json.dumps(
|
||||
{
|
||||
"issuer": issuer,
|
||||
"authorization_endpoint": f"{issuer}/authorize",
|
||||
"token_endpoint": f"{issuer}/token",
|
||||
"jwks_uri": f"{issuer}/jwks",
|
||||
}
|
||||
).encode()
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.send_header("Content-Length", str(len(body)))
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
return
|
||||
self.send_response(404)
|
||||
self.end_headers()
|
||||
|
||||
httpd, port = self._serve(Handler)
|
||||
try:
|
||||
# Loopback http is permitted by _require_https_or_loopback.
|
||||
issuer = f"http://127.0.0.1:{port}"
|
||||
holder["issuer"] = issuer
|
||||
p = oidc_plugin.SelfHostedOIDCProvider(
|
||||
issuer=issuer, client_id=_CLIENT_ID
|
||||
)
|
||||
disco = p._get_discovery()
|
||||
assert disco["token_endpoint"] == f"{issuer}/token"
|
||||
assert disco["authorization_endpoint"] == f"{issuer}/authorize"
|
||||
assert disco["jwks_uri"] == f"{issuer}/jwks"
|
||||
finally:
|
||||
httpd.shutdown()
|
||||
httpd.server_close()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# start_login
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue