fix(rbac): не отвечать 401 за несуществующий путь (#3324)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 15s
CI / changes (pull_request) Successful in 18s
CI Trade-In / browser-tests (pull_request) Has been skipped
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 5m36s

rbac_guard — HTTP-middleware, он отрабатывает до роутинга и потому отвечал
401 с rbac-текстом даже на пути, которых в приложении нет. Аноним получал
бесплатный оракул периметра: мусор под «интересным» префиксом давал 401, а
такой же мусор под публичным префиксом — 404 роутера, то есть выключенная
ручка была отличима от несуществующей.

Guard теперь пропускает запрос дальше, если ни один маршрут роутера не
матчится (Match.NONE у всех) — 404 отдаёт тот же роутер, что и на любой
другой мусор. Существующие маршруты не затронуты: Match.PARTIAL (путь есть,
метод другой) по-прежнему идёт в guard, реальный закрытый маршрут анониму
даёт 401, публичный — работает без идентичности.
This commit is contained in:
bot-backend 2026-09-05 22:51:22 +05:00
parent 99f112db0b
commit 0ef1880e55
3 changed files with 73 additions and 3 deletions

View file

@ -35,6 +35,7 @@ from typing import Any
from fastapi import Request
from fastapi.responses import JSONResponse, Response
from starlette.routing import Match
from app.core.auth import get_role, is_path_allowed
from app.core.config import settings
@ -181,6 +182,32 @@ def _db_role_path_allowed(role: str, path: str) -> bool:
return any(_db_glob_match(p, path) for p in paths)
def _path_is_routed(request: Request) -> bool:
"""Есть ли у пути хоть один маршрут в роутере приложения.
#3324: guard — HTTP-middleware, он отрабатывает ДО роутинга, поэтому раньше
отвечал 401 и на пути, которых в приложении нет вовсе. Анониму этого хватало,
чтобы бесплатно разведать периметр: мусор под «интересным» префиксом
(``/api/public/whatever``) давал 401 с rbac-текстом, а мусор под публичным
префиксом 404 роутера. Выключенная/закрытая ручка отличалась от
несуществующей. Несуществующий путь обязан отвечать одинаково независимо от
префикса, поэтому такие запросы пропускаются дальше 404 отдаёт роутер, тот
же самый, что и на любой другой мусор.
Ослабления нет: ``Match.NONE`` по ВСЕМ маршрутам значит, что выполнять
нечего хендлера, до которого можно было бы дотянуться, не существует.
``Match.PARTIAL`` (путь есть, метод другой) считается маршрутом и идёт в
guard как раньше там путь реально существует, скрывать нечего.
Неизвестное приложение (``scope["app"]`` не выставлен) ведём себя как
раньше, то есть отдаём запрос в guard.
"""
router = getattr(request.scope.get("app"), "router", None)
if router is None:
return True
return any(route.matches(request.scope)[0] != Match.NONE for route in router.routes)
def _propagate_authenticated_user(request: Request, username: str) -> None:
"""Инжектит ``X-Authenticated-User`` в ASGI scope — ПЕРЕЗАПИСЫВАЯ, а не
только добавляя при отсутствии, чтобы ``RateLimitMiddleware``/
@ -239,6 +266,11 @@ async def rbac_guard(
if path in _PUBLIC_PATHS or path.startswith(_PUBLIC_PATH_PREFIXES):
return await call_next(request)
# #3324: путь, которого нет в роутере, отвечает как любой несуществующий
# путь (404 роутера) — иначе 401 работает оракулом существования ручки.
if not _path_is_routed(request):
return await call_next(request)
username: str | None = None
role: str | None = None
from_session = False

View file

@ -248,6 +248,13 @@ def _build_test_app(store: _Store) -> FastAPI:
async def tradein_cache_stats() -> dict:
return {"ok": True}
# По той же причине ручка настоящая: с #3324 guard не отвечает 401/403 за
# несуществующий путь (несуществующее обязано быть неотличимо от
# несуществующего), поэтому admin-гейт проверяется на реальном роуте.
@app.get("/api/v1/admin/dummy")
async def admin_dummy() -> dict:
return {"ok": True}
def _override_get_db(): # generator dependency — matches app.core.db.get_db shape
yield _FakeDB(store)
@ -1306,9 +1313,9 @@ def test_session_user_can_reach_tradein_but_not_admin(client: TestClient, store:
assert ok.status_code == 200
denied = client.get("/api/v1/admin/dummy")
# rbac_guard's admin-gate matches the path regex BEFORE routing even happens
# (route isn't registered on this test app) — role=employee != admin -> 403,
# never a 404 (a bare "any non-2xx" assertion would mask a rbac_guard typo).
# Роут зарегистрирован (см. _build_test_app), поэтому 403 приходит именно от
# admin-гейта rbac_guard'а: role=employee != admin. Не 404 — иначе «any
# non-2xx» маскировал бы опечатку в guard'е; и не 200 — иначе гейт не сработал.
assert denied.status_code == 403

View file

@ -384,6 +384,37 @@ def test_rbac_guard_no_header_on_non_admin_path_returns_401(client: TestClient)
assert "no authenticated user" in resp.json()["detail"].lower()
def test_rbac_guard_unrouted_path_answers_the_same_everywhere(client: TestClient) -> None:
"""#3324: аноним не должен по ответу отличать несуществующий путь от закрытого.
Сравниваются два ОТВЕТА между собой, а не с константой: важно именно
неразличимость. До фикса мусор под «интересным» префиксом получал 401 с
rbac-текстом, а такой же мусор рядом 404 роутера, и по этой разнице
периметр разведывался бесплатно.
"""
for interesting, boring in (
# Пара, на которой оракул был виден в чистом виде: под публичным
# префиксом guard молчал (404 роутера), а рядом отвечал 401.
("/api/v1/trade-in/zzz", "/api/v1/trade-in/r/zzz"),
("/api/public/zzz", "/api/zzz"),
("/trade-in/api/public/zzz", "/trade-in/api/zzz"),
("/api/v1/admin/zzz", "/zzz"),
):
first = client.get(interesting)
second = client.get(boring)
assert (first.status_code, first.json()) == (second.status_code, second.json()), (
f"{interesting}{first.status_code} {first.json()}, "
f"{boring}{second.status_code} {second.json()}"
)
assert first.status_code == 404
# Защита не ослаблена: РЕАЛЬНЫЙ закрытый маршрут анониму по-прежнему 401.
assert client.get("/api/v1/trade-in/support/unread").status_code == 401
assert client.get("/api/v1/me").status_code == 401
# Реальный публичный маршрут по-прежнему работает без идентичности.
assert client.get("/api/v1/trade-in/support/anon/unread").status_code == 200
# ---------------------------------------------------------------------------
# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов
# ---------------------------------------------------------------------------