mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
feat(dashboard-auth): cookie helpers for session_at/session_rt/pkce
Phase 3, Task 3.1. Three cookies: - hermes_session_at: OAuth access token (HttpOnly, TTL = token TTL) - hermes_session_rt: OAuth refresh token (HttpOnly, 30d max-age) - hermes_session_pkce: PKCE state + verifier + provider hint (10min) All SameSite=Lax + Path=/. Secure flag is set ONLY when the request scheme is https — uvicorn proxy_headers=True (enabled in gated mode at Phase 3.5) rewrites scheme from X-Forwarded-Proto so Fly's TLS terminator works.
This commit is contained in:
parent
628a52fce2
commit
a30c4d8ebd
2 changed files with 278 additions and 0 deletions
120
hermes_cli/dashboard_auth/cookies.py
Normal file
120
hermes_cli/dashboard_auth/cookies.py
Normal file
|
|
@ -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"
|
||||
158
tests/hermes_cli/test_dashboard_auth_cookies.py
Normal file
158
tests/hermes_cli/test_dashboard_auth_cookies.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue