v1.351 — пересмотр логики выдачи CSRF-токена
This commit is contained in:
@@ -182,7 +182,7 @@ docker compose -f docker-compose.test.yml down -v # остановить и с
|
||||
- Ветка **`main`** — релиз прода: готовое промоутишь из `dev` через `merge dev→main`. Файлы во
|
||||
всех ветках одинаковы (окружение задаёт `.env`/compose, а не ветка) → merge безболезненный;
|
||||
чистоту прод-образа обеспечивает `.dockerignore`, а не разные наборы файлов.
|
||||
- **Деплой на Pi:** `git pull` ветки `main` → `docker compose up -d --build`.
|
||||
- **Деплой на Pi:** `git pull` ветки `main` → `scripts/build-push.ps1`.
|
||||
- **Чистая выгрузка в папку без git** (опц.): `scripts/export-prod.sh <dir> main` (через
|
||||
`git archive` + `export-ignore` — в папку идёт ровно прод-набор, без лаунчера/тестов/dev-входа).
|
||||
|
||||
|
||||
@@ -65,12 +65,19 @@ def generate_csrf_token() -> str:
|
||||
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()
|
||||
response.set_cookie(
|
||||
key=CSRF_COOKIE,
|
||||
value=csrf,
|
||||
max_age=max_age,
|
||||
max_age=_csrf_max_age(),
|
||||
httponly=False, # должен читаться JS, чтобы продублировать в заголовок
|
||||
secure=settings.cookie_secure,
|
||||
samesite="lax",
|
||||
@@ -80,6 +87,14 @@ def _set_csrf_cookie(response: Response, max_age: int) -> str:
|
||||
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:
|
||||
ttl = settings.jwt_user_ttl_minutes
|
||||
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,
|
||||
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:
|
||||
@@ -109,7 +124,7 @@ def set_admin_session(response: Response, admin_id: int) -> None:
|
||||
path=_ADMIN_PATH,
|
||||
domain=settings.cookie_domain_value,
|
||||
)
|
||||
_set_csrf_cookie(response, ttl * 60)
|
||||
_set_csrf_cookie(response)
|
||||
|
||||
|
||||
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"}
|
||||
|
||||
|
||||
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:
|
||||
"""Double-submit CSRF на чистом ASGI: для аутентифицированных мутаций на /api требуем
|
||||
совпадения заголовка X-CSRF-Token и cookie csrf_token.
|
||||
|
||||
Если сессия есть, а csrf_token в запросе нет (cookie истекла или её стёрли), любой ответ
|
||||
на /api — включая отказ ниже — перевыдаёт токен. Иначе состояние не лечилось: токен
|
||||
выдаётся только при входе, а войти и выйти мешала эта же проверка.
|
||||
|
||||
Намеренно НЕ на BaseHTTPMiddleware: тот буферизует потоковые ответы и ломает SSE
|
||||
(/api/events). Чистый ASGI пропускает стримы насквозь, вмешиваясь только при отказе CSRF.
|
||||
(/api/events). Чистый ASGI пропускает стримы насквозь.
|
||||
"""
|
||||
|
||||
def __init__(self, app) -> None: # noqa: ANN001
|
||||
@@ -48,13 +71,15 @@ class CSRFMiddleware:
|
||||
async def __call__(self, scope, receive, send): # noqa: ANN001
|
||||
if scope["type"] == "http":
|
||||
request = Request(scope)
|
||||
if request.method in _UNSAFE_METHODS and request.url.path.startswith("/api"):
|
||||
if request.url.path.startswith("/api"):
|
||||
has_session = (
|
||||
security.USER_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)
|
||||
if not cookie_token or cookie_token != header_token:
|
||||
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