fix(dashboard-auth): bypass loopback WS peer check in gated mode
When the OAuth gate is active, start_server runs uvicorn with proxy_headers=True so the dashboard can honour X-Forwarded-Proto from Fly's TLS terminator (cookies, redirect URI reconstruction). A side effect: ws.client.host is rewritten to the X-Forwarded-For value, which on Fly is the real internet client IP — never loopback. The loopback peer guard in _ws_client_is_allowed then rejected every WS upgrade in gated mode (4403 close) even after a successful OAuth round trip and ticket consumption, silently breaking /api/pty, /api/ws, /api/pub, and /api/events. Fix: in gated mode, bypass the peer-IP check. The OAuth gate + single-use ticket is the auth. The Host/Origin guard in _ws_host_origin_is_allowed still runs and is what protects against DNS-rebinding here, not the peer IP. Loopback mode behaviour is unchanged: the legacy ?token= path is the only auth there and we don't want LAN hosts guessing tokens. Regression coverage: TestWsRequestIsAllowedGated pins all four behaviours — non-loopback peer allowed in gated mode, non-loopback peer rejected in loopback mode, loopback peer allowed in loopback mode, and the Host/Origin guard still firing on a rebinding attempt with gated mode + matching peer.
This commit is contained in:
@@ -3376,8 +3376,20 @@ _LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1", "localhost", "testclient"})
|
||||
def _ws_client_is_allowed(ws: "WebSocket") -> bool:
|
||||
"""Check if the WebSocket client IP is acceptable.
|
||||
|
||||
Allows loopback clients only.
|
||||
Loopback mode: only loopback clients allowed — the legacy
|
||||
``?token=<_SESSION_TOKEN>`` path is the only auth we have, so we
|
||||
don't want LAN hosts guessing tokens.
|
||||
|
||||
Gated mode: any peer is allowed — uvicorn's ``proxy_headers=True``
|
||||
(enabled when the OAuth gate is active so cookies can pick up
|
||||
``X-Forwarded-Proto``) rewrites ``ws.client.host`` to the
|
||||
X-Forwarded-For value, which is the real internet client IP. The
|
||||
OAuth gate + single-use ``?ticket=`` is the auth at that point; the
|
||||
Host/Origin guard in :func:`_ws_host_origin_is_allowed` is what
|
||||
blocks DNS-rebinding here, not the peer IP.
|
||||
"""
|
||||
if getattr(app.state, "auth_required", False):
|
||||
return True
|
||||
client_host = ws.client.host if ws.client else ""
|
||||
if not client_host:
|
||||
return True
|
||||
|
||||
Reference in New Issue
Block a user