fix(dashboard_auth): allow any http:// host in redirect_uri fast-fail (#38827)
The Nous dashboard OAuth 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" on the login screen. This broke self-hosted dashboards reached over plain HTTP — LAN IPs, internal hostnames, and reverse proxies that terminate TLS upstream. The Portal-side check (agent-redirect-uri.ts) 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 replaced test_rejects_http_with_non_localhost (which pinned the old behavior) with test_allows_http_with_arbitrary_host covering a Fly hostname, a LAN IP, and an internal hostname.
This commit is contained in:
@ -383,21 +383,17 @@ class NousDashboardAuthProvider(DashboardAuthProvider):
|
||||
"""Surface obviously-broken redirect_uris before bouncing to Portal.
|
||||
|
||||
The Portal-side check (``agent-redirect-uri.ts``) is authoritative;
|
||||
this is a fast-fail for the common operator-error case.
|
||||
this is a fast-fail for the common operator-error case. 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; Portal makes the final
|
||||
call on which redirect_uris are permitted.
|
||||
"""
|
||||
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', "
|
||||
|
||||
@ -494,11 +494,15 @@ class TestStartLogin:
|
||||
with pytest.raises(ProviderError, match="http"):
|
||||
provider.start_login(redirect_uri="ftp://x/auth/callback")
|
||||
|
||||
def test_rejects_http_with_non_localhost(self, provider):
|
||||
with pytest.raises(ProviderError, match="localhost"):
|
||||
provider.start_login(
|
||||
redirect_uri="http://hermes.fly.dev/auth/callback"
|
||||
)
|
||||
def test_allows_http_with_arbitrary_host(self, provider):
|
||||
# http:// is permitted for any host now, not just localhost — the
|
||||
# Portal-side check 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.fly.dev/auth/callback")
|
||||
provider.start_login(redirect_uri="http://192.168.1.50:8080/auth/callback")
|
||||
provider.start_login(redirect_uri="http://my-internal-host/auth/callback")
|
||||
|
||||
def test_allows_http_localhost(self, provider):
|
||||
# Should not raise.
|
||||
|
||||
Reference in New Issue
Block a user