diff --git a/scripts/smoke-mera-perimeter.sh b/scripts/smoke-mera-perimeter.sh index 47994555..952bc695 100644 --- a/scripts/smoke-mera-perimeter.sh +++ b/scripts/smoke-mera-perimeter.sh @@ -214,7 +214,17 @@ check "trade-in /api/v1/me — 401 anonymous" "$BASE_MAIN/trade-in/api/v1/me" 40 # зелёной только потому, что guard отвечал 401 на ЛЮБОЙ путь; после #3352 # несуществующий путь даёт 404, и проверка честно покраснела. check "trade-in /api/v1/trade-in/history — 401 anonymous (чужие оценки)" "$BASE_MAIN/trade-in/api/v1/trade-in/history" 401 -check "trade-in /api/v1/admin/* — 401 anonymous" "$BASE_MAIN/trade-in/api/v1/admin/users" 401 +# #3360: admin-префикс анониму отвечает 404, а НЕ 401. Периметр здесь не срезан +# (Caddy-блок /trade-in/api/* стоит выше basic_auth-снипета, и срезать нельзя — +# admin-UI зовёт эти же пути из браузера), поэтому существование ручки прячет сам +# guard. Пара путей взята намеренно разная: `/proxies` — РЕАЛЬНЫЙ роут +# (app/api/v1/admin.py), `/users` — несуществующий. Одинаковый код на обоих и +# означает, что перебором имён admin-API снаружи ничего не узнать; 401 на первом +# = регресс маскировки (app/core/rbac.py::_unauthenticated). +check "trade-in /api/v1/admin/proxies — 404 anonymous (существующая ручка скрыта)" \ + "$BASE_MAIN/trade-in/api/v1/admin/proxies" 404 +check "trade-in /api/v1/admin/users — 404 anonymous (несуществующая — тот же ответ)" \ + "$BASE_MAIN/trade-in/api/v1/admin/users" 404 # 4. gendsgn.ru/api/v1/admin/* отдаёт 401 анониму (auth gate стоит ДО роутинга # в FastAPI — конкретный путь неважен, любой /api/v1/admin/* перехватывается diff --git a/tradein-mvp/backend/app/core/rbac.py b/tradein-mvp/backend/app/core/rbac.py index f760ec54..22551be0 100644 --- a/tradein-mvp/backend/app/core/rbac.py +++ b/tradein-mvp/backend/app/core/rbac.py @@ -238,6 +238,34 @@ def _path_is_routed(request: Request) -> bool: return any(route.matches(request.scope)[0] != Match.NONE for route in router.routes) +def _unauthenticated(path: str, detail: str) -> JSONResponse: + """Отказ анониму: 401 везде, но на admin-префиксе — 404 роутера. + + #3360: периметр admin-путей снаружи НЕ срезан (``caddy/sites/apps.caddy``, + блок ``handle /trade-in/api/*`` стоит выше ``import + caddy/users.caddy.snippet``), и срезать его нельзя — admin-UI кабинета зовёт + ``/api/v1/admin/*`` ИЗ БРАУЗЕРА (tradein-mvp/frontend/src/app/scrapers/**, + components/scrapers/**, lib/admin-audit-api.ts). Значит внешний аноним + доходит сюда, а после #3324 (несуществующий путь → 404 роутера) 401 на + существующей ручке работал оракулом: перебором имён восстанавливался список + admin-API. Отвечаем ТЕМ ЖЕ, что роутер отдаёт на несуществующий путь, — + существующая и несуществующая admin-ручки анониму неразличимы. + + Скрываем ровно от НЕаутентифицированного. Аутентифицированный не-admin + по-прежнему получает 403 «admin only»: он уже прошёл идентификацию, прятать + от него наличие ручки незачем, а 404 вместо 403 маскировал бы отладку. + + Тело — литерал ``{"detail": "Not Found"}``: это ответ дефолтного + http_exception_handler FastAPI, тот же, что придёт из роутера. Тест + сравнивает два ЖИВЫХ ответа между собой, а не с этой константой, — если + фреймворк сменит формулировку, покраснеет он, а не прод. + """ + if _ADMIN_API_RE.match(path): + logger.info("RBAC: anonymous probe of admin path %s — cloaked as 404", path) + return JSONResponse(status_code=404, content={"detail": "Not Found"}) + return JSONResponse(status_code=401, content={"detail": detail}) + + def _propagate_authenticated_user(request: Request, username: str) -> None: """Инжектит ``X-Authenticated-User`` в ASGI scope — ПЕРЕЗАПИСЫВАЯ, а не только добавляя при отсутствии, — чтобы ``RateLimitMiddleware``/ @@ -340,18 +368,12 @@ async def rbac_guard( # auth_mode == "db_only" — легаси trusted-header путь ПОЛНОСТЬЮ # отключён, даже если валидный X-Authenticated-User присутствует. if settings.auth_mode != "dual": - return JSONResponse( - status_code=401, - content={"detail": "valid session required"}, - ) + return _unauthenticated(path, "valid session required") # ---- legacy trusted-header path — BIT-FOR-BIT как было до #2552 ---- username = request.headers.get("X-Authenticated-User") if not username: - return JSONResponse( - status_code=401, - content={"detail": "no authenticated user (valid session required)"}, - ) + return _unauthenticated(path, "no authenticated user (valid session required)") # #2213 defense-in-depth: если общий секрет задан — запрос с X-Authenticated-User # ОБЯЗАН нести валидный X-Internal-Auth-Secret (его добавляет Caddy из env). @@ -367,10 +389,7 @@ async def rbac_guard( username, path, ) - return JSONResponse( - status_code=401, - content={"detail": "invalid or missing internal auth secret"}, - ) + return _unauthenticated(path, "invalid or missing internal auth secret") try: role = get_role(username) diff --git a/tradein-mvp/backend/tests/test_rbac.py b/tradein-mvp/backend/tests/test_rbac.py index 27e16daa..71335416 100644 --- a/tradein-mvp/backend/tests/test_rbac.py +++ b/tradein-mvp/backend/tests/test_rbac.py @@ -34,6 +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 import config from app.core.rbac import _db_role_path_allowed, rbac_guard @@ -270,9 +271,14 @@ def test_rbac_guard_admin_allowed(client: TestClient) -> None: assert resp.json() == {"ok": True} -def test_rbac_guard_no_header_on_admin_path_returns_401(client: TestClient) -> None: +def test_rbac_guard_no_header_on_admin_path_returns_404(client: TestClient) -> None: + """#3360: было 401 («ручка есть, но не для тебя»), стало 404 — как у + несуществующего пути. Доступа это не меняет (запрос по-прежнему отбит ДО + хендлера), меняется только то, что аноним узнаёт из ответа. Неразличимость + с несуществующей ручкой проверяется ниже, + test_anon_cannot_tell_existing_admin_route_from_missing_one.""" resp = client.get("/api/v1/admin/dummy") - assert resp.status_code == 401 + assert resp.status_code == 404 def test_rbac_guard_unknown_user_on_admin_path_returns_403(client: TestClient) -> None: @@ -419,6 +425,79 @@ def test_rbac_guard_unrouted_path_answers_the_same_everywhere(client: TestClient assert client.get("/api/v1/trade-in/support/anon/unread").status_code == 200 +def test_anon_cannot_tell_existing_admin_route_from_missing_one(client: TestClient) -> None: + """#3360: анониму СУЩЕСТВУЮЩАЯ admin-ручка отвечает ровно как несуществующая. + + Периметр admin-путей снаружи не срезан (`caddy/sites/apps.caddy`: блок + `handle /trade-in/api/*` выше `import caddy/users.caddy.snippet`) и срезать + его нельзя — admin-UI кабинета зовёт `/api/v1/admin/*` ИЗ БРАУЗЕРА + (tradein-mvp/frontend/src/app/scrapers/**, lib/admin-audit-api.ts). Значит + внешний аноним доходит до guard'а, а после #3324 (несуществующий путь → 404 + роутера) 401 на существующей ручке работал оракулом имён admin-API. + + Сравниваются два ЖИВЫХ ответа между собой, а не с константой: важна именно + неразличимость, а не конкретная формулировка тела от FastAPI. + """ + existing = client.get("/api/v1/admin/dummy") # роут есть — см. _build_test_app + missing = client.get("/api/v1/admin/zzz-no-such-handle") + assert (existing.status_code, existing.json()) == (missing.status_code, missing.json()), ( + f"существующая → {existing.status_code} {existing.json()}, " + f"несуществующая → {missing.status_code} {missing.json()}" + ) + assert existing.status_code == 404 + + # Маскировка НЕ трогает аутентифицированных: admin работает как раньше, + # а не-admin получает свой 403 «admin only» — прятать наличие ручки от + # опознанного человека незачем (и это ломало бы отладку прав). + ok = client.get("/api/v1/admin/dummy", headers={"X-Authenticated-User": "admintest"}) + assert ok.status_code == 200, ok.text + denied = client.get("/api/v1/admin/dummy", headers={"X-Authenticated-User": "analysttest"}) + assert denied.status_code == 403, denied.text + assert denied.json()["detail"] == "admin only" + + # non-admin путь анониму — по-прежнему 401 (#3324 оставил его как есть). + assert client.get("/api/v1/me").status_code == 401 + + +def test_anon_admin_cloak_covers_all_unauthenticated_branches( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """Все три ветки «личность не установлена» на admin-префиксе дают тот же 404. + + Одной ветки мало: guard отказывает анониму в трёх местах (нет заголовка, + auth_mode=db_only, подделанный X-Authenticated-User без #2213-секрета), и + любая пропущенная снова становится оракулом существования ручки. + """ + baseline = client.get("/api/v1/admin/zzz-no-such-handle") + + monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "s3cr3t-value") + forged = client.get("/api/v1/admin/dummy", headers={"X-Authenticated-User": "admintest"}) + assert (forged.status_code, forged.json()) == (baseline.status_code, baseline.json()), ( + f"подделка мимо Caddy → {forged.status_code} {forged.json()}" + ) + # Тот же запрос на non-admin пути секрет-гейт по-прежнему отбивает 401 — + # маскировка живёт ровно на admin-префиксе, а не поверх всего guard'а. + non_admin = client.get("/api/v1/me", headers={"X-Authenticated-User": "admintest"}) + assert non_admin.status_code == 401 + assert "internal auth secret" in non_admin.json()["detail"].lower() + + monkeypatch.setattr(config.settings, "auth_mode", "db_only") + db_only = client.get("/api/v1/admin/dummy") + assert (db_only.status_code, db_only.json()) == (baseline.status_code, baseline.json()), ( + f"db_only без сессии → {db_only.status_code} {db_only.json()}" + ) + # Различимость ветки: без заголовка 401 на non-admin пути даёт и db_only, и + # dual — отличается только текст. Сравнение РАВЕНСТВОМ, а не `in`: + # db_only-строка является подстрокой dual-строки ("no authenticated user + # (valid session required)"), и `in` прошёл бы, даже если monkeypatch + # режима тихо не подействовал, т.е. ветка была бы нефальсифицируемой. + me = client.get("/api/v1/me") + assert me.status_code == 401 + assert me.json()["detail"] == "valid session required", ( + f"ожидалась db_only-ветка, получено {me.json()['detail']!r}" + ) + + def test_real_app_does_not_redirect_trailing_slash() -> None: """#3324: `/api/v1/me/` анониму → 404, а не 307 на существующий путь.