diff --git a/auth/roles.yaml b/auth/roles.yaml index 3227f325..d1e26301 100644 --- a/auth/roles.yaml +++ b/auth/roles.yaml @@ -79,7 +79,13 @@ users: user8: pilot user9: pilot user10: pilot - praktika: expired # пробный доступ закончился 2026-06-27 — см. NoAccessScreen variant="trial" + praktika: pilot # ГК «Практика» — доступ восстановлен 2026-07-27 (решение владельца + # продукта; ранее expired с 2026-06-27). Безлимитная квота оценок + # выдана через account_quota_overrides.unlimited (migration 191), + # не через код — см. app.services.account_quota.is_unlimited. admintest: admin # temp QA 2026-05-26 pilottest: pilot # temp QA 2026-05-26 analysttest: analyst # temp QA 2026-06-07 (#962) + expiredtest: expired # temp QA 2026-07-27 — role=expired regression coverage для + # test_rbac.py (praktika перестал быть expired-фикстурой + # после восстановления доступа) diff --git a/tradein-mvp/backend/app/services/account_quota.py b/tradein-mvp/backend/app/services/account_quota.py index 2923b76a..5b51be37 100644 --- a/tradein-mvp/backend/app/services/account_quota.py +++ b/tradein-mvp/backend/app/services/account_quota.py @@ -12,7 +12,11 @@ used только для user2, 189 закрывает остальные аккаунты + запрещает регресс. В коде декремента `used` НЕТ — increment() только `used + 1` под TOCTOU-guard (#747); любой negative used приходит исключительно извне (ручной UPDATE). -- Без лимита (unlimited): роль admin ИЛИ username == 'kopylov'. +- Без лимита (unlimited): роль admin (без похода в БД) ИЛИ персональный грант + account_quota_overrides.unlimited = true (миграция 191_account_quota_unlimited_flag.sql). + До миграции 191 unlimited для non-admin аккаунтов был захардкожен как + `username == 'kopylov'` прямо в коде — данные (kopylov + praktika) заменяют этот + хардкод целиком, единый источник правды для всех безлимитных non-admin грантов. - Учитываются ТОЛЬКО успешные оценки (инкремент ПОСЛЕ estimate_quality). - Если заголовок X-Authenticated-User отсутствует (dev без Caddy) → unlimited, лимит не применяется (fail-open). @@ -47,19 +51,37 @@ def current_period() -> str: return datetime.now(UTC).strftime("%Y-%m") -def is_unlimited(username: str) -> bool: +def is_unlimited(db: Session, username: str) -> bool: """True если пользователь не ограничен квотой. - Unlimited: роль admin ИЛИ username == 'kopylov'. - KeyError (неизвестный пользователь) → трактуется как limited (False). + Unlimited если: + - роль admin (RBAC roles.yaml, in-memory, БЕЗ похода в БД — admin гарантированно + безлимитен по дизайну RBAC, отдельная per-user запись не нужна); + - ЛИБО персональный грант account_quota_overrides.unlimited = true (миграция + 191) — единственный источник правды для non-admin безлимитных аккаунтов, + включая kopylov (перенесён сюда этой же миграцией, до 191 был захардкожен + как `username == 'kopylov'`) и praktika (пилот восстановлен 2026-07-27). + + KeyError (неизвестный пользователь, не в roles.yaml) → трактуется как limited + (False), БЕЗ похода в БД — override-таблица не источник правды для юзеров, + которых вообще нет в RBAC-конфиге. """ - if username == "kopylov": - return True try: role = get_role(username) - return role == "admin" except KeyError: return False + if role == "admin": + return True + row = db.execute( + text( + """ + SELECT unlimited FROM account_quota_overrides + WHERE username = :u + """ + ), + {"u": username}, + ).fetchone() + return bool(row is not None and row.unlimited) def user_limit(db: Session, username: str) -> int: @@ -99,7 +121,7 @@ def get_status(db: Session, username: str | None) -> dict: "unlimited": True, } - unlimited = is_unlimited(username) + unlimited = is_unlimited(db, username) period = current_period() limit = user_limit(db, username) @@ -143,7 +165,7 @@ def check_and_raise(db: Session, username: str | None) -> None: if username is None: return - if is_unlimited(username): + if is_unlimited(db, username): return period = current_period() @@ -182,7 +204,7 @@ def increment(db: Session, username: str | None) -> bool: WHERE (он только для DO UPDATE), поэтому первая оценка месяца проходит. `lim` — персональный лимит (user_limit), НЕ жёстко зашитый глобальный MONTHLY_LIMIT. """ - if username is None or is_unlimited(username): + if username is None or is_unlimited(db, username): return True period = current_period() diff --git a/tradein-mvp/backend/data/sql/191_account_quota_unlimited_flag.sql b/tradein-mvp/backend/data/sql/191_account_quota_unlimited_flag.sql new file mode 100644 index 00000000..3d1191c6 --- /dev/null +++ b/tradein-mvp/backend/data/sql/191_account_quota_unlimited_flag.sql @@ -0,0 +1,68 @@ +-- Migration 191: account_quota_overrides.unlimited — безлимит как данные, не хардкод +-- +-- WHY: +-- app.services.account_quota.is_unlimited() до этой миграции проверял ровно два +-- условия: роль admin ИЛИ literal `username == 'kopylov'` — захардкоженное сравнение +-- строки прямо в коде. Восстановление пилота praktika (ГК «Практика», доступ вернул +-- владелец продукта 2026-07-27 — см. auth/roles.yaml) с безлимитным грантом сделало +-- бы это хардкодом ВТОРОГО имени: не масштабируется (каждый следующий безлимитный +-- клиент требовал бы code-change + review + deploy вместо data-change) и плохо само +-- по себе как паттерн (магическая строка вместо конфигурируемых данных). +-- +-- unlimited — отдельная boolean-колонка, а не sentinel-значение monthly_limit +-- (-1 / 0): `0` неоднозначен («ноль оценок в месяц» vs «без лимита»), explicit +-- boolean честнее и не требует специального парсинга в user_limit()/is_unlimited(). +-- +-- kopylov ПЕРЕНЕСЁН в данные этой же миграцией (хардкод в коде убран, не оставлен +-- параллельно) — единый источник правды для non-admin unlimited-аккаунтов вместо +-- двух параллельных механизмов (код-константа + таблица). Порядок деплоя +-- (SQL-миграция применяется РАНЬШЕ, чем стартует новый код — см. +-- .claude/rules/sql.md "Migration order") гарантирует, что строка kopylov уже в +-- таблице к моменту, когда новый is_unlimited() (без хардкода) начинает работать — +-- поведение kopylov не меняется ни на секунду простоя. +-- +-- WHAT: +-- 1. account_quota_overrides.unlimited boolean NOT NULL DEFAULT false. +-- 2. Seed: kopylov (перенос хардкода) + praktika (новый грант, пилот восстановлен +-- 2026-07-27) — оба unlimited=true. monthly_limit=999999 — placeholder: get_status() +-- безусловно читает monthly_limit через user_limit() даже для unlimited-аккаунтов +-- (чтобы вернуть какое-то "limit" поле в /quota), а само enforcement для unlimited +-- обходит этот лимит (is_unlimited() гейтит раньше в check_and_raise()/increment()). +-- Значение просто не должно выглядеть абсурдным, если когда-либо surfaced напрямую. +-- +-- IDEMPOTENCY: +-- ALTER TABLE ... ADD COLUMN IF NOT EXISTS (новая колонка) + INSERT ... ON CONFLICT +-- DO UPDATE (повторный прогон сходится к тому же состоянию, не дублирует строки). +-- +-- Dependencies: 185_account_quota_overrides.sql (создаёт account_quota_overrides). + +BEGIN; + +ALTER TABLE account_quota_overrides + ADD COLUMN IF NOT EXISTS unlimited boolean NOT NULL DEFAULT false; + +COMMENT ON COLUMN account_quota_overrides.unlimited IS + 'Безлимитный грант (нет месячного лимита оценок) — читается ' + 'app.services.account_quota.is_unlimited(). Заменяет прежний хардкод username в коде.'; + +INSERT INTO account_quota_overrides (username, monthly_limit, unlimited, note) +VALUES ('kopylov', 999999, true, + 'Личный аккаунт — безлимит перенесён из хардкода is_unlimited() в данные ' + '(migration 191, 2026-07-27), поведение не изменилось') +ON CONFLICT (username) DO UPDATE SET + monthly_limit = EXCLUDED.monthly_limit, + unlimited = EXCLUDED.unlimited, + note = EXCLUDED.note, + updated_at = now(); + +INSERT INTO account_quota_overrides (username, monthly_limit, unlimited, note) +VALUES ('praktika', 999999, true, + 'ГК «Практика» — пилот восстановлен 2026-07-27 (решение владельца продукта), ' + 'безлимитный грант') +ON CONFLICT (username) DO UPDATE SET + monthly_limit = EXCLUDED.monthly_limit, + unlimited = EXCLUDED.unlimited, + note = EXCLUDED.note, + updated_at = now(); + +COMMIT; diff --git a/tradein-mvp/backend/tests/test_account_quota.py b/tradein-mvp/backend/tests/test_account_quota.py index d9afbd0e..362f4b4a 100644 --- a/tradein-mvp/backend/tests/test_account_quota.py +++ b/tradein-mvp/backend/tests/test_account_quota.py @@ -1,16 +1,21 @@ """Tests for app.services.account_quota — monthly estimate quota enforcement. Coverage: - (a) admin и kopylov unlimited — не блокируются, increment является no-op + (a) admin, kopylov, praktika unlimited — не блокируются, increment является no-op (b) обычный pilot-юзер блокируется на 16-м запросе (429 + нужный detail) (c) increment растит used счётчик (d) get_status корректен для different сценариев (e) отсутствие заголовка X-Authenticated-User = unlimited (fail-open) (f) #747 — атомарно-условный increment (TOCTOU fix) - (g) account_quota_overrides — персональный лимит вместо negative-used хака + (g) account_quota_overrides.monthly_limit — персональный лимит вместо negative-used + хака (h) insufficient_data результат НЕ инкрементит квоту (geocode-fail не сжигает слот) + (i) account_quota_overrides.unlimited — data-driven безлимит (migration 191): + kopylov (перенесён из хардкода) и praktika (восстановленный пилот) безлимитны + через таблицу, не через код -DB мокируется через MagicMock — реальная БД не требуется. +DB мокируется через _FakeDB (роутинг по SQL-тексту, см. ниже) — реальная БД не +требуется. """ from __future__ import annotations @@ -48,107 +53,167 @@ from app.services.account_quota import ( # noqa: E402 # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- +# +# is_unlimited() теперь (migration 191) может как шорткатиться БЕЗ похода в БД +# (роль admin, ИЛИ username вообще не в roles.yaml → KeyError), так и делать +# реальный SELECT unlimited FROM account_quota_overrides (обычный pilot / kopylov / +# praktika). Это значит, что порядок/количество db.execute() вызовов зависит от +# username, а не только от вызываемой функции — позиционные side_effect-списки +# были бы хрупкими. Вместо этого _FakeDB роутит execute() по ТЕКСТУ SQL, что +# устойчиво к тому, сколько раз и в каком порядке реально стучимся в БД. class _Row: - """Row stand-in поддерживающий и .used/.monthly_limit, и индексный доступ row[0].""" + """Row stand-in: произвольные named-поля + позиционный доступ row[0] + (нужен increment() для RETURNING used в debug-логе).""" - def __init__(self, value: int) -> None: - self.used = value - self.monthly_limit = value - self._t = (value,) + def __init__(self, **fields: int | bool) -> None: + for name, value in fields.items(): + setattr(self, name, value) + self._t = tuple(fields.values()) def __getitem__(self, i: int) -> int: return self._t[i] -def _result(row: _Row | None) -> MagicMock: - result = MagicMock() - result.fetchone.return_value = row - return result +class _FakeDB: + """DB session mock, роутит execute() по подстроке в SQL-тексте, а не по + порядку вызова — устойчив к тому, что is_unlimited() иногда обращается к БД + (обычный pilot / kopylov / praktika), а иногда шорткатится без неё (admin / + неизвестный username).""" + + def __init__( + self, + *, + unlimited: bool | None = None, + override_limit: int | None = None, + used: int | None = None, + upsert_used: int | None = None, + ) -> None: + self.unlimited = unlimited + self.override_limit = override_limit + self.used = used + self.upsert_used = upsert_used + self.commits = 0 + self.execute_calls: list[tuple[str, dict]] = [] + + def execute(self, stmt: object, params: dict | None = None) -> MagicMock: + sql = str(stmt) + self.execute_calls.append((sql, dict(params or {}))) + result = MagicMock() + if "SELECT unlimited FROM account_quota_overrides" in sql: + result.fetchone.return_value = ( + None if self.unlimited is None else _Row(unlimited=self.unlimited) + ) + elif "SELECT monthly_limit FROM account_quota_overrides" in sql: + result.fetchone.return_value = ( + None if self.override_limit is None else _Row(monthly_limit=self.override_limit) + ) + elif "INSERT INTO account_estimate_usage" in sql: + result.fetchone.return_value = ( + None if self.upsert_used is None else _Row(used=self.upsert_used) + ) + elif "SELECT used FROM account_estimate_usage" in sql: + result.fetchone.return_value = None if self.used is None else _Row(used=self.used) + else: + raise AssertionError(f"_FakeDB: unrecognized SQL: {sql!r}") + return result + + def commit(self) -> None: + self.commits += 1 def _override_result(limit: int | None) -> MagicMock: - """Mock результата запроса account_quota_overrides.monthly_limit (user_limit()).""" - return _result(None if limit is None else _Row(limit)) - - -def _used_result(used: int | None) -> MagicMock: - """Mock результата запроса account_estimate_usage.used (или UPSERT RETURNING used).""" - return _result(None if used is None else _Row(used)) - - -def _db_with_used(used: int, *, override_limit: int | None = None) -> MagicMock: - """DB session mock для get_status/check_and_raise: ровно 2 execute() — - (1) user_limit() override lookup, (2) account_estimate_usage.used lookup.""" - db = MagicMock() - db.execute.side_effect = [_override_result(override_limit), _used_result(used)] - return db - - -def _db_no_row(*, override_limit: int | None = None) -> MagicMock: - """DB session mock где строки usage ещё нет (первая оценка месяца).""" - db = MagicMock() - db.execute.side_effect = [_override_result(override_limit), _used_result(None)] - return db - - -def _db_for_increment(*, override_limit: int | None, upsert_used: int | None) -> MagicMock: - """DB session mock для increment(): (1) user_limit() override lookup, - (2) UPSERT ... RETURNING used (None если WHERE used < lim не матчит).""" - db = MagicMock() - db.execute.side_effect = [_override_result(override_limit), _used_result(upsert_used)] - return db + """Mock результата запроса account_quota_overrides.monthly_limit — для тестов, + вызывающих user_limit() напрямую (без is_unlimited в цепочке).""" + result = MagicMock() + result.fetchone.return_value = None if limit is None else _Row(monthly_limit=limit) + return result # --------------------------------------------------------------------------- -# (a) admin and kopylov are unlimited +# (a) admin / kopylov / praktika unlimited # --------------------------------------------------------------------------- def test_is_unlimited_admin() -> None: - assert is_unlimited("admin") is True + """admin шорткатится по роли — БЕЗ похода в БД.""" + db = MagicMock() + assert is_unlimited(db, "admin") is True + db.execute.assert_not_called() def test_is_unlimited_kopylov() -> None: - assert is_unlimited("kopylov") is True + """kopylov — unlimited через account_quota_overrides.unlimited=true (migration + 191), не через хардкод в коде.""" + db = _FakeDB(unlimited=True) + assert is_unlimited(db, "kopylov") is True + + +def test_is_unlimited_praktika() -> None: + """praktika — восстановленный пилот с безлимитным грантом (migration 191).""" + db = _FakeDB(unlimited=True) + assert is_unlimited(db, "praktika") is True def test_is_unlimited_pilot_user1() -> None: - assert is_unlimited("user1") is False + """user1 — обычный pilot, нет override-строки → limited.""" + db = _FakeDB() + assert is_unlimited(db, "user1") is False def test_is_unlimited_unknown_user() -> None: - """Неизвестный пользователь → False (KeyError трактуется как limited).""" - assert is_unlimited("ghost_unknown_xyz") is False + """Неизвестный пользователь → False (KeyError трактуется как limited), БЕЗ + похода в БД.""" + db = MagicMock() + assert is_unlimited(db, "ghost_unknown_xyz") is False + db.execute.assert_not_called() def test_check_and_raise_admin_not_blocked() -> None: """admin с used=15 не получает 429.""" - db = _db_with_used(MONTHLY_LIMIT) - # Should not raise - check_and_raise(db, "admin") + db = MagicMock() + check_and_raise(db, "admin") # не должно поднять исключение + db.execute.assert_not_called() def test_check_and_raise_kopylov_not_blocked() -> None: - """kopylov с used=100 не получает 429.""" - db = _db_with_used(100) - check_and_raise(db, "kopylov") + """kopylov с used=100 (гипотетически) не получает 429 — is_unlimited гейтит + раньше usage-lookup.""" + db = _FakeDB(unlimited=True) + check_and_raise(db, "kopylov") # не должно поднять исключение + + +def test_check_and_raise_praktika_not_blocked() -> None: + """praktika (unlimited=true) не получает 429 независимо от used.""" + db = _FakeDB(unlimited=True) + check_and_raise(db, "praktika") # не должно поднять исключение def test_increment_admin_is_noop() -> None: """increment для admin → никаких db.execute вызовов.""" db = MagicMock() - increment(db, "admin") + assert increment(db, "admin") is True db.execute.assert_not_called() db.commit.assert_not_called() def test_increment_kopylov_is_noop() -> None: - """increment для kopylov → no-op.""" - db = MagicMock() - increment(db, "kopylov") - db.execute.assert_not_called() + """increment для kopylov (unlimited=true) → True, БЕЗ UPSERT/commit — + is_unlimited() гейтит раньше инкремента (единственный execute — проверка + unlimited-флага, не usage-UPSERT).""" + db = _FakeDB(unlimited=True) + assert increment(db, "kopylov") is True + assert db.commits == 0 + assert not any("INSERT INTO account_estimate_usage" in sql for sql, _ in db.execute_calls) + + +def test_increment_praktika_is_noop() -> None: + """increment для praktika (unlimited=true) → True, без UPSERT/commit.""" + db = _FakeDB(unlimited=True) + assert increment(db, "praktika") is True + assert db.commits == 0 + assert not any("INSERT INTO account_estimate_usage" in sql for sql, _ in db.execute_calls) # --------------------------------------------------------------------------- @@ -158,7 +223,7 @@ def test_increment_kopylov_is_noop() -> None: def test_check_and_raise_pilot_not_blocked_at_14() -> None: """used=14 < 15 → не блокируется.""" - db = _db_with_used(14) + db = _FakeDB(used=14) check_and_raise(db, "user1") # должно пройти без исключения @@ -168,7 +233,7 @@ def test_check_and_raise_pilot_not_blocked_at_15_boundary() -> None: Логика: used >= limit → block. После 15-й успешной оценки increment делает used=15, поэтому следующий запрос (16-й) блокируется. """ - db = _db_with_used(MONTHLY_LIMIT) + db = _FakeDB(used=MONTHLY_LIMIT) from fastapi import HTTPException with pytest.raises(HTTPException) as exc_info: @@ -179,7 +244,7 @@ def test_check_and_raise_pilot_not_blocked_at_15_boundary() -> None: def test_check_and_raise_pilot_blocked_exact_detail() -> None: """Проверяем точный текст сообщения 429.""" - db = _db_with_used(MONTHLY_LIMIT) + db = _FakeDB(used=MONTHLY_LIMIT) from fastapi import HTTPException with pytest.raises(HTTPException) as exc_info: @@ -192,7 +257,7 @@ def test_check_and_raise_pilot_blocked_exact_detail() -> None: def test_check_and_raise_pilot_blocked_over_limit() -> None: """used=20 тоже блокируется (нет override → глобальный лимит).""" - db = _db_with_used(20) + db = _FakeDB(used=20) from fastapi import HTTPException with pytest.raises(HTTPException) as exc_info: @@ -206,18 +271,17 @@ def test_check_and_raise_pilot_blocked_over_limit() -> None: def test_increment_pilot_calls_upsert() -> None: - """increment для pilot → выполняет user_limit lookup + UPSERT (2 execute), 1 commit.""" - db = _db_for_increment(override_limit=None, upsert_used=1) + """increment для pilot → is_unlimited-lookup + user_limit-lookup + UPSERT + (3 execute), 1 commit.""" + db = _FakeDB(upsert_used=1) result = increment(db, "user1") assert result is True - assert db.execute.call_count == 2 - db.commit.assert_called_once() + assert len(db.execute_calls) == 3 + assert db.commits == 1 - # Второй вызов — UPSERT; проверяем что SQL содержит ON CONFLICT ... DO UPDATE - call_args = db.execute.call_args_list[1] - sql_text = str(call_args[0][0]) # first positional arg — text() object - assert "ON CONFLICT" in sql_text - assert "used" in sql_text + upsert_calls = [sql for sql, _ in db.execute_calls if "ON CONFLICT" in sql] + assert len(upsert_calls) == 1 + assert "used" in upsert_calls[0] def test_increment_none_username_is_noop() -> None: @@ -229,9 +293,9 @@ def test_increment_none_username_is_noop() -> None: def test_increment_pilot_first_estimate_of_month() -> None: """Первый инкремент (нет строки в БД) — должен всё равно выполнить UPSERT.""" - db = _db_for_increment(override_limit=None, upsert_used=1) + db = _FakeDB(upsert_used=1) increment(db, "user5") - assert db.execute.call_count == 2 + assert len(db.execute_calls) == 3 # --------------------------------------------------------------------------- @@ -252,16 +316,28 @@ def test_get_status_none_username() -> None: def test_get_status_admin() -> None: """admin → unlimited, remaining=limit вне зависимости от used.""" - db = _db_with_used(7) + db = _FakeDB(used=7) status = get_status(db, "admin") assert status["unlimited"] is True assert status["remaining"] == MONTHLY_LIMIT assert status["used"] == 7 # фактический used из БД +def test_get_status_praktika_unlimited() -> None: + """praktika (unlimited=true) → unlimited=True, remaining=limit, без деления + на ноль и без «Осталось N из 0» (limit берётся из user_limit(), не 0).""" + db = _FakeDB(unlimited=True, override_limit=999_999, used=42) + status = get_status(db, "praktika") + assert status["unlimited"] is True + assert status["limit"] == 999_999 + assert status["remaining"] == 999_999 + assert status["used"] == 42 + assert status["limit"] > 0 # защита от «N из 0» + + def test_get_status_pilot_with_used() -> None: """pilot с used=10 → remaining=5.""" - db = _db_with_used(10) + db = _FakeDB(used=10) status = get_status(db, "user2") assert status["unlimited"] is False assert status["used"] == 10 @@ -271,7 +347,7 @@ def test_get_status_pilot_with_used() -> None: def test_get_status_pilot_no_row_yet() -> None: """Новый месяц — строки нет → used=0, remaining=15.""" - db = _db_no_row() + db = _FakeDB() status = get_status(db, "user3") assert status["used"] == 0 assert status["remaining"] == MONTHLY_LIMIT @@ -280,7 +356,7 @@ def test_get_status_pilot_no_row_yet() -> None: def test_get_status_pilot_exhausted() -> None: """used=15 → remaining=0.""" - db = _db_with_used(MONTHLY_LIMIT) + db = _FakeDB(used=MONTHLY_LIMIT) status = get_status(db, "user4") assert status["remaining"] == 0 assert status["unlimited"] is False @@ -288,7 +364,7 @@ def test_get_status_pilot_exhausted() -> None: def test_get_status_pilot_over_limit_remaining_zero() -> None: """used=20 → remaining=0 (не отрицательное).""" - db = _db_with_used(20) + db = _FakeDB(used=20) status = get_status(db, "user5") assert status["remaining"] == 0 @@ -320,7 +396,7 @@ def quota_app() -> FastAPI: application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") def _override_db(): - yield _db_no_row() + yield _FakeDB() application.dependency_overrides[get_db] = _override_db return application @@ -363,6 +439,39 @@ def test_quota_endpoint_admin_unlimited(quota_app: FastAPI) -> None: assert data["unlimited"] is True +@pytest.fixture() +def quota_app_praktika_unlimited() -> FastAPI: + """FastAPI app где БД отдаёт unlimited=true для praktika.""" + from app.api.v1 import trade_in as trade_in_module + from app.core.db import get_db + + application = FastAPI() + application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") + + def _override_db(): + yield _FakeDB(unlimited=True, override_limit=999_999, used=100) + + application.dependency_overrides[get_db] = _override_db + return application + + +def test_quota_endpoint_praktika_unlimited(quota_app_praktika_unlimited: FastAPI) -> None: + """GET /quota с praktika (unlimited grant) → unlimited=True, осмысленный + (не нулевой) limit/remaining — фронт (page.tsx) всё равно скрывает эти числа + при unlimited=True, но backend не должен отдавать «0 из 0».""" + client = TestClient(quota_app_praktika_unlimited) + resp = client.get( + "/api/v1/trade-in/quota", + headers={"X-Authenticated-User": "praktika"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["unlimited"] is True + assert data["limit"] > 0 + assert data["remaining"] > 0 + assert data["used"] == 100 + + # --------------------------------------------------------------------------- # Integration: POST /estimate quota enforcement через TestClient # --------------------------------------------------------------------------- @@ -378,7 +487,7 @@ def estimate_app_exhausted() -> FastAPI: application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") def _override_db(): - yield _db_with_used(MONTHLY_LIMIT) + yield _FakeDB(used=MONTHLY_LIMIT) application.dependency_overrides[get_db] = _override_db return application @@ -430,6 +539,39 @@ def test_estimate_no_header_not_blocked(estimate_app_exhausted: FastAPI) -> None assert resp.status_code != 429 +@pytest.fixture() +def estimate_app_praktika_unlimited() -> FastAPI: + """FastAPI app где БД отдаёт unlimited=true для praktika (used заведомо + «за пределами» обычного лимита — проверяем что это НЕ блокирует).""" + from app.api.v1 import trade_in as trade_in_module + from app.core.db import get_db + + application = FastAPI() + application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") + + def _override_db(): + yield _FakeDB(unlimited=True, override_limit=999_999, used=MONTHLY_LIMIT + 500) + + application.dependency_overrides[get_db] = _override_db + return application + + +def test_estimate_praktika_not_blocked_429(estimate_app_praktika_unlimited: FastAPI) -> None: + """POST /estimate с praktika (unlimited=true, used far above обычного лимита) + → НЕ 429 — восстановленный пилот с безлимитным грантом не упирается в квоту.""" + client = TestClient(estimate_app_praktika_unlimited, raise_server_exceptions=False) + resp = client.post( + "/api/v1/trade-in/estimate", + json={ + "address": "г. Екатеринбург, ул. Малышева, 1", + "area_m2": 50.0, + "rooms": 2, + }, + headers={"X-Authenticated-User": "praktika"}, + ) + assert resp.status_code != 429 + + # --------------------------------------------------------------------------- # (f) #747 — атомарно-условный increment (TOCTOU fix) # --------------------------------------------------------------------------- @@ -444,25 +586,46 @@ class _AtomicResult: class _AtomicQuotaFakeDB: - """In-memory fake, воспроизводящий user_limit() override-lookup + атомарный - conditional UPSERT (#747): - - execute() без ключа "lim" в params → user_limit() override lookup — - возвращает override_limit (или None → increment() падает на глобальный - MONTHLY_LIMIT как :lim в следующем вызове). - - execute() с ключом "lim" → UPSERT: строки ещё нет (used=None) → INSERT used=1, - RETURNING row (WHERE не применяется к INSERT); used < lim → used+=1, RETURNING - row; used >= lim → конфликтная строка НЕ обновлена, RETURNING пуст (None). + """In-memory fake, воспроизводящий: + - is_unlimited(): SELECT unlimited FROM account_quota_overrides -> + unlimited_override (None → строки нет → not unlimited); + - user_limit(): SELECT monthly_limit FROM account_quota_overrides -> + override_limit (None → глобальный MONTHLY_LIMIT); + - increment(): атомарный conditional UPSERT (#747) — WHERE used < :lim в SQL, + эмулируется через params["lim"]. + + Роутинг по ТЕКСТУ SQL (не по наличию "lim" в params) — обе override-lookup + query (is_unlimited и user_limit) не содержат "lim" в params, поэтому их нужно + различать по содержимому запроса, а не по форме params. """ - def __init__(self, *, used: int | None, override_limit: int | None = None) -> None: + def __init__( + self, + *, + used: int | None, + override_limit: int | None = None, + unlimited_override: bool | None = None, + ) -> None: self.used = used self.override_limit = override_limit + self.unlimited_override = unlimited_override self.commits = 0 - def execute(self, _stmt: object, params: dict) -> _AtomicResult: - if "lim" not in params: - row = _Row(self.override_limit) if self.override_limit is not None else None + def execute(self, stmt: object, params: dict) -> _AtomicResult: + sql = str(stmt) + if "SELECT unlimited FROM account_quota_overrides" in sql: + row = ( + _Row(unlimited=self.unlimited_override) + if self.unlimited_override is not None + else None + ) return _AtomicResult(row) + if "SELECT monthly_limit FROM account_quota_overrides" in sql: + row = ( + _Row(monthly_limit=self.override_limit) if self.override_limit is not None else None + ) + return _AtomicResult(row) + # UPSERT — WHERE used < :lim lim = params["lim"] if self.used is None: self.used = 1 @@ -470,7 +633,7 @@ class _AtomicQuotaFakeDB: self.used += 1 else: return _AtomicResult(None) - return _AtomicResult(_Row(self.used)) + return _AtomicResult(_Row(used=self.used)) def commit(self) -> None: self.commits += 1 @@ -521,15 +684,26 @@ def test_increment_atomic_override_below_global_blocks_early() -> None: """override=5 (ниже глобального 15): used=5 уже блокирует, хотя < MONTHLY_LIMIT. Демонстрирует что per-user override заменяет глобальный лимит полностью — - не является дополнительным потолком поверх него. + не является дополнительным потолком поверх него. username вымышленный (не в + roles.yaml) — is_unlimited() шорткатится на KeyError без похода в БД. """ db = _AtomicQuotaFakeDB(used=5, override_limit=5) assert increment(db, "user_low_override") is False assert db.used == 5 # не выросло +def test_increment_atomic_kopylov_unlimited_bypasses_upsert() -> None: + """kopylov (unlimited=true через account_quota_overrides) — increment() True + без похода в UPSERT-ветку, used в фейке не растёт.""" + db = _AtomicQuotaFakeDB(used=MONTHLY_LIMIT, unlimited_override=True) + assert increment(db, "kopylov") is True + assert db.commits == 0 + assert db.used == MONTHLY_LIMIT # не тронут — is_unlimited гейтит раньше UPSERT + + # --------------------------------------------------------------------------- -# (g) account_quota_overrides — персональный лимит (замена negative-used хака) +# (g) account_quota_overrides.monthly_limit — персональный лимит (замена +# negative-used хака) # --------------------------------------------------------------------------- @@ -549,7 +723,7 @@ def test_user_limit_with_override_returns_override() -> None: def test_get_status_override_limit() -> None: """user2 с override=50, used=0 (после сброса хака) → limit=50, remaining=50.""" - db = _db_with_used(0, override_limit=50) + db = _FakeDB(used=0, override_limit=50) status = get_status(db, "user2") assert status["limit"] == 50 assert status["remaining"] == 50 @@ -559,7 +733,7 @@ def test_get_status_override_limit() -> None: def test_get_status_override_remaining_clamped_even_if_used_negative() -> None: """Кламп: даже если used снова просочится отрицательным (regression прежнего negative-used хака), remaining НЕ превышает limit — не «Осталось 50 из 15».""" - db = _db_with_used(-35, override_limit=50) + db = _FakeDB(used=-35, override_limit=50) status = get_status(db, "user2") assert status["limit"] == 50 assert status["used"] == -35 # raw used не скрываем — диагностическая честность @@ -568,8 +742,9 @@ def test_get_status_override_remaining_clamped_even_if_used_negative() -> None: def test_check_and_raise_override_blocks_below_global_limit() -> None: - """override=5 (ниже глобального 15) — used=5 блокируется, хотя < MONTHLY_LIMIT.""" - db = _db_with_used(5, override_limit=5) + """override=5 (ниже глобального 15) — used=5 блокируется, хотя < MONTHLY_LIMIT. + username вымышленный (не в roles.yaml) — is_unlimited() KeyError-шорткат.""" + db = _FakeDB(used=5, override_limit=5) from fastapi import HTTPException with pytest.raises(HTTPException) as exc_info: @@ -579,20 +754,20 @@ def test_check_and_raise_override_blocks_below_global_limit() -> None: def test_check_and_raise_override_allows_above_global_limit() -> None: """override=50 — used=20 (> глобального 15) НЕ блокируется.""" - db = _db_with_used(20, override_limit=50) + db = _FakeDB(used=20, override_limit=50) check_and_raise(db, "user2") # не должно поднять исключение def test_increment_override_blocks_at_override_not_global() -> None: """increment уважает per-user override: used>=override → False, даже если used < MONTHLY_LIMIT (15).""" - db = _db_for_increment(override_limit=5, upsert_used=None) # WHERE used<5 не матчит + db = _FakeDB(override_limit=5, upsert_used=None) # WHERE used<5 не матчит assert increment(db, "user_low_override") is False def test_increment_override_allows_above_global_limit() -> None: """increment с override=50: used=20 (>15 глобального) успешно инкрементит.""" - db = _db_for_increment(override_limit=50, upsert_used=21) + db = _FakeDB(override_limit=50, upsert_used=21) assert increment(db, "user2") is True @@ -648,7 +823,7 @@ def estimate_app_ok() -> FastAPI: application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") def _override_db(): - yield _db_no_row() + yield _FakeDB() application.dependency_overrides[get_db] = _override_db return application diff --git a/tradein-mvp/backend/tests/test_migration_191_account_quota_unlimited_flag.py b/tradein-mvp/backend/tests/test_migration_191_account_quota_unlimited_flag.py new file mode 100644 index 00000000..f3e1cb5b --- /dev/null +++ b/tradein-mvp/backend/tests/test_migration_191_account_quota_unlimited_flag.py @@ -0,0 +1,83 @@ +"""Static guards for migration 191 (account_quota_overrides.unlimited). + +Прод применяет data/sql построчно строго (ON_ERROR_STOP). Полный DB-прогон +требует живой БД; здесь фиксируем структурные инварианты, которые ГАРАНТИРУЮТ +идемпотентность и что миграция реально закрывает bootstrap unlimited-грантов +для kopylov (перенос хардкода из app.services.account_quota.is_unlimited) и +praktika (пилот восстановлен 2026-07-27). +""" + +from __future__ import annotations + +import re +from pathlib import Path + +_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql" +_MIGRATION_191 = _SQL_DIR / "191_account_quota_unlimited_flag.sql" + + +def _sql() -> str: + return _MIGRATION_191.read_text(encoding="utf-8") + + +def _executable_sql() -> str: + """SQL без построчных `--`-комментариев — только исполняемый код.""" + lines = [] + for raw in _sql().splitlines(): + code = raw.split("--", 1)[0] + if code.strip(): + lines.append(code) + return "\n".join(lines) + + +def _flat(text: str) -> str: + return re.sub(r"\s+", " ", text).strip().lower() + + +def test_migration_191_exists() -> None: + assert _MIGRATION_191.exists(), f"missing migration: {_MIGRATION_191}" + + +def test_migration_191_is_transactional() -> None: + sql = _sql() + assert "BEGIN;" in sql + assert "COMMIT;" in sql + + +def test_migration_191_adds_unlimited_column_idempotently() -> None: + """ADD COLUMN IF NOT EXISTS — безопасен при повторном прогоне.""" + flat = _flat(_executable_sql()) + assert "alter table account_quota_overrides" in flat + assert "add column if not exists unlimited boolean not null default false" in flat + + +def test_migration_191_seeds_kopylov_and_praktika_unlimited() -> None: + """Оба грант-аккаунта присутствуют в seed-данных с unlimited=true.""" + flat = _flat(_executable_sql()) + assert "'kopylov'" in flat + assert "'praktika'" in flat + # Ровно 2 INSERT INTO account_quota_overrides в этом файле (kopylov + praktika). + assert flat.count("insert into account_quota_overrides") == 2 + # unlimited=true передаётся позиционно в VALUES для обеих строк. + assert flat.count("999999, true") == 2 + + +def test_migration_191_seed_upserts_are_idempotent() -> None: + """ON CONFLICT (username) DO UPDATE — повторный прогон сходится к тому же + состоянию, не дублирует строки (username — PRIMARY KEY, см. migration 185).""" + flat = _flat(_executable_sql()) + assert flat.count("on conflict (username) do update set") == 2 + assert "unlimited = excluded.unlimited" in flat + + +def test_migration_191_no_psycopg_trap() -> None: + """Никаких :param::type — psycopg v3 требует CAST(... AS type) (не применимо + в чистом .sql без bind params, но проверяем на регресс copy-paste).""" + assert not re.search(r":\w+::", _sql()) + + +def test_migration_191_no_destructive_ddl() -> None: + """Миграция не должна содержать DROP TABLE / TRUNCATE (см. .claude/rules/sql.md).""" + flat = _flat(_executable_sql()) + assert "drop table" not in flat + assert "truncate" not in flat diff --git a/tradein-mvp/backend/tests/test_rbac.py b/tradein-mvp/backend/tests/test_rbac.py index 39c50e51..601287f3 100644 --- a/tradein-mvp/backend/tests/test_rbac.py +++ b/tradein-mvp/backend/tests/test_rbac.py @@ -277,8 +277,11 @@ def test_rbac_guard_unknown_user_blocked_on_non_admin_path(client: TestClient) - def test_rbac_expired_denied_on_non_admin_api(client: TestClient) -> None: """#R2-H3: revoked (role=expired, paths:[] deny:/**) НЕ достаёт non-admin API. Раньше rbac_guard гейтил только /admin/* → expired имел полный non-admin доступ - (POST /search экспорт листингов и т.д.). Теперь scope-blocked → 403.""" - for user in ("praktika",): # role=expired (trial отозван; user2 восстановлен 2026-07-13) + (POST /search экспорт листингов и т.д.). Теперь scope-blocked → 403. + + Fixture-юзер: expiredtest (temp QA, roles.yaml) — praktika перестал быть + expired-фикстурой 2026-07-27 (доступ восстановлен, роль теперь pilot).""" + for user in ("expiredtest",): for path in ("/api/v1/search", "/api/v1/trade-in/dummy"): resp = client.get(path, headers={"X-Authenticated-User": user}) assert resp.status_code == 403, f"{user} {path}: {resp.status_code}" @@ -287,11 +290,13 @@ def test_rbac_expired_denied_on_non_admin_api(client: TestClient) -> None: def test_rbac_expired_allowed_on_bootstrap(client: TestClient) -> None: """expired ДОЛЖЕН достучаться до /me (получить role=expired → trial-экран) и - /brand/* (брендинг trial-экрана) — иначе UX сломан. Bootstrap-исключение.""" - resp_me = client.get("/api/v1/me", headers={"X-Authenticated-User": "praktika"}) + /brand/* (брендинг trial-экрана) — иначе UX сломан. Bootstrap-исключение. + + Fixture-юзер: expiredtest (см. test_rbac_expired_denied_on_non_admin_api).""" + resp_me = client.get("/api/v1/me", headers={"X-Authenticated-User": "expiredtest"}) assert resp_me.status_code == 200, resp_me.text assert resp_me.json()["role"] == "expired" - resp_brand = client.get("/api/v1/brand/dummy", headers={"X-Authenticated-User": "praktika"}) + resp_brand = client.get("/api/v1/brand/dummy", headers={"X-Authenticated-User": "expiredtest"}) assert resp_brand.status_code == 200, resp_brand.text