All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m56s
Ревью нашло два способа положить сервис ровно под той нагрузкой, ради которой писалась защита. Первый: `min()` вычисляет оба аргумента, поэтому `float(2 ** (excess - 1))` при 1045 неудачах по имени за окно падал с OverflowError. Счётчик ничем не ограничен сверху — `record()` только копит метки и на лимит не смотрит. С этой попытки и до конца окна вход отдавал 500 мгновенно, без задержки и без записи в аудит: терялись обе ценности PR, и трение, и сигнал. Показатель степени зажат; 2**16 заведомо выше любого разумного потолка, поэтому видимое поведение не меняется. Второй: сон шёл внутри области жизни сессии БД. В дефолтном режиме `get_identity_db` отдаёт ту же сессию, что `get_db`, а SELECT в `get_user_by_username` оставляет её в открытой транзакции — соединение висело занятым все восемь секунд. Пятнадцати одновременных неудач хватало, чтобы выбрать QueuePool целиком и уронить любой другой эндпоинт по pool_timeout. Отказ в обслуживании против всех сразу — хуже той блокировки учётки, ради ухода от которой замедление и выбиралось. Соединение теперь возвращается в пул перед сном. Заодно: длина имени ограничена 64 (верх CHECK'а реестра) — сырое имя становится ключом обоих лимитеров, а их словарь при часовом окне не подчищается; и явно записано, что `limit` у счётчика на имя не порог.
322 lines
21 KiB
Python
322 lines
21 KiB
Python
"""POST /api/v1/auth/login + /logout — DB-backed session auth (#2552, эпик #2549).
|
||
|
||
Переходный механизм, параллельный legacy Caddy trusted-header auth (roles.yaml).
|
||
См. `app.core.rbac.rbac_guard` (dual-mode resolver) и `app.services.auth_session`
|
||
(session CRUD). Mounted at `/api/v1/auth`; через Caddy `uri strip_prefix /trade-in`
|
||
это `/trade-in/api/v1/auth/*` снаружи.
|
||
|
||
Security:
|
||
- Неверные creds (неизвестный username / доступ закрыт / password_hash NULL /
|
||
неверный пароль) → ОДИНАКОВЫЙ 401 с generic сообщением — не раскрываем,
|
||
существует ли username (user-enumeration защита).
|
||
- Состояние доступа проверяется ТОЛЬКО ПОСЛЕ проверки пароля, и осмысленный
|
||
ответ (403 «пробный доступ закончился») получает исключительно тот, кто
|
||
пароль уже доказал. Ветвление ДО пароля превратило бы отдельный статус в
|
||
оракул существования логина: перебором можно было бы перечислить аккаунты,
|
||
не зная ни одного пароля (миграция data/sql/auth/004, WHY-2).
|
||
- #2552 post-review Medium 2: `verify_password` ВСЕГДА вызывается ровно
|
||
один раз — для несуществующего username / NULL password_hash сверяем
|
||
против статичного dummy-хеша (`_DUMMY_PASSWORD_HASH`, сгенерирован один
|
||
раз на импорте модуля), результат игнорируется. Без этого короткое
|
||
замыкание (`user is None → сразу 401`) давало наблюдаемую разницу во
|
||
времени ответа (~1мс без bcrypt vs ~100-300мс с ним) — классический
|
||
timing-oracle для user-enumeration, даже при одинаковом detail-сообщении.
|
||
- Rate-limit по (username, IP) — ЖЁСТЧЕ общего `RateLimitMiddleware`
|
||
(`/api/*`), т.к. login — типичная brute-force поверхность. Использует
|
||
`SlidingWindowLimiter` (тот же примитив, что и общий rate-limit). Ключ
|
||
length-prefixed (`len(username):username:ip`) — без этого произвольный
|
||
username с `:` внутри мог бы схлопнуть бюджет с другой (username, ip)
|
||
парой (IPv6-адреса тоже содержат `:`, так что просто эскейпить разделитель
|
||
в username недостаточно — паразитная граница возможна с обеих сторон).
|
||
- Поверх него — ГЛОБАЛЬНЫЙ счётчик неудач на ИМЯ, без IP в ключе (#2571):
|
||
лимит по паре (username, IP) распределённый перебор обходит целиком, просто
|
||
меняя адрес. Превышение порога не блокирует вход, а замедляет ответ
|
||
(`_throttle_delay_s`) — см. развёрнутое обоснование там же.
|
||
- Raw-пароль НИКОГДА не логируется и не попадает в user_events payload —
|
||
только username/ip/user_agent/path/method и (для неудач) состояние
|
||
счётчика попыток: сколько их за окно и какая задержка применена.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import asyncio
|
||
import logging
|
||
import secrets
|
||
from typing import Annotated
|
||
|
||
from fastapi import APIRouter, Depends, HTTPException, Request, Response
|
||
from pydantic import BaseModel, Field
|
||
from sqlalchemy.orm import Session
|
||
|
||
from app.core.config import settings
|
||
from app.core.password import hash_password, verify_password
|
||
from app.core.ratelimit import SlidingWindowLimiter, _client_ip
|
||
from app.services.auth_session import create_session, get_user_by_username, revoke_session
|
||
from app.services.identity_store import AccessState, get_identity_db
|
||
from app.services.user_events import schedule_event
|
||
|
||
logger = logging.getLogger(__name__)
|
||
|
||
router = APIRouter()
|
||
|
||
# Отдельный, более узкий бюджет чем общий per-user/per-IP `/api/*` лимит
|
||
# (см. app.core.ratelimit.SlidingWindowLimiter docstring — designed именно для
|
||
# такого случая). Ключ = username+IP: не даёт распределённому brute-force по
|
||
# ОДНОМУ аккаунту с разных IP уйти от лимита целиком (per-IP было бы недостаточно),
|
||
# и не блокирует ВЕСЬ IP из-за перебора чужих логинов одним же клиентом.
|
||
_LOGIN_LIMITER = SlidingWindowLimiter(
|
||
limit=settings.login_rate_limit,
|
||
window_s=settings.login_rate_limit_window_s,
|
||
)
|
||
|
||
# Глобальный счётчик неудач НА ИМЯ (#2571) — ключ БЕЗ IP, поэтому попытки со
|
||
# всех адресов складываются в один бюджет. Дополняет `_LOGIN_LIMITER`, а не
|
||
# заменяет: тот режет частый перебор с одного адреса, этот — редкий, но с
|
||
# тысячи адресов (credential stuffing), от которого per-(username, IP) ключ не
|
||
# защищает вообще — каждый новый адрес получает свежие login_rate_limit попыток.
|
||
#
|
||
# Живёт В ПАМЯТИ ПРОЦЕССА — сознательно, а не по недосмотру. Прод-бэкенд
|
||
# запущен одним uvicorn-воркером (docker-compose.prod.yml, комментарий над
|
||
# `command`: «Single worker сохраняется для предсказуемости»), значит счётчик и
|
||
# так глобален, а Redis в auth-пути добавил бы сетевую зависимость там, где её
|
||
# падение = либо дыра (fail-open), либо отказ входа (fail-closed).
|
||
# Потолок: появятся воркеры (`--workers N`) — потолок делится на N, и его надо
|
||
# переносить в Redis (`app.services.cache` уже держит там пул). Тот же ceiling
|
||
# у соседнего `_LOGIN_LIMITER`; перезапуск процесса обнуляет оба.
|
||
#
|
||
# ⚠️ `limit` здесь НЕ ПОРОГ и ничего не режет: мы зовём только `record()`, а он
|
||
# на лимит не смотрит — считает и отдаёт число попыток в окне. Настоящий порог
|
||
# живёт в `_throttle_delay_s`, которая читает настройку на каждом вызове (и
|
||
# потому подхватывает monkeypatch в тестах). Значение продублировано сюда ровно
|
||
# для того, чтобы `retry_after()` на этом объекте — если его однажды позовут —
|
||
# отвечал по тому же числу, а не по случайному.
|
||
_USERNAME_FAIL_LIMITER = SlidingWindowLimiter(
|
||
limit=settings.login_username_fail_threshold,
|
||
window_s=settings.login_username_fail_window_s,
|
||
)
|
||
|
||
# Timing-oracle защита (см. module docstring): bcrypt-хеш случайного пароля,
|
||
# сгенерированный ОДИН РАЗ на импорте модуля — используется вместо
|
||
# password_hash, когда юзер не найден/деактивирован/без пароля, чтобы
|
||
# `verify_password` (доминирующая по времени операция, ~100-300мс) всегда
|
||
# отрабатывала полный bcrypt-компар, независимо от того, существует ли аккаунт.
|
||
_DUMMY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16))
|
||
|
||
_INVALID_CREDENTIALS_DETAIL = "неверный логин или пароль"
|
||
|
||
# Единственный ответ логина, который НЕ generic 401: пароль верный, но пробный
|
||
# период истёк. `code` — машиночитаемый контракт для фронта (текст можно менять,
|
||
# ветку по нему — нет). Потребитель: `loginErrorMessage` в
|
||
# tradein-mvp/frontend/src/app/login/page.tsx — читает `detail.code` из
|
||
# `HTTPError.body` (frontend/src/lib/api.ts отдаёт тело ответа как есть) и
|
||
# показывает экран про пробный период вместо generic «Проверьте подключение».
|
||
# Меняешь значение здесь — меняй и там.
|
||
_ACCESS_EXPIRED_CODE = "access_expired"
|
||
_ACCESS_EXPIRED_MESSAGE = "Пробный доступ закончился"
|
||
|
||
|
||
class LoginRequest(BaseModel):
|
||
# max_length=64 — ровно верхняя граница CHECK'а реестра
|
||
# (`users_username_ascii_ck`, data/sql/auth/001), так что живое имя отсечь
|
||
# нельзя. Ограничение нужно не валидации ради: сырое имя становится ключом
|
||
# ОБОИХ лимитеров, а их `defaultdict` подчищается только при >10000 ключей и
|
||
# только от пустых корзин — при окне в час корзины непустые, освобождать
|
||
# нечего. Без границы длины килобайтные имена растили бы память ключами.
|
||
# Паттерн/минимум длины НЕ дублируем: в режиме `identity_store="tradein"`
|
||
# CHECK'а нет и живут не-ASCII имена (см. тест на кириллицу).
|
||
username: str = Field(max_length=64)
|
||
password: str
|
||
|
||
|
||
class LoginResponse(BaseModel):
|
||
ok: bool = True
|
||
|
||
|
||
def _throttle_delay_s(fails_in_window: int) -> float:
|
||
"""Насколько задержать ответ на неудачный вход при *fails_in_window* неудачах
|
||
по этому имени за окно. 0 — пока порог не перебран.
|
||
|
||
Замедление, а НЕ блокировка — намеренно. Жёсткая блокировка учётки после N
|
||
неудач лечится злоумышленником в свою пользу: не зная ни одного пароля, он
|
||
гарантированно выключает вход конкретному человеку (директору, админу) —
|
||
отказ в обслуживании дешевле и надёжнее, чем то, от чего блокировка
|
||
защищает. Задержка же не отнимает доступ ни у кого: владелец пароля войдёт
|
||
с первой попытки, просто ответ на очередную НЕУДАЧУ придёт медленнее.
|
||
|
||
Рост удвоением от 1с с потолком `login_username_throttle_max_delay_s`:
|
||
первые перебранные попытки почти незаметны, а сотни — упираются в потолок.
|
||
Потолок обязателен: без него задержка становится той же блокировкой, только
|
||
растянутой во времени.
|
||
|
||
Показатель степени зажат (`min(..., 16)`) — это не косметика. `min()` считает
|
||
ОБА аргумента до сравнения, поэтому наивный `float(2 ** (excess - 1))` при
|
||
excess>=1025 падает с `OverflowError: int too large to convert to float` —
|
||
то есть ровно под целевой нагрузкой (1045 неудач по имени за час = 0.3 rps)
|
||
защита начинала отдавать 500 мгновенно и без аудита, вместо 401 с задержкой.
|
||
2**16 = 65536с заведомо больше любого разумного потолка, так что зажим
|
||
видимого поведения не меняет, а арифметику делает безусловно конечной.
|
||
"""
|
||
excess = fails_in_window - settings.login_username_fail_threshold
|
||
if excess <= 0:
|
||
return 0.0
|
||
return min(settings.login_username_throttle_max_delay_s, 2.0 ** min(excess - 1, 16))
|
||
|
||
|
||
async def _reject_invalid_credentials(
|
||
db: Session, username: str, ip: str, user_agent: str | None
|
||
) -> HTTPException:
|
||
"""Единый хвост ЛЮБОГО отказа по кредам: счётчик → аудит → задержка → 401.
|
||
|
||
Один код на все ветки отказа (нет такого имени / неверный пароль / доступ
|
||
закрыт / password_hash NULL) — это не борьба с дублированием, а инвариант:
|
||
ветки обязаны быть неразличимы снаружи. Разъедься они по телу хендлера —
|
||
и достаточно забыть задержку в одной, чтобы «быстрый 401» стал оракулом
|
||
существования учётки ровно в том же виде, что и разные сообщения об ошибке.
|
||
Поэтому счётчик ведётся по ПРИСЛАННОМУ имени, без проверки, есть ли такое
|
||
в реестре: несуществующее имя копит неудачи и тормозит так же, как живое.
|
||
(`get_user_by_username` сверяет `username = :username` по text-колонке без
|
||
нормализации, так что сырое имя — тот же ключ, что и у поиска: регистром
|
||
счётчик не обойти.)
|
||
|
||
Возвращает `HTTPException`, а не бросает: `raise await …` не собирается, а
|
||
`raise (await …)` читается хуже, чем `raise` над возвращённым значением.
|
||
|
||
*db* нужен ровно затем, чтобы ОТДАТЬ соединение перед сном. `get_identity_db`
|
||
в дефолтном режиме (`identity_store="tradein"`, он же прод) отдаёт ту же
|
||
сессию, что `get_db` — движок с QueuePool на 5+10 соединений. После SELECT в
|
||
`get_user_by_username` сессия держит соединение в открытой транзакции, и сон
|
||
внутри её области жизни превращал бы каждую спящую попытку в занятое
|
||
соединение: ~15 одновременных неудач выбирают пул целиком, и тогда ЛЮБОЙ
|
||
эндпоинт ждёт checkout 30с и падает. Отказ в обслуживании против всех сразу —
|
||
хуже той блокировки учётки, ради отказа от которой всё это писалось.
|
||
"""
|
||
fails = _USERNAME_FAIL_LIMITER.record(username)
|
||
delay_s = _throttle_delay_s(fails)
|
||
|
||
schedule_event(
|
||
event_type="login_failed",
|
||
username=username,
|
||
ip=ip,
|
||
user_agent=user_agent,
|
||
path="/api/v1/auth/login",
|
||
method="POST",
|
||
# Состояние глобального счётчика — в аудит: по нему в user_events видно
|
||
# именно РАСПРЕДЕЛЁННЫЙ перебор (десятки неудач по одному имени с разных
|
||
# ip_address), который иначе выглядит как россыпь одиночных неудач.
|
||
payload={"username_fails_in_window": fails, "throttle_delay_s": delay_s},
|
||
)
|
||
|
||
if delay_s > 0:
|
||
logger.warning(
|
||
"login throttle: username=%r fails=%d delay=%.1fs ip=%s",
|
||
username,
|
||
fails,
|
||
delay_s,
|
||
ip,
|
||
)
|
||
# Соединение — в пул ДО сна (см. docstring). Сессия дальше не нужна:
|
||
# вызывающий немедленно делает raise, а повторный close() в самой
|
||
# зависимости идемпотентен.
|
||
db.close()
|
||
# await, не time.sleep: событийный цикл в это время обслуживает всех
|
||
# остальных — тормозим перебор, а не сервис.
|
||
await asyncio.sleep(delay_s)
|
||
|
||
return HTTPException(status_code=401, detail=_INVALID_CREDENTIALS_DETAIL)
|
||
|
||
|
||
@router.post("/login", response_model=LoginResponse)
|
||
async def login(
|
||
body: LoginRequest,
|
||
request: Request,
|
||
response: Response,
|
||
db: Annotated[Session, Depends(get_identity_db)],
|
||
) -> LoginResponse:
|
||
ip = _client_ip(request)
|
||
user_agent = request.headers.get("user-agent")
|
||
rate_key = f"{len(body.username)}:{body.username}:{ip}"
|
||
|
||
retry_after = _LOGIN_LIMITER.check(rate_key)
|
||
if retry_after is not None:
|
||
raise HTTPException(
|
||
status_code=429,
|
||
detail="слишком много попыток входа, попробуйте позже",
|
||
headers={"Retry-After": str(int(retry_after) + 1)},
|
||
)
|
||
|
||
user = get_user_by_username(db, body.username)
|
||
hash_to_check = (
|
||
user["password_hash"]
|
||
if user is not None and user["password_hash"] is not None
|
||
else _DUMMY_PASSWORD_HASH
|
||
)
|
||
# ВСЕГДА вызывается — dummy-хеш при отсутствующем юзере/NULL password_hash
|
||
# держит время ответа одинаковым независимо от существования аккаунта.
|
||
password_ok = verify_password(body.password, hash_to_check)
|
||
|
||
# Пароль проверен ВЫШЕ и безусловно — только теперь смотрим на состояние
|
||
# доступа. Порядок несущий, а не стилистический: см. модульный docstring.
|
||
if user is None or not password_ok:
|
||
raise await _reject_invalid_credentials(db, body.username, ip, user_agent)
|
||
|
||
access_state = user["access_state"]
|
||
if access_state is AccessState.TRIAL_EXPIRED:
|
||
# Пароль верный, сессия НЕ создаётся. Единственный не-generic ответ:
|
||
# аккаунт существует и владелец это уже доказал паролем, так что
|
||
# осмысленный текст ничего не раскрывает постороннему.
|
||
# В режиме identity_store="tradein" эта ветка недостижима: булев
|
||
# is_active даёт только active/disabled (identity_store.to_access_state).
|
||
schedule_event(
|
||
event_type="login_blocked_expired",
|
||
username=user["username"],
|
||
ip=ip,
|
||
user_agent=user_agent,
|
||
path="/api/v1/auth/login",
|
||
method="POST",
|
||
)
|
||
raise HTTPException(
|
||
status_code=403,
|
||
detail={"code": _ACCESS_EXPIRED_CODE, "message": _ACCESS_EXPIRED_MESSAGE},
|
||
)
|
||
|
||
if not access_state.can_sign_in:
|
||
# disabled (и любое нераспознанное состояние — to_access_state fail-closed)
|
||
# → ТОТ ЖЕ generic 401, то же событие и та же задержка, что при неверном
|
||
# пароле: заблокированный аккаунт неотличим от несуществующего.
|
||
raise await _reject_invalid_credentials(db, body.username, ip, user_agent)
|
||
|
||
token = create_session(db, user_id=user["user_id"], ip=ip, user_agent=user_agent)
|
||
|
||
response.set_cookie(
|
||
key=settings.session_cookie_name,
|
||
value=token,
|
||
max_age=settings.session_ttl_hours * 3600,
|
||
httponly=True,
|
||
secure=True,
|
||
samesite="lax",
|
||
path="/",
|
||
)
|
||
|
||
schedule_event(
|
||
event_type="login_success",
|
||
username=user["username"],
|
||
ip=ip,
|
||
user_agent=user_agent,
|
||
path="/api/v1/auth/login",
|
||
method="POST",
|
||
)
|
||
|
||
return LoginResponse(ok=True)
|
||
|
||
|
||
@router.post("/logout")
|
||
async def logout(
|
||
request: Request,
|
||
response: Response,
|
||
db: Annotated[Session, Depends(get_identity_db)],
|
||
) -> dict[str, bool]:
|
||
token = request.cookies.get(settings.session_cookie_name)
|
||
if token:
|
||
revoke_session(db, token)
|
||
response.delete_cookie(key=settings.session_cookie_name, path="/")
|
||
return {"ok": True}
|