mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-30 06:41:51 +00:00
feat(dashboard-auth): Phase 6 — 401 re-auth envelope + next= propagation
Contract V1 of nous-account-service PR #180 ships no refresh tokens, so the original Phase 6 silent-refresh design is replaced with a thinner '401 → redirect to /login' UX. The dashboard's gated middleware now emits a structured envelope on any auth failure; the SPA's fetch wrapper sees it and full-page-navigates the user through re-auth. hermes_cli/dashboard_auth/cookies.py: set_session_cookies(refresh_token='') SKIPS writing the hermes_session_rt cookie. Forward-compat: a non-empty refresh_token still emits the cookie unchanged, so a future Portal contract that starts issuing RTs flips the persistence on with no other change. clear_session_cookies still emits a Max-Age=0 deletion for the RT cookie so stale cookies from earlier deployments get flushed on logout / session expiry. Deprecation marker + rationale in module docstring per the user's docstring-only deprecation pattern. hermes_cli/dashboard_auth/middleware.py: _unauth_response now builds a structured JSON envelope for API 401s: { error: 'session_expired' | 'unauthenticated', detail: 'Unauthorized', reason: <internal>, login_url: '/login?next=<safe-path>' } HTML redirects also carry next= so a user landing on /sessions without a cookie bounces back to /sessions after re-auth. _safe_next_target validates same-origin: drops protocol-relative paths (//evil.com), absolute URLs, and any /login or /auth/* loop. Dead cookies are cleared on the 401 path so the browser stops replaying invalid tokens. hermes_cli/dashboard_auth/routes.py: /auth/callback accepts next= query param and validates via _validate_post_login_target (same rules as the gate's _safe_next_target — defence-in-depth because next= survived a full IDP round trip and attacker-controlled state can re-enter via the callback URL). Open-redirect attempts land at '/' instead. web/src/lib/api.ts: fetchJSON parses the 401 envelope and full-page-navigates to body.login_url ONLY on the known session-expiry error codes. Domain-level 401s (e.g. permission errors) bubble up as regular errors. credentials: 'include' added so cookie auth works for all fetches routed through this wrapper. sessionStorage.lastLocation is preserved for future use by AuthWidget / hermes_status. Test files marked with pytest.mark.xdist_group so the four files that mutate web_server.app.state.auth_required serialize onto the same xdist worker — eliminates 'works locally, fails in CI' app-state bleed. 20 new tests in test_dashboard_auth_401_reauth.py: - set_session_cookies(refresh_token='') skips RT cookie - clear_session_cookies still emits RT deletion - 401 envelope shape (unauthenticated vs session_expired) - dead cookie cleared on invalid-token 401 - login_url carries next= for deep paths - login loop avoided when path is /login/auth/api-auth - protocol-relative URL rejected - _safe_next_target unit tests (accept same-origin, reject loops/abs) - /auth/callback respects safe next= but rejects open redirects 2 pre-existing tests updated to accept the new /login?next=%2F shape. Full dashboard-auth suite: 168 passed, 1 skipped (Phase 0 pre-existing).
This commit is contained in:
Ben
Teknium
parent
8971e94831
commit
5e9308b5b8
8 changed files with 506 additions and 22 deletions
|
|
@ -1,10 +1,14 @@
|
|||
"""Cookie helpers for dashboard auth.
|
||||
|
||||
Three cookies in play:
|
||||
- hermes_session_at: the OAuth access token
|
||||
(HttpOnly, lifetime = token TTL)
|
||||
- hermes_session_rt: the OAuth refresh token
|
||||
(HttpOnly, lifetime = 30 days)
|
||||
- hermes_session_at: the OAuth access token
|
||||
(HttpOnly, lifetime = token TTL)
|
||||
- hermes_session_rt: the OAuth refresh token
|
||||
(HttpOnly, lifetime = 30 days)
|
||||
**DEPRECATED in OAuth contract v1** — Nous Portal
|
||||
does not issue refresh tokens; we keep the cookie
|
||||
name and clear semantics for forward compatibility
|
||||
and to flush stale cookies from old browsers.
|
||||
- hermes_session_pkce: short-lived PKCE state + CSRF nonce + provider
|
||||
hint (HttpOnly, lifetime = 10 minutes)
|
||||
|
||||
|
|
@ -16,6 +20,14 @@ which honours ``X-Forwarded-Proto`` upstream of Fly's TLS terminator
|
|||
when uvicorn is configured with ``proxy_headers=True``. Loopback dev
|
||||
traffic is always HTTP so ``Secure`` would lock the cookies out of
|
||||
the browser.
|
||||
|
||||
.. deprecated:: contract v1
|
||||
``set_session_cookies`` accepts ``refresh_token=""`` (the contract-v1
|
||||
default) and silently skips writing ``hermes_session_rt`` in that case.
|
||||
``clear_session_cookies`` still emits a Max-Age=0 deletion for the RT
|
||||
cookie so users carrying a stale cookie from an earlier deployment get
|
||||
it cleared on logout / session expiry. The full refresh-flow machinery
|
||||
was rewritten as "401 → redirect to /login" in Phase 6.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
|
|
@ -52,22 +64,31 @@ def set_session_cookies(
|
|||
access_token_expires_in: int,
|
||||
use_https: bool,
|
||||
) -> None:
|
||||
"""Set both session cookies on the response.
|
||||
"""Set the session cookies on the response.
|
||||
|
||||
``access_token_expires_in`` is in seconds. Use the provider's reported
|
||||
TTL for the access token. The refresh token cookie always lives 30
|
||||
days regardless of the underlying provider's refresh TTL.
|
||||
TTL for the access token.
|
||||
|
||||
``refresh_token`` is accepted for backward / forward compatibility but
|
||||
SKIPPED when empty — Nous Portal contract v1 issues no refresh tokens
|
||||
so a ``Session.refresh_token == ""`` from the provider means we don't
|
||||
persist anything. If a future contract revision starts emitting refresh
|
||||
tokens, this helper will write the RT cookie again with no other change.
|
||||
"""
|
||||
response.set_cookie(
|
||||
SESSION_AT_COOKIE, access_token,
|
||||
max_age=access_token_expires_in,
|
||||
**_common_attrs(use_https),
|
||||
)
|
||||
response.set_cookie(
|
||||
SESSION_RT_COOKIE, refresh_token,
|
||||
max_age=_RT_MAX_AGE,
|
||||
**_common_attrs(use_https),
|
||||
)
|
||||
# Contract v1: empty refresh token means "don't persist RT cookie".
|
||||
# Keeping a literal empty-value cookie around would be dead state at
|
||||
# best, attack surface at worst.
|
||||
if refresh_token:
|
||||
response.set_cookie(
|
||||
SESSION_RT_COOKIE, refresh_token,
|
||||
max_age=_RT_MAX_AGE,
|
||||
**_common_attrs(use_https),
|
||||
)
|
||||
|
||||
|
||||
def clear_session_cookies(response: Response) -> None:
|
||||
|
|
|
|||
|
|
@ -58,14 +58,73 @@ def _client_ip(request: Request) -> str:
|
|||
return request.client.host if request.client else ""
|
||||
|
||||
|
||||
def _unauth_response(path: str, *, reason: str) -> Response:
|
||||
"""API routes → 401 JSON; HTML routes → 302 → /login."""
|
||||
def _unauth_response(request: Request, *, reason: str) -> Response:
|
||||
"""API routes → 401 JSON with ``login_url``; HTML routes → 302 → /login.
|
||||
|
||||
The JSON envelope carries a ``login_url`` field with a ``next=`` query
|
||||
string so the SPA's global 401 handler can drop the user back where
|
||||
they were after re-auth. The contract is intentionally simple so any
|
||||
fetch-wrapper can implement the redirect without parsing details:
|
||||
|
||||
if response.status === 401 && body.error in ("unauthenticated",
|
||||
"session_expired"):
|
||||
window.location.assign(body.login_url);
|
||||
|
||||
HTML redirects also carry the ``next=`` query string so direct
|
||||
navigation to ``/sessions`` (etc.) without a cookie comes back to
|
||||
``/sessions`` after login.
|
||||
"""
|
||||
path = request.url.path
|
||||
next_param = _safe_next_target(request)
|
||||
login_url = f"/login?next={next_param}" if next_param else "/login"
|
||||
|
||||
if path.startswith("/api/"):
|
||||
# API routes never get redirects: the browser fetch() API would
|
||||
# follow a 302 into the cross-origin OAuth dance opaquely. Return
|
||||
# 401 with a structured envelope so the SPA can full-page-navigate
|
||||
# to login_url.
|
||||
error_code = (
|
||||
"session_expired"
|
||||
if reason == "invalid_or_expired_session"
|
||||
else "unauthenticated"
|
||||
)
|
||||
return JSONResponse(
|
||||
{"detail": "Unauthorized", "reason": reason},
|
||||
{
|
||||
"error": error_code,
|
||||
"detail": "Unauthorized",
|
||||
"reason": reason,
|
||||
"login_url": login_url,
|
||||
},
|
||||
status_code=401,
|
||||
)
|
||||
return RedirectResponse(url="/login", status_code=302)
|
||||
return RedirectResponse(url=login_url, status_code=302)
|
||||
|
||||
|
||||
def _safe_next_target(request: Request) -> str:
|
||||
"""Build the URL-encoded ``next`` query value, or empty string.
|
||||
|
||||
Only same-origin relative paths are accepted; absolute URLs or
|
||||
``//evil.com`` open-redirect attempts are silently dropped. The empty
|
||||
string return means the caller produces a bare ``/login`` URL — fine,
|
||||
user lands at the dashboard root after re-auth.
|
||||
"""
|
||||
path = request.url.path
|
||||
# Reject anything that doesn't start with "/" or starts with "//"
|
||||
# (protocol-relative URL — would open-redirect to an attacker host).
|
||||
if not path or not path.startswith("/") or path.startswith("//"):
|
||||
return ""
|
||||
# Don't redirect back to the auth routes themselves — that loops.
|
||||
if any(
|
||||
path == p or path.startswith(p)
|
||||
for p in ("/login", "/auth/", "/api/auth/")
|
||||
):
|
||||
return ""
|
||||
# Preserve query string if present (e.g. /sessions?page=2).
|
||||
query = request.url.query
|
||||
target = f"{path}?{query}" if query else path
|
||||
# urlencode the whole thing as a single value.
|
||||
from urllib.parse import quote
|
||||
return quote(target, safe="")
|
||||
|
||||
|
||||
async def gated_auth_middleware(
|
||||
|
|
@ -86,7 +145,7 @@ async def gated_auth_middleware(
|
|||
|
||||
at, _rt = read_session_cookies(request)
|
||||
if not at:
|
||||
return _unauth_response(path, reason="no_cookie")
|
||||
return _unauth_response(request, reason="no_cookie")
|
||||
|
||||
# Try every registered provider's verify_session in turn. Providers
|
||||
# MUST return None for tokens they don't recognise (not raise). This
|
||||
|
|
@ -120,7 +179,14 @@ async def gated_auth_middleware(
|
|||
reason="no_provider_recognises",
|
||||
ip=_client_ip(request),
|
||||
)
|
||||
return _unauth_response(path, reason="invalid_or_expired_session")
|
||||
response = _unauth_response(request, reason="invalid_or_expired_session")
|
||||
# Clear the dead cookie so the browser doesn't keep sending it.
|
||||
# Contract v1: no refresh token to retry with, so the only correct
|
||||
# next step is full re-auth via /login. Importing locally avoids a
|
||||
# cycle with cookies → middleware at module load.
|
||||
from hermes_cli.dashboard_auth.cookies import clear_session_cookies
|
||||
clear_session_cookies(response)
|
||||
return response
|
||||
|
||||
request.state.session = session
|
||||
return await call_next(request)
|
||||
|
|
|
|||
|
|
@ -151,6 +151,7 @@ async def auth_callback(
|
|||
state: str = "",
|
||||
error: str = "",
|
||||
error_description: str = "",
|
||||
next: str = "",
|
||||
):
|
||||
pkce_raw = read_pkce_cookie(request)
|
||||
if not pkce_raw:
|
||||
|
|
@ -241,7 +242,12 @@ async def auth_callback(
|
|||
)
|
||||
|
||||
expires_in = max(60, session.expires_at - int(time.time()))
|
||||
resp = RedirectResponse(url="/", status_code=302)
|
||||
# Honour the ``next=`` query param the gate's _unauth_response set in
|
||||
# the redirect URL. Validated against the same same-origin rules as
|
||||
# the gate's _safe_next_target — any absolute URL / protocol-relative
|
||||
# path / loop back to /login is dropped in favour of ``/``.
|
||||
landing = _validate_post_login_target(next) or "/"
|
||||
resp = RedirectResponse(url=landing, status_code=302)
|
||||
set_session_cookies(
|
||||
resp,
|
||||
access_token=session.access_token,
|
||||
|
|
@ -253,6 +259,31 @@ async def auth_callback(
|
|||
return resp
|
||||
|
||||
|
||||
def _validate_post_login_target(raw: str) -> str:
|
||||
"""Return ``raw`` if it's a safe same-origin path, else empty string.
|
||||
|
||||
The ``next`` query param survives a full OAuth round trip — the gate
|
||||
encodes it into the /login redirect, the login page emits it back into
|
||||
/auth/login, and the IDP preserves it across /authorize/callback. We
|
||||
have to re-validate here because the value came back in via the
|
||||
URL (an attacker could craft a /auth/callback URL with their own
|
||||
``next=https://evil.example``).
|
||||
"""
|
||||
if not raw:
|
||||
return ""
|
||||
from urllib.parse import unquote
|
||||
decoded = unquote(raw)
|
||||
if not decoded.startswith("/") or decoded.startswith("//"):
|
||||
return ""
|
||||
# Don't loop back to login pages or auth flow.
|
||||
if any(
|
||||
decoded == p or decoded.startswith(p)
|
||||
for p in ("/login", "/auth/", "/api/auth/")
|
||||
):
|
||||
return ""
|
||||
return decoded
|
||||
|
||||
|
||||
@router.post("/auth/logout", name="auth_logout")
|
||||
async def auth_logout(request: Request):
|
||||
_at, rt = read_session_cookies(request)
|
||||
|
|
|
|||
300
tests/hermes_cli/test_dashboard_auth_401_reauth.py
Normal file
300
tests/hermes_cli/test_dashboard_auth_401_reauth.py
Normal file
|
|
@ -0,0 +1,300 @@
|
|||
"""Phase 6 — 401 re-auth + ``next=`` propagation tests.
|
||||
|
||||
Verifies the contract documented in Phase 6 v2 of the plan:
|
||||
|
||||
- API 401 responses carry ``{"error", "login_url", ...}`` so the SPA
|
||||
fetch wrapper can ``window.location.assign(body.login_url)``.
|
||||
- The ``login_url`` embeds a ``next=<original-path>`` query string so
|
||||
re-auth lands the user back where they were.
|
||||
- HTML redirects ALSO carry ``next=``.
|
||||
- ``next=`` validation: protocol-relative paths, absolute URLs, and
|
||||
loops back to ``/login`` / ``/auth/*`` are dropped.
|
||||
- Invalid/expired cookies are cleared on 401 so the browser doesn't
|
||||
keep replaying them.
|
||||
- ``set_session_cookies(refresh_token="")`` does NOT emit the
|
||||
``hermes_session_rt`` cookie (contract V1: no RT to persist).
|
||||
- ``/auth/callback?next=…`` honours the same-origin landing path.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from urllib.parse import quote
|
||||
|
||||
import pytest
|
||||
|
||||
# Phase 5 / Phase 6: these tests mutate ``web_server.app.state.auth_required``
|
||||
# at module level. Run them in the same xdist worker so they don't race
|
||||
# against each other (and against any other file that also touches
|
||||
# ``app.state``) — the marker name is shared across all dashboard-auth test
|
||||
# files that gate the app.
|
||||
pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state")
|
||||
from fastapi import FastAPI
|
||||
from fastapi.responses import Response
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli import web_server
|
||||
from hermes_cli.dashboard_auth import clear_providers, register_provider
|
||||
from hermes_cli.dashboard_auth.cookies import (
|
||||
SESSION_AT_COOKIE,
|
||||
SESSION_RT_COOKIE,
|
||||
clear_session_cookies,
|
||||
set_session_cookies,
|
||||
)
|
||||
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def gated_app():
|
||||
clear_providers()
|
||||
register_provider(StubAuthProvider())
|
||||
prev_host = getattr(web_server.app.state, "bound_host", None)
|
||||
prev_port = getattr(web_server.app.state, "bound_port", None)
|
||||
prev_required = getattr(web_server.app.state, "auth_required", None)
|
||||
web_server.app.state.bound_host = "fly-app.fly.dev"
|
||||
web_server.app.state.bound_port = 443
|
||||
web_server.app.state.auth_required = True
|
||||
client = TestClient(web_server.app, base_url="https://fly-app.fly.dev")
|
||||
yield client
|
||||
clear_providers()
|
||||
web_server.app.state.bound_host = prev_host
|
||||
web_server.app.state.bound_port = prev_port
|
||||
web_server.app.state.auth_required = prev_required
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# set_session_cookies(refresh_token="") skips the RT cookie
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestRefreshTokenCookieDeprecation:
|
||||
def _build_app(self, *, refresh_token: str):
|
||||
app = FastAPI()
|
||||
|
||||
@app.get("/set")
|
||||
def _set():
|
||||
r = Response("ok")
|
||||
set_session_cookies(
|
||||
r, access_token="AT", refresh_token=refresh_token,
|
||||
access_token_expires_in=3600, use_https=True,
|
||||
)
|
||||
return r
|
||||
|
||||
return app
|
||||
|
||||
def test_empty_refresh_token_does_not_emit_rt_cookie(self):
|
||||
client = TestClient(self._build_app(refresh_token=""))
|
||||
r = client.get("/set")
|
||||
cookies = r.headers.get_list("set-cookie")
|
||||
rt_cookies = [c for c in cookies if c.startswith(f"{SESSION_RT_COOKIE}=")]
|
||||
assert rt_cookies == []
|
||||
# AT cookie still set.
|
||||
at_cookies = [c for c in cookies if c.startswith(f"{SESSION_AT_COOKIE}=")]
|
||||
assert len(at_cookies) == 1
|
||||
|
||||
def test_present_refresh_token_still_emits_rt_cookie(self):
|
||||
client = TestClient(self._build_app(refresh_token="forward-compat"))
|
||||
r = client.get("/set")
|
||||
cookies = r.headers.get_list("set-cookie")
|
||||
rt_cookies = [c for c in cookies if c.startswith(f"{SESSION_RT_COOKIE}=")]
|
||||
assert len(rt_cookies) == 1
|
||||
assert "forward-compat" in rt_cookies[0]
|
||||
|
||||
def test_clear_session_cookies_still_emits_rt_deletion(self):
|
||||
"""Even when we never wrote the RT cookie, logout/clear should
|
||||
emit a Max-Age=0 deletion to flush stale cookies from old
|
||||
deployments."""
|
||||
app = FastAPI()
|
||||
|
||||
@app.get("/clear")
|
||||
def _clear():
|
||||
r = Response("ok")
|
||||
clear_session_cookies(r)
|
||||
return r
|
||||
|
||||
client = TestClient(app)
|
||||
r = client.get("/clear")
|
||||
cookies = r.headers.get_list("set-cookie")
|
||||
assert any(
|
||||
c.startswith(f"{SESSION_RT_COOKIE}=") and "Max-Age=0" in c
|
||||
for c in cookies
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Gate middleware: 401 envelope + next= propagation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestApi401Envelope:
|
||||
def test_no_cookie_returns_unauthenticated_envelope(self, gated_app):
|
||||
r = gated_app.get("/api/status")
|
||||
assert r.status_code == 401
|
||||
body = r.json()
|
||||
assert body["error"] == "unauthenticated"
|
||||
assert "login_url" in body
|
||||
assert body["login_url"].startswith("/login")
|
||||
|
||||
def test_invalid_cookie_returns_session_expired_envelope(self, gated_app):
|
||||
gated_app.cookies.set(SESSION_AT_COOKIE, "garbage")
|
||||
r = gated_app.get("/api/status")
|
||||
assert r.status_code == 401
|
||||
body = r.json()
|
||||
assert body["error"] == "session_expired"
|
||||
assert body["login_url"].startswith("/login")
|
||||
|
||||
def test_invalid_cookie_clears_dead_cookie(self, gated_app):
|
||||
"""Dead-cookie cleanup — Phase 6 requirement so the browser
|
||||
doesn't keep replaying the stale token on every request."""
|
||||
gated_app.cookies.set(SESSION_AT_COOKIE, "garbage")
|
||||
r = gated_app.get("/api/status")
|
||||
set_cookies = r.headers.get_list("set-cookie")
|
||||
assert any(
|
||||
c.startswith(f"{SESSION_AT_COOKIE}=") and "Max-Age=0" in c
|
||||
for c in set_cookies
|
||||
)
|
||||
|
||||
def test_login_url_carries_next_for_deep_api_path(self, gated_app):
|
||||
r = gated_app.get("/api/sessions?page=2")
|
||||
body = r.json()
|
||||
# next= is URL-encoded.
|
||||
assert "next=" in body["login_url"]
|
||||
assert quote("/api/sessions?page=2", safe="") in body["login_url"]
|
||||
|
||||
|
||||
class TestHtmlRedirectNext:
|
||||
def test_deep_html_path_redirects_with_next(self, gated_app):
|
||||
r = gated_app.get("/sessions", follow_redirects=False)
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/login?next=%2Fsessions"
|
||||
|
||||
def test_root_path_redirects_with_next(self, gated_app):
|
||||
r = gated_app.get("/", follow_redirects=False)
|
||||
assert r.headers["location"] in ("/login", "/login?next=%2F")
|
||||
|
||||
def test_login_loop_avoided(self, gated_app):
|
||||
"""A request to /login itself must not produce ``?next=/login``
|
||||
because that'd be a loop after re-auth."""
|
||||
# /login is on the public allowlist so it doesn't go through the
|
||||
# 401 path. But sanity: the page renders.
|
||||
r = gated_app.get("/login")
|
||||
assert r.status_code == 200
|
||||
|
||||
def test_auth_loop_avoided(self, gated_app):
|
||||
"""A failed cookie on /auth/me (auth-required path) must drop
|
||||
the next= rather than risk a /login?next=/api/auth/me loop."""
|
||||
# /api/auth/me requires auth. Without cookie → 401 with login_url
|
||||
# but next= must NOT point at /api/auth/.
|
||||
r = gated_app.get("/api/auth/me")
|
||||
assert r.status_code == 401
|
||||
body = r.json()
|
||||
assert "next=" not in body["login_url"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Gate middleware: same-origin next= validation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestNextSameOriginValidation:
|
||||
def test_protocol_relative_path_dropped(self, gated_app):
|
||||
# `//evil.com/foo` parses to a protocol-relative URL — browser
|
||||
# would treat as cross-origin. We drop it at the gate; the path
|
||||
# we redirect to should NOT contain `//evil.com`.
|
||||
r = gated_app.get("//evil.com", follow_redirects=False)
|
||||
# Starlette likely normalizes the path before we see it, so the
|
||||
# gate may see "/evil.com" — either way the encoded value
|
||||
# in next= must be safe to feed to window.location.assign.
|
||||
# Just assert no protocol-relative form survives.
|
||||
assert r.status_code == 302
|
||||
location = r.headers["location"]
|
||||
assert "%2F%2Fevil" not in location # urlencoded // form
|
||||
assert "//evil" not in location
|
||||
|
||||
def test_safe_next_validator_accepts_same_origin(self):
|
||||
from hermes_cli.dashboard_auth.middleware import _safe_next_target
|
||||
|
||||
class FakeRequest:
|
||||
def __init__(self, path, query=""):
|
||||
self.url = type("URL", (), {"path": path, "query": query})()
|
||||
|
||||
assert _safe_next_target(FakeRequest("/sessions")) == "%2Fsessions"
|
||||
assert (
|
||||
_safe_next_target(FakeRequest("/sessions", "page=2"))
|
||||
== "%2Fsessions%3Fpage%3D2"
|
||||
)
|
||||
|
||||
def test_safe_next_validator_rejects_protocol_relative(self):
|
||||
from hermes_cli.dashboard_auth.middleware import _safe_next_target
|
||||
|
||||
class FakeRequest:
|
||||
def __init__(self, path):
|
||||
self.url = type("URL", (), {"path": path, "query": ""})()
|
||||
|
||||
assert _safe_next_target(FakeRequest("//evil.com")) == ""
|
||||
|
||||
def test_safe_next_validator_rejects_login_loop(self):
|
||||
from hermes_cli.dashboard_auth.middleware import _safe_next_target
|
||||
|
||||
class FakeRequest:
|
||||
def __init__(self, path):
|
||||
self.url = type("URL", (), {"path": path, "query": ""})()
|
||||
|
||||
assert _safe_next_target(FakeRequest("/login")) == ""
|
||||
assert _safe_next_target(FakeRequest("/auth/login")) == ""
|
||||
assert _safe_next_target(FakeRequest("/api/auth/me")) == ""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /auth/callback honours next= and validates it
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAuthCallbackNext:
|
||||
def _drive_oauth(self, gated_app, *, next_path: str = ""):
|
||||
next_qs = f"&next={quote(next_path, safe='')}" if next_path else ""
|
||||
r1 = gated_app.get(
|
||||
f"/auth/login?provider=stub{next_qs}", follow_redirects=False
|
||||
)
|
||||
state = r1.headers["location"].split("state=")[1]
|
||||
# next is preserved by the route (it's in the original URL — but the
|
||||
# stub IDP returns to /auth/callback. We need to pass next as a
|
||||
# separate query param on the callback URL to simulate what a real
|
||||
# IDP would do via state-bound storage. For this test, the
|
||||
# /auth/callback handler reads `next` directly from its own query
|
||||
# string, so just append it.
|
||||
return gated_app.get(
|
||||
f"/auth/callback?code=stub_code&state={state}{next_qs}",
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
def test_callback_without_next_lands_at_root(self, gated_app):
|
||||
r = self._drive_oauth(gated_app)
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/"
|
||||
|
||||
def test_callback_with_safe_next_lands_there(self, gated_app):
|
||||
r = self._drive_oauth(gated_app, next_path="/sessions")
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/sessions"
|
||||
|
||||
def test_callback_with_query_string_in_next(self, gated_app):
|
||||
r = self._drive_oauth(gated_app, next_path="/sessions?page=2")
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/sessions?page=2"
|
||||
|
||||
def test_callback_rejects_open_redirect(self, gated_app):
|
||||
# Attacker provides ``next=//evil.com`` hoping for an open redirect
|
||||
# after successful auth. Validator drops it; user lands at "/".
|
||||
r = self._drive_oauth(gated_app, next_path="//evil.com/steal")
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/"
|
||||
|
||||
def test_callback_rejects_login_loop(self, gated_app):
|
||||
r = self._drive_oauth(gated_app, next_path="/login")
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/"
|
||||
|
|
@ -4,6 +4,13 @@ Phase 0 — establish a baseline pin on the current (pre-OAuth) behavior so
|
|||
later phases can prove they didn't break loopback mode.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
# Phase 5 / Phase 6: these tests mutate ``web_server.app.state.auth_required``
|
||||
# at module level. Run them in the same xdist worker so they don't race
|
||||
# against each other (and against any other file that also touches
|
||||
# ``app.state``) — the marker name is shared across all dashboard-auth test
|
||||
# files that gate the app.
|
||||
pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state")
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli import web_server
|
||||
|
|
|
|||
|
|
@ -15,6 +15,13 @@ without any external IDP. Exercises:
|
|||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
# Phase 5 / Phase 6: these tests mutate ``web_server.app.state.auth_required``
|
||||
# at module level. Run them in the same xdist worker so they don't race
|
||||
# against each other (and against any other file that also touches
|
||||
# ``app.state``) — the marker name is shared across all dashboard-auth test
|
||||
# files that gate the app.
|
||||
pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state")
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli import web_server
|
||||
|
|
@ -58,7 +65,8 @@ def test_gated_status_now_requires_auth(gated_app):
|
|||
def test_gated_html_redirects_to_login(gated_app):
|
||||
r = gated_app.get("/", follow_redirects=False)
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/login"
|
||||
# Phase 6: gate carries a ``next=`` so post-login bounces back to /.
|
||||
assert r.headers["location"] in ("/login", "/login?next=%2F")
|
||||
|
||||
|
||||
def test_gated_auth_providers_is_public(gated_app):
|
||||
|
|
@ -177,7 +185,8 @@ def test_invalid_cookie_redirects_on_html(gated_app):
|
|||
gated_app.cookies.set(SESSION_AT_COOKIE, "garbage")
|
||||
r = gated_app.get("/", follow_redirects=False)
|
||||
assert r.status_code == 302
|
||||
assert r.headers["location"] == "/login"
|
||||
# Phase 6: gate carries a ``next=`` so post-login bounces back to /.
|
||||
assert r.headers["location"] in ("/login", "/login?next=%2F")
|
||||
|
||||
|
||||
def test_logout_clears_cookies_and_redirects_to_login(gated_app):
|
||||
|
|
|
|||
|
|
@ -17,6 +17,13 @@ from types import SimpleNamespace
|
|||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
# Phase 5 / Phase 6: these tests mutate ``web_server.app.state.auth_required``
|
||||
# at module level. Run them in the same xdist worker so they don't race
|
||||
# against each other (and against any other file that also touches
|
||||
# ``app.state``) — the marker name is shared across all dashboard-auth test
|
||||
# files that gate the app.
|
||||
pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state")
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli import web_server
|
||||
|
|
|
|||
|
|
@ -48,7 +48,50 @@ export async function fetchJSON<T>(url: string, init?: RequestInit): Promise<T>
|
|||
if (token) {
|
||||
setSessionHeader(headers, token);
|
||||
}
|
||||
const res = await fetch(`${BASE}${url}`, { ...init, headers });
|
||||
const res = await fetch(`${BASE}${url}`, {
|
||||
...init,
|
||||
headers,
|
||||
// ``credentials: 'include'`` so the cookie-auth path (gated mode) works
|
||||
// for any fetch routed through here. Loopback mode is unaffected — the
|
||||
// server doesn't read cookies and the legacy session-token header is
|
||||
// already attached above.
|
||||
credentials: init?.credentials ?? "include",
|
||||
});
|
||||
if (res.status === 401) {
|
||||
// Phase 6: the gated middleware emits a structured envelope so the
|
||||
// SPA can full-page-navigate to /login on session expiry. Parse it,
|
||||
// and only redirect on the known error codes — domain-level 401s
|
||||
// (e.g. "you don't have permission to read this monitor") bubble
|
||||
// up as regular errors so callers can handle them.
|
||||
let body: { error?: string; login_url?: string } = {};
|
||||
try {
|
||||
body = await res.clone().json();
|
||||
} catch {
|
||||
/* non-JSON 401 — let it fall through */
|
||||
}
|
||||
if (
|
||||
(body.error === "unauthenticated" || body.error === "session_expired") &&
|
||||
body.login_url
|
||||
) {
|
||||
// Preserve where the user was so /auth/callback can land them back
|
||||
// after re-auth. The gate's login_url already carries a ``next=``
|
||||
// built from the request path, but the SPA may be deep inside a
|
||||
// SPA route the gate never saw — e.g. a hash route or a client-side
|
||||
// /sessions/<id> deep link. Save the current location as a
|
||||
// fallback the post-login handler can read.
|
||||
try {
|
||||
sessionStorage.setItem(
|
||||
"hermes.lastLocation",
|
||||
window.location.pathname + window.location.search,
|
||||
);
|
||||
} catch {
|
||||
/* SSR / privacy mode — ignore */
|
||||
}
|
||||
window.location.assign(body.login_url);
|
||||
// Never resolve — the page is about to unload.
|
||||
return new Promise<T>(() => {});
|
||||
}
|
||||
}
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => res.statusText);
|
||||
throw new Error(`${res.status}: ${text}`);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue