mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-23 06:28:34 +00:00
fix(auth): persist csrf_token cookie for the access_token lifetime (#3872)
The csrf_token double-submit cookie is set without max_age (a session cookie), while the access_token cookie is persistent over HTTPS (max_age = token_expiry_days, see _set_session_cookie). The two cookies represent one session but had different lifetimes. iOS Safari home-screen PWAs evict session cookies when iOS terminates the standalone web app, but keep persistent ones. On reopen the user still appears logged in (the persistent access_token survives and GET /api/v1/auth/me is CSRF-exempt), but the session-only csrf_token is gone, so the frontend's readCsrfCookie() returns null and sends no X-CSRF-Token header. The first state-changing request then fails with 403 "CSRF token missing. Include X-CSRF-Token header." Only re-login restored it. This only manifests over HTTPS, which is why plain-HTTP local dev never sees it. Give csrf_token the same max_age as access_token at both mint sites -- CSRFMiddleware (auth POSTs) and _set_csrf_cookie (OIDC GET callback) -- mirroring _set_session_cookie's `... if is_https else None` guard so the double-submit pair always shares a lifetime: persistent together over HTTPS, session-only together over plain HTTP. Regression tests in tests/test_auth_type_system.py: - test_csrf_cookie_persistent_on_https - test_csrf_cookie_session_only_on_http - test_oidc_callback_csrf_cookie_persistent_on_https Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d9305989f3
commit
70d53da787
@ -14,6 +14,7 @@ from starlette.middleware.base import BaseHTTPMiddleware
|
||||
from starlette.responses import JSONResponse
|
||||
from starlette.types import ASGIApp
|
||||
|
||||
from app.gateway.auth.config import get_auth_config
|
||||
from app.gateway.auth_disabled import is_auth_disabled
|
||||
|
||||
CSRF_COOKIE_NAME = "csrf_token"
|
||||
@ -220,6 +221,12 @@ class CSRFMiddleware(BaseHTTPMiddleware):
|
||||
httponly=False, # Must be JS-readable for Double Submit Cookie pattern
|
||||
secure=is_https,
|
||||
samesite="strict",
|
||||
# Match the access_token cookie's lifetime (auth.py::_set_session_cookie)
|
||||
# so the double-submit pair never diverges. A session-only csrf_token is
|
||||
# evicted when iOS Safari terminates a home-screen PWA while the persistent
|
||||
# access_token survives — leaving the user "logged in" but unable to make
|
||||
# any state-changing request (403 "CSRF token missing").
|
||||
max_age=get_auth_config().token_expiry_days * 24 * 3600 if is_https else None,
|
||||
)
|
||||
|
||||
return response
|
||||
|
||||
@ -559,6 +559,10 @@ def _set_csrf_cookie(response: Response, request: Request) -> None:
|
||||
httponly=False, # Must be JS-readable for Double Submit Cookie pattern
|
||||
secure=is_https,
|
||||
samesite="strict",
|
||||
# Persist for the same lifetime as the access_token (see _set_session_cookie)
|
||||
# so the double-submit pair is evicted together, never leaving a logged-in
|
||||
# session whose csrf_token was dropped (e.g. iOS Safari PWA termination).
|
||||
max_age=get_auth_config().token_expiry_days * 24 * 3600 if is_https else None,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@ -714,3 +714,82 @@ def test_csrf_cookie_not_secure_on_http():
|
||||
assert csrf_cookies, "csrf_token cookie not set on HTTP register"
|
||||
csrf_header = csrf_cookies[0]
|
||||
assert "secure" not in csrf_header.lower().replace("samesite", "")
|
||||
|
||||
|
||||
def test_csrf_cookie_persistent_on_https():
|
||||
"""HTTPS register → csrf_token cookie is persistent (has max_age), like access_token.
|
||||
|
||||
Regression for iOS Safari home-screen PWAs. When iOS terminates a
|
||||
standalone web app it evicts *session* cookies but keeps *persistent*
|
||||
ones. The access_token cookie is persistent over HTTPS (carries
|
||||
max_age), so the user still appears logged in after reopening — but a
|
||||
session-only csrf_token cookie is dropped, so the first state-changing
|
||||
request fails with 403 "CSRF token missing. Include X-CSRF-Token
|
||||
header." The two cookies represent one session and must share a lifetime.
|
||||
"""
|
||||
_setup_config()
|
||||
client = _get_auth_client()
|
||||
resp = client.post(
|
||||
"/api/v1/auth/register",
|
||||
json={"email": _unique_email("csrf-persist"), "password": "Tr0ub4dor3a"},
|
||||
headers={"x-forwarded-proto": "https"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
set_cookies = _get_set_cookie_headers(resp)
|
||||
csrf_cookies = [h for h in set_cookies if "csrf_token=" in h]
|
||||
assert csrf_cookies, "csrf_token cookie not set on HTTPS register"
|
||||
assert "max-age" in csrf_cookies[0].lower(), "csrf_token must be persistent over HTTPS so iOS PWAs don't drop it as a session cookie"
|
||||
# It must pair with the access_token's lifetime: both persistent on HTTPS.
|
||||
access_cookies = [h for h in set_cookies if "access_token=" in h]
|
||||
assert access_cookies and "max-age" in access_cookies[0].lower()
|
||||
|
||||
|
||||
def test_csrf_cookie_session_only_on_http():
|
||||
"""HTTP register → csrf_token cookie has NO max_age (session cookie).
|
||||
|
||||
Mirrors the access_token's ``... if is_https else None`` guard so the
|
||||
pair stays symmetric: persistent together over HTTPS, session-only
|
||||
together over plain HTTP (local dev). Keeping them in lockstep is what
|
||||
avoids the "logged in but csrf_token gone" state.
|
||||
"""
|
||||
_setup_config()
|
||||
client = _get_auth_client()
|
||||
resp = client.post(
|
||||
"/api/v1/auth/register",
|
||||
json={"email": _unique_email("csrf-session"), "password": "Tr0ub4dor3a"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
csrf_cookies = [h for h in _get_set_cookie_headers(resp) if "csrf_token=" in h]
|
||||
assert csrf_cookies, "csrf_token cookie not set on HTTP register"
|
||||
assert "max-age" not in csrf_cookies[0].lower()
|
||||
|
||||
|
||||
def test_oidc_callback_csrf_cookie_persistent_on_https():
|
||||
"""The OIDC-callback CSRF cookie helper is persistent over HTTPS too.
|
||||
|
||||
``routers.auth._set_csrf_cookie`` is the second place a csrf_token cookie
|
||||
is minted (GET OIDC callback, which CSRFMiddleware does not cover). It has
|
||||
the same session-vs-persistent asymmetry and the same iOS PWA failure
|
||||
mode, so it must also carry max_age over HTTPS.
|
||||
"""
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import Response
|
||||
|
||||
from app.gateway.routers.auth import _set_csrf_cookie
|
||||
|
||||
_setup_config()
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "GET",
|
||||
"path": "/api/v1/auth/callback/example",
|
||||
"headers": [(b"x-forwarded-proto", b"https")],
|
||||
"scheme": "http",
|
||||
"server": ("internal", 8000),
|
||||
"query_string": b"",
|
||||
}
|
||||
response = Response()
|
||||
_set_csrf_cookie(response, Request(scope))
|
||||
set_cookie = response.headers.get("set-cookie", "").lower()
|
||||
assert "csrf_token=" in set_cookie
|
||||
assert "secure" in set_cookie
|
||||
assert "max-age" in set_cookie
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user