lekss361/gendesign

Fork 0

fix(tradein/auth): единый источник ролей — БД first, YAML только legacy-fallback (закрывает эскалацию до admin и 403 своим) #3331

Merged

bot-backend merged 3 commits from fix/3316-role-single-source into main

2026-09-05 17:37:38 +00:00

bot-backend commented

2026-09-02 09:50:52 +00:00

Collaborator

Closes #3316 [SECURITY][P1]. Аудит 01-02.09, линза payments-auth. Двусторонний дефект одного корня: роль жила в roles.yaml, пользователи — в БД, никто не сверял.

Резолвер (`app/core/auth.py::get_role`)

Один SELECT role FROM tradein_users WHERE username=:username (имя таблицы из фиксированного словаря, значение — bind): непустая роль из БД — ответ; строки нет / роль пуста / реестр недоступен — прежний путь по roles.yaml, KeyError если нет и там. Вызывающие не тронуты — все уже ходят через эту функцию (rbac, _assert_estimate_access, /history, team, account_quota).

Закрывает обе стороны:

эскалация: сотрудник, созданный менеджером с именем admintest из YAML, резолвится в employee из БД, не в admin из YAML; плюс defense-in-depth — create_employee отказывает на имена, числящиеся в YAML с ролью ≠ employee;
403 своим: сотрудник вне YAML резолвится по DB-роли и читает свою оценку.

Побочные правки, без которых поведение бы поехало

rbac_guard/get_user_scope: роль из реестра → DB_ROLE_PATHS-матчер (иначе 403 на всё);
_batch_quota_status: роли из уже прочитанных строк (иначе N+1 — закреплено существующим тестом);
право персонального unlimited осталось за YAML в обоих местах — иначе override начал бы работать там, где раньше молча игнорировался.

Почему 13 прод-юзеров не сменят роль

tradein_users.role — NOT NULL CHECK (миграция 192), пустой роли не бывает; строка есть только у заведённых через team-API/сид 193. Чисто-legacy YAML-логины идут прежней веткой (тест сверяет ВЕСЬ маппинг roles.yaml). У praktika DB-роль manager — она и до фикса на реальном периметре шла из БД (сессионная ветка; legacy trusted-header снаружи срезан Caddy с #2558). Прод-SELECT перед мержем — отдельным комментарием ниже.

Тесты

Новый test_role_single_source.py (5), релевантные прогоны: 247 passed + 255 passed, 3 skipped. Фальсификация (снятие 3 строк DB-first): 2 красных ПО ЗНАЧЕНИЮ — assert 'admin' == 'employee' (эскалация) и HTTPException: 403: user not in roles config на СВОЕЙ оценке.

Прод-приёмка после деплоя

роли всех существующих юзеров не изменились (снимок до/после);
create_employee с именем admin → отказ;
buyer1/логины живы (YAML-ветка works).

Closes #3316 [SECURITY][P1]. Аудит 01-02.09, линза payments-auth. Двусторонний дефект одного корня: роль жила в roles.yaml, пользователи — в БД, никто не сверял. ## Резолвер (`app/core/auth.py::get_role`) Один `SELECT role FROM tradein_users WHERE username=:username` (имя таблицы из фиксированного словаря, значение — bind): непустая роль из БД — ответ; строки нет / роль пуста / реестр недоступен — прежний путь по roles.yaml, KeyError если нет и там. Вызывающие не тронуты — все уже ходят через эту функцию (rbac, `_assert_estimate_access`, /history, team, account_quota). Закрывает обе стороны: - **эскалация**: сотрудник, созданный менеджером с именем `admintest` из YAML, резолвится в employee из БД, не в admin из YAML; плюс defense-in-depth — `create_employee` отказывает на имена, числящиеся в YAML с ролью ≠ employee; - **403 своим**: сотрудник вне YAML резолвится по DB-роли и читает свою оценку. ## Побочные правки, без которых поведение бы поехало - `rbac_guard`/`get_user_scope`: роль из реестра → `DB_ROLE_PATHS`-матчер (иначе 403 на всё); - `_batch_quota_status`: роли из уже прочитанных строк (иначе N+1 — закреплено существующим тестом); - право персонального `unlimited` осталось за YAML в обоих местах — иначе override начал бы работать там, где раньше молча игнорировался. ## Почему 13 прод-юзеров не сменят роль `tradein_users.role` — NOT NULL CHECK (миграция 192), пустой роли не бывает; строка есть только у заведённых через team-API/сид 193. Чисто-legacy YAML-логины идут прежней веткой (тест сверяет ВЕСЬ маппинг roles.yaml). У praktika DB-роль manager — она и до фикса на реальном периметре шла из БД (сессионная ветка; legacy trusted-header снаружи срезан Caddy с #2558). Прод-SELECT перед мержем — отдельным комментарием ниже. ## Тесты Новый `test_role_single_source.py` (5), релевантные прогоны: `247 passed` + `255 passed, 3 skipped`. Фальсификация (снятие 3 строк DB-first): 2 красных ПО ЗНАЧЕНИЮ — `assert 'admin' == 'employee'` (эскалация) и `HTTPException: 403: user not in roles config` на СВОЕЙ оценке. ## Прод-приёмка после деплоя - роли всех существующих юзеров не изменились (снимок до/после); - `create_employee` с именем `admin` → отказ; - buyer1/логины живы (YAML-ветка works).

bot-backend added 1 commit 2026-09-02 09:50:53 +00:00

fix(tradein): резолвить роль из реестра, roles.yaml — только fallback (#3316 )

CI Trade-In / changes (pull_request) Successful in 12s

Details

CI / changes (pull_request) Successful in 15s

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Failing after 5m27s

Details

4feb61c006

Роль жила в двух местах сразу: люди заводятся в БД (`tradein_users.role`),
а `get_role` читал ТОЛЬКО `auth/roles.yaml` — и никто эти два источника не
сверял. Дефект двусторонний:

  * вверх: менеджер заводил сотрудника с именем, которое уже числится в
    roles.yaml админом (проверялись лишь regex и уникальность в БД) — на
    входе тот получал admin из YAML, то есть чтение ЛЮБОЙ чужой оценки
    (admin проходит мимо ownership-check в trade_in.py) и безлимитную квоту;
  * вниз: сотрудник, которого в roles.yaml нет, ловил KeyError → 403 на
    СОБСТВЕННУЮ оценку.

Источник теперь один и лечится один раз — в `app.core.auth.get_role`:
реестр (`tradein_users.role` / `auth.users.role`) спрашивается первым,
roles.yaml остаётся fallback для legacy-юзеров, у которых строки в реестре
нет. Реестр недоступен → тоже fallback: падение БД не выключает legacy-вход.
Вызывающие (rbac, trade_in, team, account_quota) не менялись.

Сопутствующее, чтобы поведение существующих аккаунтов не поехало:
  * rbac_guard выбирает матчер путей по РОДУ роли (роль реестра → DB_ROLE_PATHS),
    иначе employee/manager на legacy-пути получил бы 403 на всё;
  * get_user_scope отдаёт scope роли реестра из того же DB_ROLE_PATHS;
  * право на персональный `unlimited` осталось за roles.yaml (account_quota +
    _batch_quota_status) — фикс убирает эскалацию, а не раздаёт новую;
  * `_batch_quota_status` берёт роли из уже прочитанных строк — иначе список
    «Команды» снова стал бы N+1.

Defense-in-depth: create_employee отдаёт 409 на username, за которым в
roles.yaml числится не-employee роль.

Light1YT referenced this pull request from a commit

2026-09-02 09:59:23 +00:00

test(tradein): прод-конфигурация ролей в приёмке #3316 + компромисс фолбэка

Light1YT added 1 commit 2026-09-02 09:59:24 +00:00

test(tradein): прод-конфигурация ролей в приёмке #3316 + компромисс фолбэка

CI Trade-In / changes (pull_request) Successful in 10s

Details

CI / changes (pull_request) Successful in 11s

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Failing after 5m8s

Details

cde95aa9f6

Review PR #3331: приёмка «роли не изменились» гонялась с ПУСТЫМ реестром, а в
проде строка в БД есть у 12 из 13 юзеров и DB-роль ИНАЯ (kopylov: manager при
YAML pilot, user1: employee при YAML pilot). Добавлены два кейса именно этой
конфигурации:

  * YAML pilot + реестр employee → employee, и scope не поехал: allow/deny
    DB_ROLE_PATHS['employee'] сверяются со списками роли pilot из roles.yaml
    целиком — дрейф ЛЮБОГО из двух списков теперь красный тест, а не тихо
    потерянный/выданный раздел в проде;
  * YAML pilot + реестр manager → manager, и лишних путей на tradein-периметре
    нет: manager отличается от employee ровно префиксом /api/v1/team/** (вне
    /trade-in/**), deny-списки совпадают.

Докстринг `_registry_role`: зафиксирован компромисс — при недоступном реестре
фолбэк временно возвращает авторитетность roles.yaml, то есть состояние, которое
фикс и лечит. Сегодня безопасно (прод-коллизий имён нет, новые закрыты
409-гвардом create_employee); появится коллизия — ветку менять на fail-closed.

Light1YT added 1 commit 2026-09-02 11:46:53 +00:00

test(tradein): изолировать test_rbac.py от состояния реестра (#3316 )

CI Trade-In / changes (pull_request) Successful in 9s

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI / changes (pull_request) Successful in 11s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 5m6s

Details

18e0f1d999

CI-красное на голове ветки: 4 теста в tests/test_rbac.py ждали YAML-роль
(kopylov=pilot, user1=pilot), а в CI-базе реестр засеян миграцией 193
(kopylov=manager, user*=employee) — DB-first резолвер честно отдавал роль из БД.
Локально те же тесты были зелёными ровно потому, что БД нет и работал
YAML-fallback: результат файла зависел от ОКРУЖЕНИЯ, а такой тест не проверяет
ничего.

Чинится не подгонкой чисел в ассертах, а изоляцией: файл проверяет ИМЕННО
legacy-путь roles.yaml (разбор файла, globs, guard и /me на trusted-header), и
теперь заявляет это явно — autouse-фикстура `_legacy_yaml_only` глушит реестр
(`_registry_role` → None). Ассерты на YAML-роли после этого законны в любом
окружении. Приоритет реестра, эквивалентность scope employee↔pilot и
конфигурация kopylov (DB manager + YAML pilot) покрыты отдельно —
tests/test_role_single_source.py.

Проверено обоими способами: полный `pytest tests` без сида и он же с
плагином-имитацией засеянного реестра (подменяется тот же шов, что и в проде,
`identity_store.identity_session`) — 5290 passed, 35 skipped в обоих.