CSRF: перевыдавать токен, если сессия есть, а cookie нет
На проде любые изменения данных, включая выход из аккаунта, отвечали 403 CSRF_FAILED, хотя сайт оставался залогиненным. Cookie csrf_token одна на игрока и админку: вход в админку перезаписывал её со сроком 8 часов, а сессия игрока живёт 7 дней. Когда токен истекал, сервер выдавал новый только при входе, а войти и выйти мешала та же проверка. Из этого состояния было не выбраться, кроме как стереть cookie сайта руками. Теперь CSRFMiddleware перевыдаёт токен на любом ответе /api, если запрос несёт сессионную cookie без csrf_token, в том числе на самом отказе. SPA на загрузке делает GET /api/users/me, поэтому пользователю хватает перезагрузить страницу. Правится только стартовое сообщение ответа, тело идёт насквозь, и SSE-поток не буферизуется. Срок токена теперь не короче самой долгой сессии, так что вход в админку больше не укорачивает токен игрока. Проверка double-submit не ослаблена: запрос с cookie, но без заголовка или с чужим токеном по-прежнему получает 403, и cookie в этом случае не перевыдаётся. Перевыданный токен из кросс-сайтового ответа атакующему ничего не даёт: прочитать cookie может только JS того же origin. Тесты закрепляют восстановление на GET и на отказе, срок после входа в админку, прежнюю строгость проверки, отсутствие токена у анонимов и то, что middleware не склеивает чанки потока. На старом коде четыре из них падают. #50 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfTsytzT6TojfmprRDKiV6
This commit is contained in:
@@ -65,12 +65,19 @@ def generate_csrf_token() -> str:
|
|||||||
return secrets.token_urlsafe(24)
|
return secrets.token_urlsafe(24)
|
||||||
|
|
||||||
|
|
||||||
def _set_csrf_cookie(response: Response, max_age: int) -> str:
|
def _csrf_max_age() -> int:
|
||||||
|
"""Токен один на обе сессии, поэтому живёт не меньше самой долгой из них: иначе вход
|
||||||
|
в админку (8 ч) перезаписывал токен игрока (7 дней), и после его истечения все мутации
|
||||||
|
игрока падали с CSRF_FAILED. Без сессии токен бесполезен — лишний срок безопасен."""
|
||||||
|
return max(settings.jwt_user_ttl_minutes, settings.jwt_admin_ttl_minutes) * 60
|
||||||
|
|
||||||
|
|
||||||
|
def _set_csrf_cookie(response: Response) -> str:
|
||||||
csrf = generate_csrf_token()
|
csrf = generate_csrf_token()
|
||||||
response.set_cookie(
|
response.set_cookie(
|
||||||
key=CSRF_COOKIE,
|
key=CSRF_COOKIE,
|
||||||
value=csrf,
|
value=csrf,
|
||||||
max_age=max_age,
|
max_age=_csrf_max_age(),
|
||||||
httponly=False, # должен читаться JS, чтобы продублировать в заголовок
|
httponly=False, # должен читаться JS, чтобы продублировать в заголовок
|
||||||
secure=settings.cookie_secure,
|
secure=settings.cookie_secure,
|
||||||
samesite="lax",
|
samesite="lax",
|
||||||
@@ -80,6 +87,14 @@ def _set_csrf_cookie(response: Response, max_age: int) -> str:
|
|||||||
return csrf
|
return csrf
|
||||||
|
|
||||||
|
|
||||||
|
def fresh_csrf_set_cookie() -> tuple[bytes, bytes]:
|
||||||
|
"""Готовый заголовок Set-Cookie со свежим токеном — для ASGI-middleware, где объекта
|
||||||
|
Response нет. Строится тем же _set_csrf_cookie, чтобы атрибуты не разъехались."""
|
||||||
|
carrier = Response()
|
||||||
|
_set_csrf_cookie(carrier)
|
||||||
|
return next((k, v) for k, v in carrier.raw_headers if k == b"set-cookie")
|
||||||
|
|
||||||
|
|
||||||
def set_user_session(response: Response, user_id: int, provider: str) -> None:
|
def set_user_session(response: Response, user_id: int, provider: str) -> None:
|
||||||
ttl = settings.jwt_user_ttl_minutes
|
ttl = settings.jwt_user_ttl_minutes
|
||||||
token = create_token(user_id, AUDIENCE_USER, ttl, provider)
|
token = create_token(user_id, AUDIENCE_USER, ttl, provider)
|
||||||
@@ -93,7 +108,7 @@ def set_user_session(response: Response, user_id: int, provider: str) -> None:
|
|||||||
path=_USER_PATH,
|
path=_USER_PATH,
|
||||||
domain=settings.cookie_domain_value,
|
domain=settings.cookie_domain_value,
|
||||||
)
|
)
|
||||||
_set_csrf_cookie(response, ttl * 60)
|
_set_csrf_cookie(response)
|
||||||
|
|
||||||
|
|
||||||
def set_admin_session(response: Response, admin_id: int) -> None:
|
def set_admin_session(response: Response, admin_id: int) -> None:
|
||||||
@@ -109,7 +124,7 @@ def set_admin_session(response: Response, admin_id: int) -> None:
|
|||||||
path=_ADMIN_PATH,
|
path=_ADMIN_PATH,
|
||||||
domain=settings.cookie_domain_value,
|
domain=settings.cookie_domain_value,
|
||||||
)
|
)
|
||||||
_set_csrf_cookie(response, ttl * 60)
|
_set_csrf_cookie(response)
|
||||||
|
|
||||||
|
|
||||||
def clear_user_session(response: Response) -> None:
|
def clear_user_session(response: Response) -> None:
|
||||||
|
|||||||
+29
-4
@@ -34,12 +34,35 @@ _STATIC_DIR = Path(os.getenv("STATIC_DIR", str(Path(__file__).resolve().parent.p
|
|||||||
_UNSAFE_METHODS = {"POST", "PUT", "PATCH", "DELETE"}
|
_UNSAFE_METHODS = {"POST", "PUT", "PATCH", "DELETE"}
|
||||||
|
|
||||||
|
|
||||||
|
def _with_fresh_csrf_cookie(send): # noqa: ANN001, ANN202
|
||||||
|
"""Дописывает свежий csrf_token в заголовки ответа. Трогает только
|
||||||
|
http.response.start: тело (в т.ч. SSE-поток) проходит насквозь, чанк за чанком."""
|
||||||
|
|
||||||
|
async def wrapped(message): # noqa: ANN001
|
||||||
|
if message["type"] == "http.response.start":
|
||||||
|
headers = list(message.get("headers", []))
|
||||||
|
already_set = any(
|
||||||
|
k.lower() == b"set-cookie" and v.startswith(security.CSRF_COOKIE.encode() + b"=")
|
||||||
|
for k, v in headers
|
||||||
|
)
|
||||||
|
if not already_set:
|
||||||
|
headers.append(security.fresh_csrf_set_cookie())
|
||||||
|
message = {**message, "headers": headers}
|
||||||
|
await send(message)
|
||||||
|
|
||||||
|
return wrapped
|
||||||
|
|
||||||
|
|
||||||
class CSRFMiddleware:
|
class CSRFMiddleware:
|
||||||
"""Double-submit CSRF на чистом ASGI: для аутентифицированных мутаций на /api требуем
|
"""Double-submit CSRF на чистом ASGI: для аутентифицированных мутаций на /api требуем
|
||||||
совпадения заголовка X-CSRF-Token и cookie csrf_token.
|
совпадения заголовка X-CSRF-Token и cookie csrf_token.
|
||||||
|
|
||||||
|
Если сессия есть, а csrf_token в запросе нет (cookie истекла или её стёрли), любой ответ
|
||||||
|
на /api — включая отказ ниже — перевыдаёт токен. Иначе состояние не лечилось: токен
|
||||||
|
выдаётся только при входе, а войти и выйти мешала эта же проверка.
|
||||||
|
|
||||||
Намеренно НЕ на BaseHTTPMiddleware: тот буферизует потоковые ответы и ломает SSE
|
Намеренно НЕ на BaseHTTPMiddleware: тот буферизует потоковые ответы и ломает SSE
|
||||||
(/api/events). Чистый ASGI пропускает стримы насквозь, вмешиваясь только при отказе CSRF.
|
(/api/events). Чистый ASGI пропускает стримы насквозь.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, app) -> None: # noqa: ANN001
|
def __init__(self, app) -> None: # noqa: ANN001
|
||||||
@@ -48,13 +71,15 @@ class CSRFMiddleware:
|
|||||||
async def __call__(self, scope, receive, send): # noqa: ANN001
|
async def __call__(self, scope, receive, send): # noqa: ANN001
|
||||||
if scope["type"] == "http":
|
if scope["type"] == "http":
|
||||||
request = Request(scope)
|
request = Request(scope)
|
||||||
if request.method in _UNSAFE_METHODS and request.url.path.startswith("/api"):
|
if request.url.path.startswith("/api"):
|
||||||
has_session = (
|
has_session = (
|
||||||
security.USER_COOKIE in request.cookies
|
security.USER_COOKIE in request.cookies
|
||||||
or security.ADMIN_COOKIE in request.cookies
|
or security.ADMIN_COOKIE in request.cookies
|
||||||
)
|
)
|
||||||
if has_session:
|
cookie_token = request.cookies.get(security.CSRF_COOKIE)
|
||||||
cookie_token = request.cookies.get(security.CSRF_COOKIE)
|
if has_session and not cookie_token:
|
||||||
|
send = _with_fresh_csrf_cookie(send)
|
||||||
|
if has_session and request.method in _UNSAFE_METHODS:
|
||||||
header_token = request.headers.get(security.CSRF_HEADER)
|
header_token = request.headers.get(security.CSRF_HEADER)
|
||||||
if not cookie_token or cookie_token != header_token:
|
if not cookie_token or cookie_token != header_token:
|
||||||
response = JSONResponse(
|
response = JSONResponse(
|
||||||
|
|||||||
@@ -0,0 +1,150 @@
|
|||||||
|
"""CSRF double-submit: токен восстанавливается, если сессия пережила cookie csrf_token,
|
||||||
|
а сама проверка мутаций остаётся такой же строгой."""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
from tests.conftest import csrf_headers, login
|
||||||
|
|
||||||
|
|
||||||
|
def _set_cookie(resp, name: str) -> str | None:
|
||||||
|
"""Заголовок Set-Cookie для cookie name (или None, если ответ её не ставит)."""
|
||||||
|
for header in resp.headers.get_list("set-cookie"):
|
||||||
|
if header.startswith(f"{name}="):
|
||||||
|
return header
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _max_age(set_cookie: str) -> int:
|
||||||
|
for part in set_cookie.split(";"):
|
||||||
|
key, _, value = part.strip().partition("=")
|
||||||
|
if key.lower() == "max-age":
|
||||||
|
return int(value)
|
||||||
|
raise AssertionError(f"нет Max-Age: {set_cookie}")
|
||||||
|
|
||||||
|
|
||||||
|
def test_missing_token_reissued_on_safe_request(client: TestClient):
|
||||||
|
"""Сессия жива, csrf_token истёк → первый же GET отдаёт новый токен, мутации проходят."""
|
||||||
|
login(client, "Игрок")
|
||||||
|
client.cookies.delete("csrf_token")
|
||||||
|
|
||||||
|
me = client.get("/api/users/me")
|
||||||
|
assert me.status_code == 200, me.text
|
||||||
|
assert _set_cookie(me, "csrf_token") is not None
|
||||||
|
assert client.cookies.get("csrf_token")
|
||||||
|
|
||||||
|
r = client.patch(
|
||||||
|
"/api/users/me/profile", json={"favorite_faction_id": None}, headers=csrf_headers(client)
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejected_mutation_reissues_token(client: TestClient):
|
||||||
|
"""Отказ CSRF без cookie сам выдаёт токен: иначе не выйти и не перезайти."""
|
||||||
|
login(client, "Игрок")
|
||||||
|
client.cookies.delete("csrf_token")
|
||||||
|
|
||||||
|
r = client.post("/api/auth/logout")
|
||||||
|
assert r.status_code == 403
|
||||||
|
assert r.json()["error"]["code"] == "CSRF_FAILED"
|
||||||
|
assert _set_cookie(r, "csrf_token") is not None
|
||||||
|
|
||||||
|
r2 = client.post("/api/auth/logout", headers=csrf_headers(client))
|
||||||
|
assert r2.status_code == 200, r2.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_admin_login_does_not_shorten_token(client: TestClient, make_admin):
|
||||||
|
"""Вход в админку перезаписывает общий csrf_token — срок не короче сессии игрока."""
|
||||||
|
r_user = client.post("/api/auth/dev/login", json={"nickname": "Игрок"})
|
||||||
|
session_age = _max_age(_set_cookie(r_user, "fs_session"))
|
||||||
|
|
||||||
|
make_admin("boss", "secret123")
|
||||||
|
r_admin = client.post(
|
||||||
|
"/api/admin/auth/login",
|
||||||
|
json={"username": "boss", "password": "secret123"},
|
||||||
|
headers=csrf_headers(client),
|
||||||
|
)
|
||||||
|
assert r_admin.status_code == 200, r_admin.text
|
||||||
|
assert _max_age(_set_cookie(r_admin, "csrf_token")) >= session_age
|
||||||
|
|
||||||
|
|
||||||
|
def test_check_is_not_weakened(client: TestClient):
|
||||||
|
"""Cookie есть, заголовка нет или он чужой — 403, и токен при этом не перевыдаётся."""
|
||||||
|
login(client, "Игрок")
|
||||||
|
token = client.cookies.get("csrf_token")
|
||||||
|
body = {"favorite_faction_id": None}
|
||||||
|
|
||||||
|
no_header = client.patch("/api/users/me/profile", json=body)
|
||||||
|
assert no_header.status_code == 403
|
||||||
|
assert _set_cookie(no_header, "csrf_token") is None
|
||||||
|
|
||||||
|
wrong = client.patch("/api/users/me/profile", json=body, headers={"X-CSRF-Token": "forged"})
|
||||||
|
assert wrong.status_code == 403
|
||||||
|
assert _set_cookie(wrong, "csrf_token") is None
|
||||||
|
|
||||||
|
assert client.cookies.get("csrf_token") == token
|
||||||
|
|
||||||
|
|
||||||
|
def test_anonymous_gets_no_token(client: TestClient):
|
||||||
|
r = client.get("/api/auth/config")
|
||||||
|
assert r.status_code == 200
|
||||||
|
assert _set_cookie(r, "csrf_token") is None
|
||||||
|
|
||||||
|
|
||||||
|
def _run_middleware(app_messages: list[dict], cookie: bytes) -> list[dict]:
|
||||||
|
"""Прогоняет CSRFMiddleware над фейковым приложением и возвращает отправленное."""
|
||||||
|
from app.main import CSRFMiddleware
|
||||||
|
|
||||||
|
async def fake_app(scope, receive, send): # noqa: ANN001
|
||||||
|
for message in app_messages:
|
||||||
|
await send(message)
|
||||||
|
|
||||||
|
sent: list[dict] = []
|
||||||
|
|
||||||
|
async def send(message): # noqa: ANN001
|
||||||
|
sent.append(message)
|
||||||
|
|
||||||
|
async def receive(): # pragma: no cover — фейковому приложению тело запроса не нужно
|
||||||
|
return {"type": "http.request", "body": b"", "more_body": False}
|
||||||
|
|
||||||
|
scope = {
|
||||||
|
"type": "http",
|
||||||
|
"method": "GET",
|
||||||
|
"path": "/api/events",
|
||||||
|
"raw_path": b"/api/events",
|
||||||
|
"root_path": "",
|
||||||
|
"scheme": "http",
|
||||||
|
"server": ("testserver", 80),
|
||||||
|
"query_string": b"",
|
||||||
|
"headers": [(b"cookie", cookie)],
|
||||||
|
}
|
||||||
|
asyncio.run(CSRFMiddleware(fake_app)(scope, receive, send))
|
||||||
|
return sent
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_keeps_stream_unbuffered():
|
||||||
|
"""Перевыдача трогает только стартовое сообщение: чанки SSE идут по одному, без склейки."""
|
||||||
|
start = {"type": "http.response.start", "status": 200, "headers": [(b"content-type", b"text/event-stream")]}
|
||||||
|
chunks = [
|
||||||
|
{"type": "http.response.body", "body": b": connected\n\n", "more_body": True},
|
||||||
|
{"type": "http.response.body", "body": b": ping\n\n", "more_body": True},
|
||||||
|
{"type": "http.response.body", "body": b"", "more_body": False},
|
||||||
|
]
|
||||||
|
sent = _run_middleware([start, *chunks], cookie=b"fs_session=abc")
|
||||||
|
|
||||||
|
assert sent[1:] == chunks
|
||||||
|
cookies = [v for k, v in sent[0]["headers"] if k == b"set-cookie"]
|
||||||
|
assert len(cookies) == 1 and cookies[0].startswith(b"csrf_token=")
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_does_not_duplicate_app_cookie():
|
||||||
|
"""Если приложение само ставит csrf_token (вход), второй Set-Cookie не дописывается."""
|
||||||
|
own = (b"set-cookie", b"csrf_token=from-app; Path=/")
|
||||||
|
start = {"type": "http.response.start", "status": 200, "headers": [own]}
|
||||||
|
body = {"type": "http.response.body", "body": b"{}", "more_body": False}
|
||||||
|
sent = _run_middleware([start, body], cookie=b"fs_session=abc")
|
||||||
|
|
||||||
|
cookies = [v for k, v in sent[0]["headers"] if k == b"set-cookie"]
|
||||||
|
assert cookies == [own[1]]
|
||||||
Reference in New Issue
Block a user