fix(tradein/auth): не ронять и не занимать пул на замедлении входа (#2571)

Ревью нашло два способа положить сервис ровно под той нагрузкой, ради
которой писалась защита.

Первый: `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` у счётчика на имя не порог.

tradein-mvp/backend/app/api/v1/auth.py

@@ -45,7 +45,7 @@ import secrets
 from typing import Annotated
 
 from fastapi import APIRouter, Depends, HTTPException, Request, Response
-from pydantic import BaseModel
+from pydantic import BaseModel, Field
 from sqlalchemy.orm import Session
 
 from app.core.config import settings
@@ -83,6 +83,13 @@ _LOGIN_LIMITER = SlidingWindowLimiter(
 # Потолок: появятся воркеры (`--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,
@@ -109,7 +116,15 @@ _ACCESS_EXPIRED_MESSAGE = "Пробный доступ закончился"
 
 
 class LoginRequest(BaseModel):
-    username: str
+    # 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
 
 
@@ -132,15 +147,23 @@ def _throttle_delay_s(fails_in_window: int) -> float:
     первые перебранные попытки почти незаметны, а сотни — упираются в потолок.
     Потолок обязателен: без него задержка становится той же блокировкой, только
     растянутой во времени.
+
+    Показатель степени зажат (`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, float(2 ** (excess - 1)))
+    return min(settings.login_username_throttle_max_delay_s, 2.0 ** min(excess - 1, 16))
 
 
 async def _reject_invalid_credentials(
-    username: str, ip: str, user_agent: str | None
+    db: Session, username: str, ip: str, user_agent: str | None
 ) -> HTTPException:
     """Единый хвост ЛЮБОГО отказа по кредам: счётчик → аудит → задержка → 401.
 
@@ -157,6 +180,15 @@ async def _reject_invalid_credentials(
 
     Возвращает `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)
@@ -182,6 +214,10 @@ async def _reject_invalid_credentials(
             delay_s,
             ip,
         )
+        # Соединение — в пул ДО сна (см. docstring). Сессия дальше не нужна:
+        # вызывающий немедленно делает raise, а повторный close() в самой
+        # зависимости идемпотентен.
+        db.close()
         # await, не time.sleep: событийный цикл в это время обслуживает всех
         # остальных — тормозим перебор, а не сервис.
         await asyncio.sleep(delay_s)
@@ -221,7 +257,7 @@ async def login(
     # Пароль проверен ВЫШЕ и безусловно — только теперь смотрим на состояние
     # доступа. Порядок несущий, а не стилистический: см. модульный docstring.
     if user is None or not password_ok:
-        raise await _reject_invalid_credentials(body.username, ip, user_agent)
+        raise await _reject_invalid_credentials(db, body.username, ip, user_agent)
 
     access_state = user["access_state"]
     if access_state is AccessState.TRIAL_EXPIRED:
@@ -247,7 +283,7 @@ async def login(
         # disabled (и любое нераспознанное состояние — to_access_state fail-closed)
         # → ТОТ ЖЕ generic 401, то же событие и та же задержка, что при неверном
         # пароле: заблокированный аккаунт неотличим от несуществующего.
-        raise await _reject_invalid_credentials(body.username, ip, user_agent)
+        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)
 

tradein-mvp/backend/tests/test_auth_api.py

@@ -445,6 +445,13 @@ def test_throttle_delay_grows_and_caps(monkeypatch: pytest.MonkeyPatch) -> None:
     assert auth_router._throttle_delay_s(6) == 4.0
     assert auth_router._throttle_delay_s(7) == 4.0  # потолок
     assert auth_router._throttle_delay_s(1000) == 4.0
+    # Счётчик ничем не ограничен сверху (`record()` только добавляет метку), а
+    # `min()` вычисляет ОБА аргумента. Без зажатого показателя степени
+    # `float(2 ** (excess - 1))` при ~1045 неудачах падает с OverflowError, и
+    # защита начинает отдавать 500 без задержки и без аудита — ровно под той
+    # нагрузкой, ради которой писалась. 1000 выше проходило впритык под обрывом.
+    assert auth_router._throttle_delay_s(5_000) == 4.0
+    assert auth_router._throttle_delay_s(10**6) == 4.0
 
 
 def test_distributed_bruteforce_one_username_many_ips_hits_global_ceiling(
@@ -500,6 +507,43 @@ def test_throttle_actually_delays_the_response(
     assert elapsed >= 1.0
 
 
+def test_db_connection_released_before_sleeping(
+    client: TestClient, store: _Store, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Соединение с БД возвращается в пул ДО сна, а не удерживается всю задержку.
+
+    На проде `get_identity_db` в дефолтном режиме отдаёт ту же сессию, что
+    `get_db` (движок с QueuePool 5+10, pool_timeout=30), а `get_user_by_username`
+    оставляет её в открытой транзакции. Сон внутри этой области жизни держал бы
+    соединение занятым все 8с: ~15 одновременно спящих неудач выбирают пул
+    целиком, и дальше ЛЮБОЙ эндпоинт ждёт checkout 30с и падает — отказ в
+    обслуживании против всех, ради ухода от которого замедление и выбиралось
+    вместо блокировки.
+
+    Проверяем порядком, а не мокой пула: если `close()` случился до сна, между
+    ним и концом ответа лежит вся задержка; если бы сессию закрывала только
+    зависимость (то есть после сна) — зазор был бы околонулевым.
+    """
+    store.add_user("holder", hash_password("Secret123!"), role="employee")
+    _throttle_settings(monkeypatch, threshold=0, max_delay_s=1.0)
+
+    closes: list[float] = []
+    real_close = _FakeDB.close
+
+    def _spy_close(self: _FakeDB) -> None:
+        closes.append(time.monotonic())
+        real_close(self)
+
+    monkeypatch.setattr(_FakeDB, "close", _spy_close)
+
+    resp = client.post("/api/v1/auth/login", json={"username": "holder", "password": "wrong"})
+    finished = time.monotonic()
+
+    assert resp.status_code == 401
+    assert closes, "сессия не закрывалась вовсе"
+    assert finished - closes[0] >= 1.0
+
+
 def test_typo_does_not_throttle_and_correct_password_still_works(
     client: TestClient, store: _Store, monkeypatch: pytest.MonkeyPatch
 ) -> None:
@@ -523,6 +567,39 @@ def test_typo_does_not_throttle_and_correct_password_still_works(
     assert config.settings.session_cookie_name in ok.cookies
 
 
+def test_counter_decays_when_window_passes(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Вторая половина DoD 2: наказание не накапливается вечно.
+
+    Окно скользящее, старые неудачи выпадают сами — снимать ничего вручную не
+    нужно. Проверяем на самом счётчике, а не через HTTP: один вызов login стоит
+    полного bcrypt (~0.25с), так что игрушечное окно истекало бы прямо посреди
+    цикла запросов и тест мерил бы скорость хеширования, а не спад счётчика.
+    """
+    _throttle_settings(monkeypatch, threshold=1, max_delay_s=4.0)
+    limiter = auth_router._USERNAME_FAIL_LIMITER
+    monkeypatch.setattr(limiter, "_window_s", 0.2)
+
+    assert [limiter.record("frank") for _ in range(3)] == [1, 2, 3]
+    assert auth_router._throttle_delay_s(3) > 0
+
+    time.sleep(0.25)  # окно прошло — прошлые неудачи больше не считаются
+
+    assert limiter.record("frank") == 1
+    assert auth_router._throttle_delay_s(1) == 0.0
+
+
+def test_username_length_is_bounded(client: TestClient) -> None:
+    """Сырое имя становится ключом обоих лимитеров, а их словарь чистится только
+    при >10000 ключей и только от пустых корзин — при окне в час чистить нечего.
+    Границу длины держим на 64 (верх CHECK'а реестра), чтобы килобайтные имена
+    не растили память ключами."""
+    resp = client.post("/api/v1/auth/login", json={"username": "x" * 65, "password": "p"})
+    assert resp.status_code == 422
+    # 64 — всё ещё валидная длина, отвечаем обычным generic-отказом.
+    ok_len = client.post("/api/v1/auth/login", json={"username": "x" * 64, "password": "p"})
+    assert ok_len.status_code == 401
+
+
 def test_throttle_identical_for_existing_and_unknown_username(
     client: TestClient, store: _Store, monkeypatch: pytest.MonkeyPatch
 ) -> None: