diff --git a/hermes_cli/dashboard_auth/cookies.py b/hermes_cli/dashboard_auth/cookies.py new file mode 100644 index 00000000000..29e6cf799b7 --- /dev/null +++ b/hermes_cli/dashboard_auth/cookies.py @@ -0,0 +1,120 @@ +"""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_pkce: short-lived PKCE state + CSRF nonce + provider + hint (HttpOnly, lifetime = 10 minutes) + +All three are ``SameSite=Lax`` (browser will send on cross-site GET +top-level navigation, which we need for the IDP redirect back to +``/auth/callback``) and ``Path=/``. ``Secure`` is set ONLY when the +dashboard was reached over HTTPS — detected via the request URL scheme, +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. +""" +from __future__ import annotations + +from typing import Optional, Tuple + +from fastapi import Request +from fastapi.responses import Response + +SESSION_AT_COOKIE = "hermes_session_at" +SESSION_RT_COOKIE = "hermes_session_rt" +PKCE_COOKIE = "hermes_session_pkce" + +# 30 days — matches Portal's REFRESH_TOKEN_TTL_SECONDS +_RT_MAX_AGE = 30 * 24 * 60 * 60 +_PKCE_MAX_AGE = 10 * 60 + + +def _common_attrs(use_https: bool) -> dict: + attrs: dict = { + "httponly": True, + "samesite": "lax", + "path": "/", + } + if use_https: + attrs["secure"] = True + return attrs + + +def set_session_cookies( + response: Response, + *, + access_token: str, + refresh_token: str, + access_token_expires_in: int, + use_https: bool, +) -> None: + """Set both 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. + """ + 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), + ) + + +def clear_session_cookies(response: Response) -> None: + """Emit Max-Age=0 deletions for both session cookies.""" + # Path must match the set-path for the delete to apply. + response.set_cookie( + SESSION_AT_COOKIE, "", max_age=0, + path="/", httponly=True, samesite="lax", + ) + response.set_cookie( + SESSION_RT_COOKIE, "", max_age=0, + path="/", httponly=True, samesite="lax", + ) + + +def set_pkce_cookie(response: Response, *, payload: str, use_https: bool) -> None: + response.set_cookie( + PKCE_COOKIE, payload, + max_age=_PKCE_MAX_AGE, + **_common_attrs(use_https), + ) + + +def clear_pkce_cookie(response: Response) -> None: + response.set_cookie( + PKCE_COOKIE, "", max_age=0, + path="/", httponly=True, samesite="lax", + ) + + +def read_session_cookies(request: Request) -> Tuple[Optional[str], Optional[str]]: + """Returns (access_token, refresh_token), either may be None.""" + at = request.cookies.get(SESSION_AT_COOKIE) + rt = request.cookies.get(SESSION_RT_COOKIE) + return at, rt + + +def read_pkce_cookie(request: Request) -> Optional[str]: + return request.cookies.get(PKCE_COOKIE) + + +def detect_https(request: Request) -> bool: + """Decide whether to set the ``Secure`` cookie flag. + + Reads ``request.url.scheme`` — under uvicorn's ``proxy_headers=True`` + (which start_server enables when the gate is active), this honours + ``X-Forwarded-Proto`` from Fly's TLS terminator. Loopback traffic is + always HTTP so this returns False there. + """ + return request.url.scheme == "https" diff --git a/tests/hermes_cli/test_dashboard_auth_cookies.py b/tests/hermes_cli/test_dashboard_auth_cookies.py new file mode 100644 index 00000000000..c13662d156d --- /dev/null +++ b/tests/hermes_cli/test_dashboard_auth_cookies.py @@ -0,0 +1,158 @@ +"""Tests for the dashboard-auth cookie helpers.""" +from __future__ import annotations + +import pytest +from fastapi import FastAPI +from fastapi.responses import Response +from fastapi.testclient import TestClient +from starlette.requests import Request + +from hermes_cli.dashboard_auth.cookies import ( + PKCE_COOKIE, + SESSION_AT_COOKIE, + SESSION_RT_COOKIE, + clear_pkce_cookie, + clear_session_cookies, + read_pkce_cookie, + read_session_cookies, + set_pkce_cookie, + set_session_cookies, +) + + +def _build_app(use_https: bool = True): + app = FastAPI() + + @app.get("/set") + def set_endpoint(): + r = Response("ok") + set_session_cookies( + r, access_token="AT", refresh_token="RT", + access_token_expires_in=3600, use_https=use_https, + ) + return r + + @app.get("/set-pkce") + def set_pkce(): + r = Response("ok") + set_pkce_cookie(r, payload="provider=stub;state=s;verifier=v", + use_https=use_https) + return r + + @app.get("/clear") + def clear(): + r = Response("ok") + clear_session_cookies(r) + clear_pkce_cookie(r) + return r + + return app + + +def test_session_cookies_are_httponly_samesite_lax_secure_in_https(): + client = TestClient(_build_app(use_https=True)) + r = client.get("/set") + cookies = r.headers.get_list("set-cookie") + at = next(c for c in cookies if c.startswith(f"{SESSION_AT_COOKIE}=")) + rt = next(c for c in cookies if c.startswith(f"{SESSION_RT_COOKIE}=")) + for c in (at, rt): + assert "HttpOnly" in c + assert "samesite=lax" in c.lower() + assert "Secure" in c + assert "Path=/" in c + + +def test_session_cookies_omit_secure_when_http(): + client = TestClient(_build_app(use_https=False)) + r = client.get("/set") + for c in r.headers.get_list("set-cookie"): + if c.startswith(f"{SESSION_AT_COOKIE}=") or c.startswith(f"{SESSION_RT_COOKIE}="): + assert "Secure" not in c, f"Cookie unexpectedly Secure: {c}" + + +def test_session_cookies_have_30day_rt_and_token_ttl_at(): + client = TestClient(_build_app(use_https=True)) + r = client.get("/set") + cookies = r.headers.get_list("set-cookie") + at = next(c for c in cookies if c.startswith(f"{SESSION_AT_COOKIE}=")) + rt = next(c for c in cookies if c.startswith(f"{SESSION_RT_COOKIE}=")) + assert "Max-Age=3600" in at + assert "Max-Age=2592000" in rt # 30 days = 30 * 86400 + + +def test_clear_session_cookies_emits_expired_at_and_rt(): + client = TestClient(_build_app()) + r = client.get("/clear") + cookies = r.headers.get_list("set-cookie") + assert any( + c.startswith(f"{SESSION_AT_COOKIE}=") and "Max-Age=0" in c + for c in cookies + ) + assert any( + c.startswith(f"{SESSION_RT_COOKIE}=") and "Max-Age=0" in c + for c in cookies + ) + + +def test_pkce_cookie_short_ttl_and_path_root(): + client = TestClient(_build_app(use_https=True)) + r = client.get("/set-pkce") + pkce = next( + c for c in r.headers.get_list("set-cookie") + if c.startswith(f"{PKCE_COOKIE}=") + ) + assert "HttpOnly" in pkce + assert "Max-Age=600" in pkce # 10 minutes + assert "Path=/" in pkce + assert "Secure" in pkce + + +def test_read_session_cookies_from_request(): + scope = { + "type": "http", + "method": "GET", + "path": "/", + "headers": [( + b"cookie", + f"{SESSION_AT_COOKIE}=at_value; {SESSION_RT_COOKIE}=rt_value".encode(), + )], + } + req = Request(scope) + at, rt = read_session_cookies(req) + assert at == "at_value" + assert rt == "rt_value" + + +def test_read_session_cookies_missing_returns_none(): + req = Request({"type": "http", "method": "GET", "path": "/", "headers": []}) + assert read_session_cookies(req) == (None, None) + + +def test_read_pkce_cookie_round_trip(): + scope = { + "type": "http", + "method": "GET", + "path": "/", + "headers": [(b"cookie", f"{PKCE_COOKIE}=state=s;verifier=v".encode())], + } + req = Request(scope) + assert read_pkce_cookie(req) == "state=s" # NB: cookie value stops at ';' + + +def test_detect_https_via_scheme(): + """``detect_https`` reads from request.url.scheme. + + Under uvicorn proxy_headers=True the scheme is rewritten from + ``X-Forwarded-Proto``; that's an integration concern, not unit. + """ + from hermes_cli.dashboard_auth.cookies import detect_https + http_req = Request({ + "type": "http", "method": "GET", "path": "/", "scheme": "http", + "headers": [], "server": ("x", 80), + }) + https_req = Request({ + "type": "http", "method": "GET", "path": "/", "scheme": "https", + "headers": [], "server": ("x", 443), + }) + assert detect_https(http_req) is False + assert detect_https(https_req) is True