diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 08d0ac32bda..061a2e46816 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -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") diff --git a/tests/hermes_cli/test_dashboard_auth_middleware.py b/tests/hermes_cli/test_dashboard_auth_middleware.py index a7363a1db8a..2e1abd4cdf1 100644 --- a/tests/hermes_cli/test_dashboard_auth_middleware.py +++ b/tests/hermes_cli/test_dashboard_auth_middleware.py @@ -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