Compare commits
No commits in common. "bab4b4f8ff7de2fcf8f7098323fc6e24ed9980ba" and "25d3db2ae7e10b60975db6d3d073e9f285556d1e" have entirely different histories.
bab4b4f8ff
...
25d3db2ae7
4 changed files with 3 additions and 140 deletions
|
|
@ -35,7 +35,6 @@ from typing import Any
|
||||||
|
|
||||||
from fastapi import Request
|
from fastapi import Request
|
||||||
from fastapi.responses import JSONResponse, Response
|
from fastapi.responses import JSONResponse, Response
|
||||||
from starlette.routing import Match
|
|
||||||
|
|
||||||
from app.core.auth import get_role, is_path_allowed
|
from app.core.auth import get_role, is_path_allowed
|
||||||
from app.core.config import settings
|
from app.core.config import settings
|
||||||
|
|
@ -182,62 +181,6 @@ def _db_role_path_allowed(role: str, path: str) -> bool:
|
||||||
return any(_db_glob_match(p, path) for p in paths)
|
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 отдаёт роутер, тот
|
|
||||||
же самый, что и на любой другой мусор.
|
|
||||||
|
|
||||||
ЧТО ИМЕННО РАЗМЕНЯНО (это НЕ «ослабления нет»). Раньше аноним получал 401 на
|
|
||||||
ЛЮБОЙ non-public путь — то есть оракул был ПРЕФИКСНЫЙ: он говорил «префикс
|
|
||||||
закрыт», но перечислить по нему таблицу маршрутов было нельзя. Теперь 401 =
|
|
||||||
«такой маршрут есть», 404 = «нет», и это уже оракул СУЩЕСТВОВАНИЯ маршрута:
|
|
||||||
перебором аноним восстанавливает список всех ручек приложения, включая имена
|
|
||||||
под ``/api/v1/admin/*``. Доступа это не даёт (закрытая ручка по-прежнему
|
|
||||||
отвечает 401/403), но карту периметра — даёт.
|
|
||||||
|
|
||||||
Почему размен принят. Точечно: 404 на несуществующее — норма HTTP, а
|
|
||||||
подобранное ИМЯ ручки без креденшелов бесполезно; исчезает же реальный
|
|
||||||
признак «этот префикс что-то охраняет». Это защита в глубину, и её глубина
|
|
||||||
здесь честно меньше, чем была.
|
|
||||||
|
|
||||||
⚠️ Периметр admin-путей снаружи НЕ срезан — проверено по конфигу, а не по
|
|
||||||
предположению: ``caddy/sites/apps.caddy`` → блок ``handle /trade-in/api/*``
|
|
||||||
делает ``uri strip_prefix /trade-in`` + ``reverse_proxy tradein-backend:8000``
|
|
||||||
и стоит ЦЕЛИКОМ ВЫШЕ ``import caddy/users.caddy.snippet`` (basic_auth), т.е.
|
|
||||||
у trade-in своя авторизация и внешнего barrier'а нет. Значит внешний
|
|
||||||
``https://gendsgn.ru/trade-in/api/v1/admin/...`` доходит до этого guard'а
|
|
||||||
анонимно, и перебор имён admin-ручек выполним снаружи, не только изнутри
|
|
||||||
docker-сети. Хочется убрать — резать надо в Caddy (отдельный issue), guard
|
|
||||||
этого не сделает: он про роли, а не про сетевой периметр.
|
|
||||||
|
|
||||||
``Match.NONE`` по ВСЕМ маршрутам значит, что выполнять нечего — хендлера, до
|
|
||||||
которого можно было бы дотянуться, не существует. ``Match.PARTIAL`` (путь
|
|
||||||
есть, метод другой) считается маршрутом и идёт в guard как раньше — там путь
|
|
||||||
реально существует, скрывать нечего.
|
|
||||||
|
|
||||||
Трейлинг-слэш был вторым каналом того же оракула в обход guard'а:
|
|
||||||
``/api/v1/me/`` не матчит ни один маршрут (``Match.NONE``) → guard пропускает
|
|
||||||
→ Starlette-роутер отвечал 307 на ``/api/v1/me``, то есть «маршрут есть»
|
|
||||||
сообщал редирект, а не 401. Закрыто в ``app/main.py``:
|
|
||||||
``FastAPI(redirect_slashes=False)`` — теперь такой путь даёт тот же 404, что
|
|
||||||
и любой другой мусор.
|
|
||||||
|
|
||||||
Неизвестное приложение (``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:
|
def _propagate_authenticated_user(request: Request, username: str) -> None:
|
||||||
"""Инжектит ``X-Authenticated-User`` в ASGI scope — ПЕРЕЗАПИСЫВАЯ, а не
|
"""Инжектит ``X-Authenticated-User`` в ASGI scope — ПЕРЕЗАПИСЫВАЯ, а не
|
||||||
только добавляя при отсутствии, — чтобы ``RateLimitMiddleware``/
|
только добавляя при отсутствии, — чтобы ``RateLimitMiddleware``/
|
||||||
|
|
@ -296,11 +239,6 @@ async def rbac_guard(
|
||||||
if path in _PUBLIC_PATHS or path.startswith(_PUBLIC_PATH_PREFIXES):
|
if path in _PUBLIC_PATHS or path.startswith(_PUBLIC_PATH_PREFIXES):
|
||||||
return await call_next(request)
|
return await call_next(request)
|
||||||
|
|
||||||
# #3324: путь, которого нет в роутере, отвечает как любой несуществующий
|
|
||||||
# путь (404 роутера) — иначе 401 работает оракулом существования ручки.
|
|
||||||
if not _path_is_routed(request):
|
|
||||||
return await call_next(request)
|
|
||||||
|
|
||||||
username: str | None = None
|
username: str | None = None
|
||||||
role: str | None = None
|
role: str | None = None
|
||||||
from_session = False
|
from_session = False
|
||||||
|
|
|
||||||
|
|
@ -224,16 +224,6 @@ app = FastAPI(
|
||||||
description="Оценка вторичного жилья (выкупная стоимость) — копия trade-in feature из gendesign", # noqa: E501
|
description="Оценка вторичного жилья (выкупная стоимость) — копия trade-in feature из gendesign", # noqa: E501
|
||||||
version="0.1.0",
|
version="0.1.0",
|
||||||
lifespan=lifespan,
|
lifespan=lifespan,
|
||||||
# #3324: трейлинг-слэш обходил rbac_guard как канал разведки периметра.
|
|
||||||
# `/api/v1/me/` не матчит ни один маршрут → guard пропускает (см.
|
|
||||||
# rbac._path_is_routed) → роутер отвечал 307 на `/api/v1/me`, т.е. «маршрут
|
|
||||||
# существует» сообщал редирект вместо 401. Выключение проверено на предмет
|
|
||||||
# поломок: ни один route не объявлен с трейлинг-слэшем (нет `@router.get("/")`
|
|
||||||
# и пустых путей), ни один из 173 вызовов `api/v1` во фронте
|
|
||||||
# (tradein-mvp/frontend/src) не заканчивается слэшем, mount/StaticFiles нет.
|
|
||||||
# Deny-правила на слэш тоже не зависят от редиректа — они переведены на
|
|
||||||
# глоб-форму специально ради этого (см. auth_session.DB_ROLE_PATHS).
|
|
||||||
redirect_slashes=False,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -249,13 +249,6 @@ def _build_test_app(store: _Store) -> FastAPI:
|
||||||
async def tradein_cache_stats() -> dict:
|
async def tradein_cache_stats() -> dict:
|
||||||
return {"ok": True}
|
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
|
def _override_get_db(): # generator dependency — matches app.core.db.get_db shape
|
||||||
yield _FakeDB(store)
|
yield _FakeDB(store)
|
||||||
|
|
||||||
|
|
@ -1366,14 +1359,10 @@ def test_session_user_can_reach_tradein_but_not_admin(client: TestClient, store:
|
||||||
assert ok.status_code == 200
|
assert ok.status_code == 200
|
||||||
|
|
||||||
denied = client.get("/api/v1/admin/dummy")
|
denied = client.get("/api/v1/admin/dummy")
|
||||||
# Роут зарегистрирован (см. _build_test_app), поэтому 403 приходит именно от
|
# rbac_guard's admin-gate matches the path regex BEFORE routing even happens
|
||||||
# admin-гейта rbac_guard'а: role=employee != admin. Не 404 — иначе «any
|
# (route isn't registered on this test app) — role=employee != admin -> 403,
|
||||||
# non-2xx» маскировал бы опечатку в guard'е; и не 200 — иначе гейт не сработал.
|
# never a 404 (a bare "any non-2xx" assertion would mask a rbac_guard typo).
|
||||||
# Один статус этого не доказывает: scope-check (deny-список employee) отвечает
|
|
||||||
# тем же 403, поэтому сверяем ТЕКСТ — 'admin only' пишет только admin-гейт,
|
|
||||||
# scope-ветка пишет 'forbidden for role' (app/core/rbac.py).
|
|
||||||
assert denied.status_code == 403
|
assert denied.status_code == 403
|
||||||
assert denied.json()["detail"] == "admin only", denied.text
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
|
|
@ -384,60 +384,6 @@ def test_rbac_guard_no_header_on_non_admin_path_returns_401(client: TestClient)
|
||||||
assert "no authenticated user" in resp.json()["detail"].lower()
|
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
|
|
||||||
# Match.PARTIAL — путь есть, метод чужой (/api/v1/me зарегистрирован как GET).
|
|
||||||
# Такой запрос обязан идти в guard (401), а НЕ проваливаться в роутер за 405:
|
|
||||||
# иначе «метод не тот» стало бы ещё одним способом обойти проверку личности.
|
|
||||||
assert client.post("/api/v1/me").status_code == 401
|
|
||||||
# Реальный публичный маршрут по-прежнему работает без идентичности.
|
|
||||||
assert client.get("/api/v1/trade-in/support/anon/unread").status_code == 200
|
|
||||||
|
|
||||||
|
|
||||||
def test_real_app_does_not_redirect_trailing_slash() -> None:
|
|
||||||
"""#3324: `/api/v1/me/` анониму → 404, а не 307 на существующий путь.
|
|
||||||
|
|
||||||
Трейлинг-слэш был вторым каналом оракула существования: путь со слэшем не
|
|
||||||
матчит ни один маршрут (Match.NONE) → guard пропускает → Starlette отвечал
|
|
||||||
307 на `/api/v1/me`, т.е. сообщал «маршрут есть» в обход 401.
|
|
||||||
|
|
||||||
Проверяется РЕАЛЬНОЕ приложение (app.main), а не локальный _build_test_app:
|
|
||||||
флаг `redirect_slashes=False` живёт именно там, и тест на своей копии app
|
|
||||||
был бы тавтологией — он подтверждал бы настройку фикстуры, а не прода.
|
|
||||||
Импорт локальный: модуль этого файла намеренно не тянет app.main (см. шапку).
|
|
||||||
"""
|
|
||||||
from app.main import app as real_app
|
|
||||||
|
|
||||||
# TestClient без `with` не запускает lifespan — в БД никто не ходит.
|
|
||||||
resp = TestClient(real_app).get("/api/v1/me/", follow_redirects=False)
|
|
||||||
assert resp.status_code == 404, f"{resp.status_code} {resp.headers.get('location')}"
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов
|
# 2026-07-31: внутренние разделы («Доля в продаже» / «Кэш») закрыты от клиентов
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue