fix(dashboard): _require_token endpoints all 401 behind the OAuth gate (#42578)

* fix(dashboard): let _require_token endpoints work behind the OAuth gate

In gated/OAuth mode (non-loopback bind without --insecure) the dashboard
authenticates the SPA via a session cookie and deliberately does NOT inject
the legacy ephemeral _SESSION_TOKEN into index.html. gated_auth_middleware
verifies the cookie and attaches request.state.session before any non-public
/api/ route runs; the legacy auth_middleware short-circuits in this mode too.

But several handlers call _require_token() directly, which only validated the
(absent) _SESSION_TOKEN header. So every cookie-authenticated request to those
endpoints 401'd — making plugin install/enable/disable, /api/dashboard/plugins/hub,
and the other _require_token routes permanently unreachable behind the gate.
In the UI this surfaced as a 401: {"detail":"Unauthorized"} popup on plugin
install for any publicly-bound (e.g. Fly-hosted NAS) dashboard.

Fix: _require_token now defers to the active gate. When auth_required is True it
accepts the request iff the gate attached a verified session (and 401s otherwise);
loopback/--insecure behavior is unchanged (still validates the session token).

Adds two regression tests driving the full in-process stub OAuth round trip:
the install endpoint must NOT 401 a logged-in request, and must still 401 with
no cookie. Verified the accept-test fails on the pre-fix code.

* test(dashboard): cover the whole _require_token route class under the gate

The install popup was one symptom of a class-wide bug: all 14 endpoints that
call _require_token directly (API-key reveal, provider validation, the
OAuth-provider connect/disconnect flow, and plugin enable/disable/update/
delete/visibility/providers) 401'd cookie-authenticated requests in gated mode.

Add a parametrized test hitting a representative spread (plugins/hub, env/reveal,
providers/validate, an oauth provider route, agent-plugin enable) asserting a
logged-in caller is never 401'd — proving the fix covers the class, not just
agent-plugins/install.
This commit is contained in:
Ben Barclay
2026-06-09 22:57:49 -07:00
committed by GitHub
parent e4a1b35a39
commit 63a421d4c0
2 changed files with 130 additions and 1 deletions
+25 -1
View File
@@ -246,7 +246,31 @@ def _has_valid_session_token(request: Request) -> bool:
def _require_token(request: Request) -> None:
"""Validate the ephemeral session token. Raises 401 on mismatch."""
"""Authorize a sensitive endpoint, raising 401 if the caller isn't allowed.
Two auth schemes protect the dashboard, exactly one active per bind:
* **Loopback / ``--insecure`` mode** (``auth_required`` False): the
ephemeral ``_SESSION_TOKEN`` is injected into the SPA HTML and echoed
back via ``X-Hermes-Session-Token`` (or the legacy ``Bearer`` header).
Validate it here.
* **Gated / OAuth mode** (``auth_required`` True): ``_SESSION_TOKEN`` is
NOT injected (the SPA authenticates with a session cookie), so there is
no token to check. The ``gated_auth_middleware`` has already verified the
cookie before the request reached this handler any non-public ``/api/``
route it lets through carries a verified ``request.state.session``. The
legacy ``auth_middleware`` likewise short-circuits in this mode. Requiring
the (absent) token here would 401 every cookie-authenticated request,
making plugin install/enable/disable and the other ``_require_token``
endpoints permanently unreachable behind the gate. Defer to the gate.
"""
if getattr(request.app.state, "auth_required", False):
# Gate is authoritative. It attaches ``request.state.session`` on
# success and 401s otherwise, so a request that reached us is already
# authenticated. Belt-and-braces: confirm the session is present.
if getattr(request.state, "session", None) is not None:
return
raise HTTPException(status_code=401, detail="Unauthorized")
if not _has_valid_session_token(request):
raise HTTPException(status_code=401, detail="Unauthorized")