mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(dashboard_auth): allow any http:// host in self-hosted OIDC redirect_uri (#55099)
The self-hosted OIDC dashboard login rejected any http:// redirect_uri whose host was not localhost/127.0.0.1, surfacing "redirect_uri may only use http:// for localhost/127.0.0.1" before reaching the IDP. This broke self-hosted dashboards reached over plain HTTP (including LAN IPs, internal hostnames, and reverse proxies that terminate TLS upstream). #38827 already dropped this check from the nous provider, but the generic self-hosted provider copied the old localhost-only branch and reintroduced the bug for HERMES_DASHBOARD_OIDC_ISSUER setups. The IDP's own allowlist is authoritative on which redirect_uris are permitted; this client-side _validate_redirect_uri is only a fast-fail for obvious operator error and should not second-guess valid http:// deployments. Fix: drop the localhost-only branch on the http scheme. Validation now enforces only that the scheme is http(s) and the path ends with /auth/callback. Updated the docstring to explain the relaxed contract, and added test_allows_http_with_arbitrary_host covering an internal hostname and a LAN IP alongside the existing localhost case.
This commit is contained in:
parent
d2ce2c852d
commit
f3d2dfbec6
2 changed files with 15 additions and 9 deletions
|
|
@ -610,21 +610,17 @@ class SelfHostedOIDCProvider(DashboardAuthProvider):
|
|||
"""Fast-fail obviously-broken redirect_uris before bouncing to the IDP.
|
||||
|
||||
The IDP's own allowlist is authoritative; this just catches the common
|
||||
operator-error case with a clear message. Mirrors the nous provider.
|
||||
operator-error case with a clear message. We allow any ``http://`` host
|
||||
(not just localhost) so self-hosted dashboards reached over plain HTTP —
|
||||
LAN IPs, internal hostnames, reverse proxies that terminate TLS upstream
|
||||
— are not rejected here; the IDP makes the final call on which
|
||||
redirect_uris are permitted. Mirrors the nous provider.
|
||||
"""
|
||||
parsed = urllib.parse.urlparse(redirect_uri)
|
||||
if parsed.scheme not in ("https", "http"):
|
||||
raise ProviderError(
|
||||
f"redirect_uri must be http(s), got {redirect_uri!r}"
|
||||
)
|
||||
if parsed.scheme == "http" and parsed.hostname not in (
|
||||
"localhost",
|
||||
"127.0.0.1",
|
||||
):
|
||||
raise ProviderError(
|
||||
"redirect_uri may only use http:// for localhost/127.0.0.1, "
|
||||
f"got {redirect_uri!r}"
|
||||
)
|
||||
if not parsed.path or not parsed.path.endswith("/auth/callback"):
|
||||
raise ProviderError(
|
||||
"redirect_uri path must end with '/auth/callback', "
|
||||
|
|
|
|||
|
|
@ -496,6 +496,16 @@ class TestStartLogin:
|
|||
with pytest.raises(ProviderError, match="/auth/callback"):
|
||||
provider.start_login(redirect_uri="https://x.example/oauth/cb")
|
||||
|
||||
def test_allows_http_with_arbitrary_host(self, provider):
|
||||
# http:// is permitted for any host now, not just localhost — the
|
||||
# IDP-side allowlist is authoritative on which redirect_uris are
|
||||
# accepted; this client-side fast-fail must not reject self-hosted
|
||||
# dashboards reached over plain HTTP (LAN IPs, internal hostnames,
|
||||
# TLS-terminating reverse proxies). Should not raise.
|
||||
provider.start_login(redirect_uri="http://hermes.example/auth/callback")
|
||||
provider.start_login(redirect_uri="http://192.168.1.50:9119/auth/callback")
|
||||
provider.start_login(redirect_uri="http://my-internal-host/auth/callback")
|
||||
|
||||
def test_allows_http_localhost_redirect(self, provider):
|
||||
provider.start_login(redirect_uri="http://localhost:8080/auth/callback")
|
||||
provider.start_login(redirect_uri="http://127.0.0.1:8080/auth/callback")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue