From abb9398f3fec260f31079119084b6563a1e106a7 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 31 Jul 2026 18:20:03 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/rbac):=20=D1=81=D0=BA=D1=80=D1=8B?= =?UTF-8?q?=D1=82=D1=8C=20=C2=AB=D0=94=D0=BE=D0=BB=D1=8F=20=D0=B2=20=D0=BF?= =?UTF-8?q?=D1=80=D0=BE=D0=B4=D0=B0=D0=B6=D0=B5=C2=BB=20=D0=B8=20=C2=AB?= =?UTF-8?q?=D0=9A=D1=8D=D1=88=C2=BB=20=D0=BE=D1=82=20=D0=BA=D0=BB=D0=B8?= =?UTF-8?q?=D0=B5=D0=BD=D1=82=D1=81=D0=BA=D0=B8=D1=85=20=D0=B0=D0=BA=D0=BA?= =?UTF-8?q?=D0=B0=D1=83=D0=BD=D1=82=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Аккаунт praktika (DB-роль manager) видел оба пункта в топбаре на /trade-in/team. Это внутренние инструменты — аналитика рынка и состояние кэшей/скраперов, — клиентские аккаунты их видеть не должны (решение владельца продукта). Гейт один — deny-список роли, потому что все три места сверяются с ним через общий матчер: пункт меню (Topbar по scopePath из /me), страница (RouteGuard) и серверные ручки (rbac_guard). Правка только фронта спрятала бы пункт, оставив прямой URL и API открытыми. Закрыто для employee/manager (DB_ROLE_PATHS) и для legacy pilot (roles.yaml): /trade-in/sale-share/** /trade-in/cache/** /trade-in/api/v1/buildings/** /trade-in/api/v1/trade-in/cache-stats/** У cache-stats ГЛОБ, а не точный путь: точный паттерн — строгое равенство, его обходит трейлинг-слэш ('…/cache-stats/' → allowed=True), и защита держалась бы на Starlette redirect_slashes, а не на RBAC. Замерено после правки: все варианты (слэш, %2f, ./, ../) дают 403, утечек нет. Основной продукт не задет: buildings.py обслуживает ТОЛЬКО sale-share, секция «Продажи в доме» на экране оценки питается estimate-хендлерами. admin и analyst сознательно вне deny — запиннено тестом, иначе «синхронизация» списков закрыла бы их молча. Заодно починен КРАСНЫЙ pre-existing тест главного бэкенда: backend/tests/test_rbac.py::test_get_role_known_users ждал pilot у всех user1..user10, но user2 («Брусника») стал expired 2026-07-30. CI это пропустил — auth/roles.yaml не входит в paths-filter backend/**, из-за чего сьют не бежал. Тесты: 153 passed (tradein) + 24 passed (site-finder, было 23+1 failed). Новые — e2e через реальный rbac_guard по session-ветке (именно ею ходит praktika), пин deny_paths в выдаче /me, границы глоба и regression-guard'ы. Проверены снятием deny: 7 тестов краснеют, т.е. не тавтологии. --- auth/roles.yaml | 36 +++++ backend/tests/test_rbac.py | 15 +- .../backend/app/services/auth_session.py | 76 +++++++++- tradein-mvp/backend/tests/test_auth_api.py | 79 +++++++++- .../backend/tests/test_auth_session.py | 21 +++ tradein-mvp/backend/tests/test_rbac.py | 141 +++++++++++++++++- .../frontend/src/app/sale-share/page.tsx | 11 +- .../src/components/trade-in/Topbar.tsx | 25 +++- 8 files changed, 395 insertions(+), 9 deletions(-) diff --git a/auth/roles.yaml b/auth/roles.yaml index ef999bff..12131be7 100644 --- a/auth/roles.yaml +++ b/auth/roles.yaml @@ -39,6 +39,39 @@ roles: - "/admin/**" - "/api/v1/admin/**" - "/trade-in/api/v1/admin/**" + # Внутренние разделы, закрытые от клиентских аккаунтов (решение владельца + # продукта 2026-07-31): «Доля в продаже» — аналитика рынка, «Кэш» — + # состояние кэшей/скраперов. Зеркало deny-списка DB-ролей employee/manager + # (tradein-mvp/backend/app/services/auth_session.py: DB_ROLE_PATHS). + # + # Зачем копия здесь, если клиенты ходят session-cookie'ой: снаружи легаси + # trusted-header ветка НЕДОСТИЖИМА — с #2558 Caddy срезает входящий + # X-Authenticated-User на всём /trade-in/* (`header_up + # -X-Authenticated-User` в handle /trade-in/api/* и в @tradein), так что + # ни один клиентский аккаунт по ней не ходит. Паттерны нужны для другого: + # 1) ВНУТРИСЕТЕВОЙ dual-mode трафик — запросы изнутри gendesign_shared с + # валидным X-Internal-Auth-Secret; ими ходят QA-смоуки вида + # `docker exec tradein-backend curl localhost:8000 + # -H 'X-Authenticated-User: ...'` — они резолвятся именно через + # roles.yaml, и без этих строк смоук показал бы 200 там, где + # реальный клиент получает 403; + # 2) чтобы legacy-pilot не расходился с DB-employee, если dual-режим + # когда-нибудь снова окажется на периметре (откат #2558 / новый + # фронт-прокси) — тогда расхождение молча откроет разделы. + # НЕ удалять как «мёртвые»: они мёртвые только пока Caddy режет заголовок. + # + # Страницы + их API вместе: deny гейтит пункт меню (Topbar через /me), + # саму страницу (RouteGuard) и серверные ручки (rbac_guard). + # + # cache-stats закрыт ГЛОБОМ, а не точным путём, намеренно: точный паттерн + # обходится трейлинг-слэшем ('…/cache-stats/' не равен '…/cache-stats' → + # allowed), и защита повисала бы на Starlette redirect_slashes, а не на + # RBAC. '/**' → '^(?:/.*)?$': сам путь + слэш + подпути, + # но НЕ соседи по префиксу ('…/cache-statistics' не матчится). + - "/trade-in/sale-share/**" + - "/trade-in/cache/**" + - "/trade-in/api/v1/buildings/**" + - "/trade-in/api/v1/trade-in/cache-stats/**" analyst: # #962 (EPIC18, ТЗ §19): analyst видит ВСЁ (deals, insights, exports, # site-finder, analytics, concept) КРОМЕ admin/data-management. @@ -48,6 +81,9 @@ roles: # для любого role != "admin" → analyst авто-403 на admin-API без доп. кода. # deny ниже драйвит фронтовый RouteGuard (deny_paths из /me) для UI-gating # /admin/** страниц. + # NB: клиентский deny 2026-07-31 («Доля в продаже» / «Кэш», см. pilot выше) + # на analyst СОЗНАТЕЛЬНО не распространён — analyst внутренняя роль и оба + # раздела для неё рабочий инструмент. Это не забытая дыра. paths: - "/**" deny: diff --git a/backend/tests/test_rbac.py b/backend/tests/test_rbac.py index 3c2435dc..21ab8026 100644 --- a/backend/tests/test_rbac.py +++ b/backend/tests/test_rbac.py @@ -110,11 +110,24 @@ def client() -> TestClient: # --------------------------------------------------------------------------- +# Пилотные логины user1..user10 в auth/roles.yaml. user2 — «Брусника»: доступ +# закрыт владельцем продукта 2026-07-30, роль переведена pilot → expired. Это +# ЕДИНСТВЕННОЕ отклонение от «все userN = pilot», и оно ожидаемое; хардкод +# именно здесь, отдельной константой, а не магическим `if` в цикле. +_EXPIRED_PILOT_LOGINS = {"user2": "«Брусника», доступ закрыт 2026-07-30"} + + def test_get_role_known_users() -> None: + """Ловит рассинхрон auth/roles.yaml с ожиданиями теста: roles.yaml лежит вне + `backend/**`, поэтому правка ролей не попадает в paths-filter CI и такой + рассинхрон CI молча пропускает (так и случилось с user2 → expired).""" assert auth_mod.get_role("admin") == "admin" assert auth_mod.get_role("kopylov") == "pilot" for n in range(1, 11): - assert auth_mod.get_role(f"user{n}") == "pilot" + login = f"user{n}" + expected = "expired" if login in _EXPIRED_PILOT_LOGINS else "pilot" + why = _EXPIRED_PILOT_LOGINS.get(login, "обычный пилотный логин") + assert auth_mod.get_role(login) == expected, f"{login}: ожидали {expected} — {why}" def test_get_role_unknown_user_raises() -> None: diff --git a/tradein-mvp/backend/app/services/auth_session.py b/tradein-mvp/backend/app/services/auth_session.py index 35b27c9c..385444dc 100644 --- a/tradein-mvp/backend/app/services/auth_session.py +++ b/tradein-mvp/backend/app/services/auth_session.py @@ -196,17 +196,85 @@ def revoke_user_sessions(db: Session, user_id: int) -> None: # НЕ являются ключами auth/roles.yaml (тот файл — legacy Caddy trusted-header путь, # который этот эпик намеренно не трогает). Маппинг ниже даёт DB-ролям тот же # paths/deny-смысл, что и legacy-ролям, БЕЗ правки roles.yaml: -# employee -> те же права, что legacy pilot (/trade-in/** только). -# manager -> employee + задел /api/v1/team/** (роутер появится в #2554). +# employee -> клиентский доступ: весь /trade-in/** МИНУС внутренние разделы +# (см. deny ниже — раньше было «ровно как legacy pilot»). +# manager -> employee + /api/v1/team/** (дашборд команды, #2556). # admin -> полный доступ, как legacy admin. +# +# Почему «Доля в продаже» и «Кэш» в deny у ОБЕИХ клиентских ролей (2026-07-31, +# решение владельца продукта): это внутренние инструменты, а не продукт клиента. +# «Доля в продаже» — аналитика рынка (сколько квартир дома выставлено, срез по +# домам/ЖК), «Кэш» — состояние кэшей и скраперов. Клиентские аккаунты видеть их +# не должны; триггер — аккаунт praktika (DB-роль manager), у которого оба пункта +# висели в топбаре на /trade-in/team. +# +# Почему в deny И страницы (/trade-in/sale-share, /trade-in/cache), И их API +# (/trade-in/api/v1/buildings/**, /trade-in/api/v1/trade-in/cache-stats/**): один +# deny-список гейтит СРАЗУ ТРИ места, потому что все трое сверяются с ним через +# один и тот же матчер — +# 1) пункт меню: Topbar фильтрует NAV_ITEMS по scopePath из /me; +# 2) сама страница: RouteGuard проверяет абсолютный путь из /me; +# 3) серверные ручки: app.core.rbac.rbac_guard (deny проверяется ПЕРВЫМ, +# внешний путь реконструируется как '/trade-in' + path). +# Только страницы = пункт исчез, но прямой URL и API остались открыты; только +# API = мёртвый пункт меню с 403 на каждый фетч. +# +# Почему '/trade-in/api/v1/buildings/**' безопасно закрывать целиком: весь +# роутер app/api/v1/buildings.py обслуживает ТОЛЬКО раздел sale-share +# (/sale-share, /sale-share/summary, /{house_id}/listings). Экран оценки его не +# использует — секция «Продажи в доме» питается estimate-хендлерами +# (useEstimatePlacementHistory / useSalesVsListings), а BuildingListingsDrawer +# импортируется единственной страницей app/sale-share/page.tsx. +# +# NB (границы глоба): '/**' компилируется в '^(?:/.*)?$' — матчит +# сам prefix, его же с трейлинг-слэшем и подпути через '/', но НЕ соседей по +# префиксу (см. app.core.rbac._db_glob_match и app.core.auth._glob_to_regex). +# Поэтому '/trade-in/cache/**' не задевает '/trade-in/cache-stats', а +# '/trade-in/api/v1/trade-in/cache-stats/**' — не '/…/cache-statistics'. +# +# Почему у cache-stats ГЛОБ, а не «более точный» '/trade-in/api/v1/trade-in/ +# cache-stats': точный паттерн — это строгое равенство, и его обходит обычный +# трейлинг-слэш (измерено: '…/cache-stats/' → allowed=True). Сегодня от этого +# спасает только Starlette redirect_slashes (307 на путь без слэша → там уже +# 403), т.е. защита держалась бы на роутере, а не на RBAC — достаточно +# выключить redirect_slashes или сменить роутер, и deny тихо перестанет +# работать. Глоб закрывает и сам путь, и слэш, и любые будущие подпути. +# НЕ «уточнять» обратно до точного пути. +# +# NB (ограничение мини-матчера — читать перед копированием паттернов): +# DB_ROLE_PATHS и pilot.deny в auth/roles.yaml — зеркала по СМЫСЛУ, но матчеры +# у них РАЗНЫЕ. app.core.rbac._db_glob_match понимает ТОЛЬКО три формы: +# '/**' | '/**' | точный путь (строгое равенство). +# app.core.auth._glob_to_regex (roles.yaml) понимает сверх этого ещё одиночную +# '*' ('/foo/*' = один сегмент). Паттерн с одиночной '*', скопированный сюда из +# roles.yaml, станет ЛИТЕРАЛЬНОЙ строкой и МОЛЧА перестанет что-либо запрещать — +# без ошибки на импорте и без падения тестов, если на него нет прямого теста. +# Т.е. в DB_ROLE_PATHS допустимы только '/**', '/**' и точный путь; +# одиночная '*' здесь = silent no-op. DB_ROLE_PATHS: dict[str, tuple[list[str], list[str]]] = { "employee": ( ["/trade-in/**", "/trade-in/api/v1/**"], - ["/admin/**", "/api/v1/admin/**", "/trade-in/api/v1/admin/**"], + [ + "/admin/**", + "/api/v1/admin/**", + "/trade-in/api/v1/admin/**", + "/trade-in/sale-share/**", + "/trade-in/cache/**", + "/trade-in/api/v1/buildings/**", + "/trade-in/api/v1/trade-in/cache-stats/**", + ], ), "manager": ( ["/trade-in/**", "/trade-in/api/v1/**", "/api/v1/team/**"], - ["/admin/**", "/api/v1/admin/**", "/trade-in/api/v1/admin/**"], + [ + "/admin/**", + "/api/v1/admin/**", + "/trade-in/api/v1/admin/**", + "/trade-in/sale-share/**", + "/trade-in/cache/**", + "/trade-in/api/v1/buildings/**", + "/trade-in/api/v1/trade-in/cache-stats/**", + ], ), "admin": (["/**"], []), } diff --git a/tradein-mvp/backend/tests/test_auth_api.py b/tradein-mvp/backend/tests/test_auth_api.py index 8b55fb13..23997f98 100644 --- a/tradein-mvp/backend/tests/test_auth_api.py +++ b/tradein-mvp/backend/tests/test_auth_api.py @@ -191,6 +191,17 @@ def _build_test_app(store: _Store) -> FastAPI: assert session-identity wins over a client-forged header (#2552 spoof fix).""" return {"user": x_authenticated_user} + # Внутренние инструменты, закрытые от клиентских DB-ролей 2026-07-31 + # («Доля в продаже» / «Кэш»). Ручки настоящие (не заглушки rbac_guard'а), + # чтобы 403 приходил именно от scope-чека, а не от отсутствия роута. + @app.get("/api/v1/buildings/sale-share") + async def buildings_sale_share() -> dict: + return {"ok": True} + + @app.get("/api/v1/trade-in/cache-stats") + async def tradein_cache_stats() -> dict: + return {"ok": True} + def _override_get_db(): # generator dependency — matches app.core.db.get_db shape yield _FakeDB(store) @@ -376,6 +387,12 @@ def test_me_with_session_cookie_returns_db_role(client: TestClient, store: _Stor assert body["role"] == "employee" assert "/trade-in/**" in body["allowed_paths"] assert "/admin/**" in body["deny_paths"] + # Пункты меню «Доля в продаже» / «Кэш» прячет Topbar, фильтруя NAV_ITEMS по + # deny_paths ИЗ /me — т.е. видимость держится на ЭТОМ выводе, а не только на + # DB_ROLE_PATHS. Сборка dict-а в app/api/v1/me.py может регрессировать + # независимо от get_db_role_scope, поэтому пиним её здесь. + assert "/trade-in/sale-share/**" in body["deny_paths"] + assert "/trade-in/cache/**" in body["deny_paths"] assert body["display_name"] == "Алиса" assert body["org"] == "ООО Ромашка" assert body["email"] == "alice@romashka.ru" @@ -387,7 +404,12 @@ def test_me_manager_role_gets_team_path(client: TestClient, store: _Store) -> No resp = client.get("/api/v1/me") assert resp.status_code == 200 - assert "/api/v1/team/**" in resp.json()["allowed_paths"] + body = resp.json() + assert "/api/v1/team/**" in body["allowed_paths"] + # Тот же пин, что и для employee: manager (роль praktika) не должен получать + # из /me deny-список без внутренних разделов — иначе пункты вернутся в топбар. + assert "/trade-in/sale-share/**" in body["deny_paths"] + assert "/trade-in/cache/**" in body["deny_paths"] def test_me_without_cookie_dual_mode_legacy_still_works(client: TestClient) -> None: @@ -477,6 +499,61 @@ def test_session_user_can_reach_tradein_but_not_admin(client: TestClient, store: assert denied.status_code == 403 +# --------------------------------------------------------------------------- +# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов — +# СКВОЗЬ РЕАЛЬНЫЙ rbac_guard по SESSION-ветке (from_session=True). +# +# Тесты в tests/test_rbac.py проверяют матчеры напрямую + guard по ЛЕГАСИ +# trusted-header ветке (is_path_allowed / roles.yaml). Но в проде клиентские +# аккаунты (praktika и прочие DB-юзеры) ходят именно session-cookie'ой, где +# scope считает ДРУГАЯ ветка — `_db_role_path_allowed(role, external_path)`. +# Без тестов ниже её можно было сломать, не уронив ни одного теста. +# +# Пути тут — ВНУТРЕННИЕ (Caddy срезает внешний /trade-in), rbac_guard +# восстанавливает внешний как '/trade-in' + path. +# --------------------------------------------------------------------------- + +_INTERNAL_TOOL_API = ("/api/v1/buildings/sale-share", "/api/v1/trade-in/cache-stats") + + +def test_session_manager_denied_on_internal_tool_api(client: TestClient, store: _Store) -> None: + store.add_user("mgr", hash_password("Secret123!"), role="manager") + client.post("/api/v1/auth/login", json={"username": "mgr", "password": "Secret123!"}) + + for path in _INTERNAL_TOOL_API: + resp = client.get(path) + assert resp.status_code == 403, f"manager {path}: {resp.status_code} {resp.text}" + assert "forbidden for role" in resp.json()["detail"].lower() + + # ...и при этом основной продукт для той же сессии открыт (иначе тест выше + # проходил бы и на «сломали scope целиком»). + ok = client.get("/api/v1/trade-in/dummy") + assert ok.status_code == 200, ok.text + + +def test_session_employee_denied_on_internal_tool_api(client: TestClient, store: _Store) -> None: + store.add_user("emp", hash_password("Secret123!"), role="employee") + client.post("/api/v1/auth/login", json={"username": "emp", "password": "Secret123!"}) + + for path in _INTERNAL_TOOL_API: + resp = client.get(path) + assert resp.status_code == 403, f"employee {path}: {resp.status_code} {resp.text}" + assert "forbidden for role" in resp.json()["detail"].lower() + + ok = client.get("/api/v1/trade-in/dummy") + assert ok.status_code == 200, ok.text + + +def test_session_admin_keeps_internal_tool_api(client: TestClient, store: _Store) -> None: + """Контрольная группа: DB-роль admin ('/**') разделы по-прежнему видит.""" + store.add_user("root", hash_password("Secret123!"), role="admin") + client.post("/api/v1/auth/login", json={"username": "root", "password": "Secret123!"}) + + for path in _INTERNAL_TOOL_API: + resp = client.get(path) + assert resp.status_code == 200, f"admin {path}: {resp.text}" + + # --------------------------------------------------------------------------- # #2552 post-review CRITICAL fix: session identity must win over a spoofed # client-sent X-Authenticated-User header (was a skip-if-present bug — the diff --git a/tradein-mvp/backend/tests/test_auth_session.py b/tradein-mvp/backend/tests/test_auth_session.py index 650186fa..b45ef98a 100644 --- a/tradein-mvp/backend/tests/test_auth_session.py +++ b/tradein-mvp/backend/tests/test_auth_session.py @@ -294,6 +294,27 @@ def test_get_db_role_scope_manager_adds_team_path() -> None: assert "/admin/**" in deny +# «Доля в продаже» и «Кэш» — внутренние инструменты (аналитика рынка / состояние +# кэшей и скраперов), клиентские роли их не видят (решение владельца 2026-07-31). +# В deny И страницы, И их API: один список гейтит пункт меню (Topbar через /me), +# страницу (RouteGuard) и серверные ручки (rbac_guard). +_INTERNAL_TOOL_DENY = ( + "/trade-in/sale-share/**", + "/trade-in/cache/**", + "/trade-in/api/v1/buildings/**", + # Глоб, а не точный путь: точный обходится трейлинг-слэшем (см. NB в + # app.services.auth_session над DB_ROLE_PATHS). + "/trade-in/api/v1/trade-in/cache-stats/**", +) + + +def test_get_db_role_scope_client_roles_deny_internal_tools() -> None: + for role in ("manager", "employee"): + _, deny = svc.get_db_role_scope(role) + for pattern in _INTERNAL_TOOL_DENY: + assert pattern in deny, f"{role} deny missing {pattern}" + + def test_get_db_role_scope_admin_full_access() -> None: paths, deny = svc.get_db_role_scope("admin") assert paths == ["/**"] diff --git a/tradein-mvp/backend/tests/test_rbac.py b/tradein-mvp/backend/tests/test_rbac.py index 6fec305c..8c8b64ab 100644 --- a/tradein-mvp/backend/tests/test_rbac.py +++ b/tradein-mvp/backend/tests/test_rbac.py @@ -34,7 +34,7 @@ from fastapi.testclient import TestClient from app.api.v1 import me as me_router from app.core import auth as auth_mod -from app.core.rbac import rbac_guard +from app.core.rbac import _db_role_path_allowed, rbac_guard @pytest.fixture(autouse=True) @@ -69,6 +69,16 @@ def _build_test_app() -> FastAPI: async def brand_dummy() -> dict: return {"ok": True} + # Внутренние инструменты, закрытые от клиентских ролей 2026-07-31 + # (см. _INTERNAL_TOOL_PATHS ниже): API «Доли в продаже» и «Кэша». + @app.get("/api/v1/buildings/sale-share") + async def buildings_sale_share() -> dict: + return {"ok": True} + + @app.get("/api/v1/trade-in/cache-stats") + async def tradein_cache_stats() -> dict: + return {"ok": True} + @app.get("/health") async def health() -> dict: return {"status": "ok"} @@ -355,3 +365,132 @@ def test_rbac_guard_no_header_on_non_admin_path_returns_401(client: TestClient) resp = client.get("/api/v1/me") assert resp.status_code == 401 assert "no authenticated user" in resp.json()["detail"].lower() + + +# --------------------------------------------------------------------------- +# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов +# --------------------------------------------------------------------------- +# +# Решение владельца продукта: оба раздела — внутренние инструменты (аналитика +# рынка / состояние кэшей и скраперов), клиентские аккаунты их видеть не должны +# (триггер — praktika, DB-роль manager, у которого оба пункта висели в топбаре). +# Deny заведён в DB_ROLE_PATHS (employee/manager) и зеркально в pilot.deny +# (auth/roles.yaml) — страницы И их API, чтобы гейт сработал сразу в трёх местах: +# пункт меню (Topbar через /me), страница (RouteGuard), ручки (rbac_guard). + +# Внешние пути (как их видит RBAC-конфиг): 2 страницы + все API раздела. +# Проверяются матчерами напрямую — регистрировать их в тестовом app не нужно. +_INTERNAL_TOOL_PATHS = ( + "/trade-in/sale-share", + "/trade-in/cache", + "/trade-in/api/v1/buildings/sale-share", + # Остальные ручки роутера buildings.py — глоб '/…/buildings/**' обязан + # покрывать и их, включая параметризованную (самый вероятный кандидат на + # переезд под другой префикс — тогда этот тест упадёт, а не промолчит). + "/trade-in/api/v1/buildings/sale-share/summary", + "/trade-in/api/v1/buildings/123/listings", + "/trade-in/api/v1/trade-in/cache-stats", + # Трейлинг-слэш: точный паттерн его НЕ ловил (allowed=True), защита висела + # на Starlette redirect_slashes — поэтому deny переведён на глоб-форму. + "/trade-in/api/v1/trade-in/cache-stats/", +) + +# Основной продукт — не должен быть задет deny выше. +_CORE_PRODUCT_PATHS = ("/trade-in/", "/trade-in/api/v1/trade-in/estimate") + + +def test_db_roles_denied_on_internal_tool_paths() -> None: + """manager/employee (DB-роли, session-auth ветка rbac_guard) → deny.""" + for role in ("manager", "employee"): + for path in _INTERNAL_TOOL_PATHS: + assert not _db_role_path_allowed(role, path), f"{role} must not reach {path}" + + +def test_db_admin_still_allowed_on_internal_tool_paths() -> None: + for path in _INTERNAL_TOOL_PATHS: + assert _db_role_path_allowed("admin", path), f"admin lost access to {path}" + + +def test_yaml_roles_deliberately_outside_client_deny() -> None: + """Пиннит ОБРАТНУЮ сторону правки 2026-07-31: роли, которые сознательно НЕ + попали под клиентский deny. + + Без этого теста «синхронизация» deny-списков между ролями в auth/roles.yaml + (соблазн скопировать pilot.deny в соседей) молча отрезала бы админа от его + же инструментов, и ни один тест бы не упал: roles.yaml лежит ВНЕ paths-фильтров + `backend/**` и `tradein-mvp/**`, т.е. CI такую правку не проверяет вовсе — + ровно тот класс рассинхрона, что уже случился с user2 (см. + backend/tests/test_rbac.py::test_get_role_known_users). + + `analyst` — внутренняя роль (paths "/**", deny только admin-управление); + решение не распространять на неё клиентский deny осознанное, а не забытое. + """ + for path in _INTERNAL_TOOL_PATHS: + assert auth_mod.is_path_allowed("admin", path), f"admin lost access to {path}" + assert auth_mod.is_path_allowed("analyst", path), ( + f"analyst lost access to {path} — если это намеренно, обнови этот тест " + f"и комментарий у роли analyst в auth/roles.yaml" + ) + + +def test_db_roles_still_allowed_on_core_product() -> None: + """Регресс: оценка (основной продукт) для клиентских ролей не задета.""" + for role in ("manager", "employee"): + for path in _CORE_PRODUCT_PATHS: + assert _db_role_path_allowed(role, path), f"{role} lost access to {path}" + + +def test_legacy_pilot_denied_on_internal_tool_paths() -> None: + """Зеркало в auth/roles.yaml: пока auth_mode=dual, legacy-pilot не должен + видеть то, что DB-employee уже не видит.""" + for path in _INTERNAL_TOOL_PATHS: + assert not auth_mod.is_path_allowed("pilot", path), f"pilot must not reach {path}" + for path in _CORE_PRODUCT_PATHS: + assert auth_mod.is_path_allowed("pilot", path), f"pilot lost access to {path}" + + +def test_rbac_guard_blocks_pilot_on_internal_tool_api(client: TestClient) -> None: + """Тот же deny через РЕАЛЬНЫЙ guard (legacy trusted-header ветка): ручки + sale-share/кэша отдают 403, а не только прячутся из меню.""" + for path in ("/api/v1/buildings/sale-share", "/api/v1/trade-in/cache-stats"): + resp = client.get(path, headers={"X-Authenticated-User": "kopylov"}) + assert resp.status_code == 403, f"pilot {path}: {resp.status_code}" + assert "forbidden for role" in resp.json()["detail"].lower() + + +def test_rbac_guard_admin_keeps_internal_tool_api(client: TestClient) -> None: + for path in ("/api/v1/buildings/sale-share", "/api/v1/trade-in/cache-stats"): + resp = client.get(path, headers={"X-Authenticated-User": "admin"}) + assert resp.status_code == 200, f"admin {path}: {resp.text}" + + +def test_internal_deny_globs_do_not_leak_to_sibling_prefixes() -> None: + """Граничный случай: '/**' компилируется в '^(?:/.*)?$' — + матчит сам prefix, prefix со слэшем и подпути через '/', но НЕ соседей по + префиксу (дефис не матчится). Именно поэтому глоб-форма безопасна как + замена точного пути: '/trade-in/cache/**' не задевает страницу + '/trade-in/cache-stats', а '/…/trade-in/cache-stats/**' — не гипотетическую + '/…/trade-in/cache-statistics'. Фиксируем семантику тестом: если её однажды + поменяют (напр. на префиксный startswith), соседние пути начнут молча + падать в 403.""" + siblings_allowed = ( + "/trade-in/cache-stats", + "/trade-in/sale-share-report", + "/trade-in/api/v1/trade-in/cache-statistics", + ) + section_denied = ( + "/trade-in/cache/detail", + "/trade-in/sale-share/123", + "/trade-in/api/v1/trade-in/cache-stats/reset", + ) + for role in ("manager", "employee"): + for path in siblings_allowed: + assert _db_role_path_allowed(role, path), f"{role} lost sibling {path}" + # ...при том что сам раздел и его подпути закрыты. + for path in section_denied: + assert not _db_role_path_allowed(role, path), f"{role} must not reach {path}" + + for path in siblings_allowed: + assert auth_mod.is_path_allowed("pilot", path), f"pilot lost sibling {path}" + for path in section_denied: + assert not auth_mod.is_path_allowed("pilot", path), f"pilot must not reach {path}" diff --git a/tradein-mvp/frontend/src/app/sale-share/page.tsx b/tradein-mvp/frontend/src/app/sale-share/page.tsx index df6eae51..e0c1fa35 100644 --- a/tradein-mvp/frontend/src/app/sale-share/page.tsx +++ b/tradein-mvp/frontend/src/app/sale-share/page.tsx @@ -5,7 +5,16 @@ * Порог % → дома вторички, где доля квартир, выставленных на продажу, ≥ порога. * Сигнал для девелопера: расселение / инвест-выход / проблемный дом. * - * Доступ: pilot + admin (RBAC roles.yaml: pilot paths `/trade-in/**`). + * Доступ: ТОЛЬКО admin (с 2026-07-31). Раздел признан внутренним инструментом — + * клиентские аккаунты его не видят: явный deny `/trade-in/sale-share/**` + + * `/trade-in/api/v1/buildings/**` заведён для DB-ролей employee/manager + * (`app/services/auth_session.py: DB_ROLE_PATHS`) и для legacy `pilot` + * (`auth/roles.yaml`). Роль `analyst` сознательно не в deny — внутренняя. + * + * NB: короткий адрес `gendsgn.ru/sale-share` (301 → сюда, см. Caddyfile) после + * этого ведёт на NoAccessScreen для всех, кроме admin. Если раздел снова станет + * продаваемым продуктом, одним снятием deny не обойтись: нужен per-account + * carve-out — сейчас скоуп только ролевой, выдать его отдельному клиенту нечем. */ import { useCallback, useMemo, useRef, useState } from "react"; import dynamic from "next/dynamic"; diff --git a/tradein-mvp/frontend/src/components/trade-in/Topbar.tsx b/tradein-mvp/frontend/src/components/trade-in/Topbar.tsx index c7b88064..7769adc9 100644 --- a/tradein-mvp/frontend/src/components/trade-in/Topbar.tsx +++ b/tradein-mvp/frontend/src/components/trade-in/Topbar.tsx @@ -107,6 +107,23 @@ interface TopbarProps { * путь, чтобы pilot их не видел в навигации. Direct URL access на * `/trade-in/scrapers/avito` НЕ блокируется (RouteGuard следует yaml). Если * нужна полная блокировка — добавить `/trade-in/scrapers/**` в pilot.deny. + * + * Исключение из этого caveat — `sale-share` и `cache` (2026-07-31): для них + * заведён ЯВНЫЙ deny (`/trade-in/sale-share/**`, `/trade-in/cache/**` + их API) + * в `DB_ROLE_PATHS` (employee/manager) и в `pilot.deny` (auth/roles.yaml). + * Т.е. это НЕ scopePath-трюк, как у скрапперов: гейт реальный, а не только + * косметический. + * + * Но точность важнее красивой формулировки — где именно он стоит: + * - пункт меню исчезает (фильтр ниже, deny из `/me`); + * - страница по прямому URL отдаёт HTTP **200** с HTML (Next.js рендерит + * маршрут всегда) — её закрывает КЛИЕНТСКИЙ `RouteGuard` (app/layout.tsx), + * рисуя NoAccessScreen вместо контента; + * - единственный СЕРВЕРНЫЙ рубеж — API: `/api/v1/buildings/**` и + * `/api/v1/trade-in/cache-stats/**` дают 403 из `rbac_guard`. + * Данные без API недостижимы, поэтому 200 на HTML безвреден — но не читай это + * как «страница блокируется на сервере»: следующий, кто добавит сюда раздел с + * SSR-данными, обязан закрывать именно его API, а не только этот список. */ const NAV_ITEMS: Array<{ key: ActiveTab; @@ -122,7 +139,11 @@ const NAV_ITEMS: Array<{ roleGate?: (role: Role) => boolean; }> = [ { key: "estimate", href: "/", scopePath: "/trade-in/", label: "Оценка" }, - // Доля квартир дома в продаже — доступно pilot (scopePath под /trade-in/**). + // Доля квартир дома в продаже — ВНУТРЕННИЙ инструмент (аналитика рынка). + // Скрыт для employee/manager/pilot явным deny `/trade-in/sale-share/**` + // (DB_ROLE_PATHS + auth/roles.yaml), а не scopePath-трюком как у скрапперов: + // scopePath остаётся честным путём страницы, фильтр ниже — прежний + // isPathAllowed, просто deny побеждает allow `/trade-in/**`. { key: "sale-share", href: "/sale-share", @@ -130,6 +151,8 @@ const NAV_ITEMS: Array<{ label: "Доля в продаже", }, { key: "history", href: "/history", scopePath: "/trade-in/history", label: "История" }, + // Кэш — внутренний инструмент (состояние кэшей/скраперов). Скрыт тем же + // способом, что и sale-share выше: явный deny `/trade-in/cache/**`. { key: "cache", href: "/cache", scopePath: "/trade-in/cache", label: "Кэш" }, // Скраперы — admin-only UI. Маппим на admin-deny path, чтобы pilot их не видел. { -- 2.45.3