Compare commits

...

2 Commits

Author SHA1 Message Date
Ben
ffce3fc63d 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.
2026-06-09 13:46:39 +10:00
Ben
a10b9b9b54 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.
2026-06-09 13:35:52 +10:00
2 changed files with 130 additions and 1 deletions

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")

View File

@@ -191,6 +191,111 @@ def test_full_login_round_trip_unlocks_gated_api(gated_app):
)
def _complete_stub_login(client) -> None:
"""Walk the stub OAuth round trip so ``client`` carries a valid session.
TestClient persists Set-Cookie across calls, so after this returns the
client's cookie jar holds ``hermes_session_at`` / ``hermes_session_rt``
and subsequent gated requests authenticate.
"""
r1 = client.get("/auth/login?provider=stub", follow_redirects=False)
assert r1.status_code == 302
state = r1.headers["location"].split("state=")[1]
r2 = client.get(
f"/auth/callback?code=stub_code&state={state}",
follow_redirects=False,
)
assert r2.status_code == 302
def test_gated_require_token_endpoint_accepts_cookie_session(gated_app):
"""Regression: ``_require_token`` endpoints must work under the OAuth gate.
In gated mode the legacy ``_SESSION_TOKEN`` is NOT injected into the SPA
(it authenticates with the session cookie). Endpoints that call
``_require_token`` directly — plugin install/enable/disable,
``/api/dashboard/plugins/hub``, and others — used to re-check the absent
token and 401 every cookie-authenticated request, making them permanently
unreachable behind the gate (the dashboard surfaced a
``401: {"detail":"Unauthorized"}`` popup on plugin install). The fix makes
``_require_token`` defer to the gate, which has already verified the cookie
and attached ``request.state.session`` before the handler runs.
We POST a deliberately invalid plugin identifier: a passing auth layer
lets the request reach the handler, which rejects the identifier with a
400. The assertion is simply "not 401" — proving auth succeeded without
coupling to the validation message.
"""
_complete_stub_login(gated_app)
r = gated_app.post(
"/api/dashboard/agent-plugins/install",
json={"identifier": "definitely not a valid identifier",
"force": False, "enable": False},
)
assert r.status_code != 401, (
"A _require_token endpoint 401'd a cookie-authenticated request under "
f"the OAuth gate (the install-popup bug). Body: {r.text}"
)
# And specifically: it reached the handler's own validation.
assert r.status_code == 400, (
f"Expected the install handler's 400 (bad identifier), got "
f"{r.status_code}: {r.text}"
)
def test_gated_require_token_endpoint_still_rejects_no_cookie(gated_app):
"""The gate must still 401 a ``_require_token`` endpoint with no session.
The fix defers to the gate — it does not make these endpoints public. A
request with no cookie is rejected by ``gated_auth_middleware`` before the
handler runs, so the install endpoint stays protected.
"""
r = gated_app.post(
"/api/dashboard/agent-plugins/install",
json={"identifier": "owner/repo", "force": False, "enable": False},
)
assert r.status_code == 401, (
f"Expected 401 for an unauthenticated install POST under the gate, "
f"got {r.status_code}: {r.text}"
)
# A representative spread of the OTHER ``_require_token`` endpoints (there are
# 14 in total). The install popup was just the reported symptom; the same bug
# made API-key reveal, provider validation, the OAuth-provider connect flow,
# and the rest of plugin management unreachable behind the gate. Each entry is
# (method, path, json_body); we assert only that a logged-in request is NOT
# 401'd — i.e. it cleared the auth layer and reached the handler. The
# handler's own status (400/404/429/etc.) is route-specific and not asserted.
_GATED_REQUIRE_TOKEN_ROUTES = [
("get", "/api/dashboard/plugins/hub", None),
("post", "/api/env/reveal", {"key": "NONEXISTENT_ENV_VAR_FOR_TEST"}),
("post", "/api/providers/validate", {"key": "OPENAI_API_KEY", "value": ""}),
("delete", "/api/providers/oauth/__not_a_real_provider__", None),
("post", "/api/dashboard/agent-plugins/__nope__/enable", None),
]
@pytest.mark.parametrize("method,path,body", _GATED_REQUIRE_TOKEN_ROUTES)
def test_gated_require_token_routes_accept_cookie_session(
gated_app, method, path, body
):
"""Every ``_require_token`` route must clear auth for a logged-in caller.
Same root cause and fix as
``test_gated_require_token_endpoint_accepts_cookie_session`` — this just
proves the fix covers the whole class, not only ``agent-plugins/install``.
"""
_complete_stub_login(gated_app)
kwargs = {"json": body} if body is not None else {}
r = gated_app.request(method.upper(), path, **kwargs)
assert r.status_code != 401, (
f"{method.upper()} {path} 401'd a cookie-authenticated request under "
f"the OAuth gate — _require_token still rejecting a valid session. "
f"Body: {r.text}"
)
def test_login_unknown_provider_returns_404(gated_app):
r = gated_app.get("/auth/login?provider=nonexistent", follow_redirects=False)
assert r.status_code == 404