All checks were successful
CI Trade-In / changes (pull_request) Successful in 11s
CI / changes (pull_request) Successful in 12s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m27s
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
test_rbac.py and test_internal_auth_secret.py each kept a hand-maintained
"MIRROR of app.main" copy of rbac_guard. The copies had already drifted:
test_rbac.py's copy was missing the #2213 X-Internal-Auth-Secret
defense-in-depth check entirely, so a regression in the real guard would not
have failed CI. Extracted rbac_guard (+ its constants) into app/core/rbac.py
(no DB/lifespan side effects) so app/main.py and both test files import and
exercise the exact same production code path instead of copies.
Verified the fix actually catches regressions: temporarily neutered the
secret-gate check in app/core/rbac.py, confirmed
test_internal_auth_secret.py went red (2 failures), then reverted — suite
back to green (31/31).
Also added tests/test_pdf_real_render.py: test_pdf_security.py stubs
WeasyPrint entirely, so it structurally cannot catch a real pagination
regression like the one just fixed in commit 42a50cf8 (extra trailing blank
5th page from the running @page header/footer). The new module renders the
REAL PDF via generate_trade_in_pdf and asserts the documented "4 pages, no
blank" invariant directly. It needs WeasyPrint's native Pango/cairo/GObject
libs (absent on this Windows sandbox and on ci-tradein.yml's bare
ubuntu-latest runner), so it self-skips via
pytest.skip(allow_module_level=True) wherever those aren't present, and runs
for real only where they are (e.g. `docker exec tradein-backend python -m
pytest -m pdf_render tests/test_pdf_real_render.py`) — see its docstring.
Existing mocked security tests are untouched (they check different things).
tests/conftest.py added (didn't exist before) to register the pdf_render
marker cleanly.
136 lines
6.3 KiB
Python
136 lines
6.3 KiB
Python
"""RBAC guard middleware — extracted from ``app/main.py``.
|
||
|
||
Historically ``rbac_guard`` lived inline in ``app/main.py`` and the test suite
|
||
(``tests/test_rbac.py``, ``tests/test_internal_auth_secret.py``) kept a
|
||
hand-maintained *copy* of it, labelled "MIRROR of app.main — keep in sync
|
||
manually". The copy drifted: it was missing the #2213
|
||
``X-Internal-Auth-Secret`` defense-in-depth check that the real guard has,
|
||
so a regression in that check would NOT have failed CI.
|
||
|
||
This module holds the real guard with no DB/lifespan/scheduler side effects
|
||
(only ``app.core.auth`` + ``app.core.config``, both side-effect-free at
|
||
import time beyond requiring ``DATABASE_URL`` in the environment for
|
||
``Settings()``). ``app/main.py`` and the test apps both import THIS module,
|
||
so tests exercise the exact production code path instead of a copy that can
|
||
silently fall out of sync.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import logging
|
||
import re
|
||
import secrets
|
||
from collections.abc import Awaitable, Callable
|
||
|
||
from fastapi import Request
|
||
from fastapi.responses import JSONResponse, Response
|
||
|
||
from app.core.auth import get_role, is_path_allowed
|
||
from app.core.config import settings
|
||
|
||
logger = logging.getLogger(__name__)
|
||
|
||
# RBAC: defense-in-depth поверх Caddy basic_auth + X-Authenticated-User
|
||
# (см. app/core/auth.py + auth/roles.yaml). Правила:
|
||
# 1) Любой non-public path требует X-Authenticated-User — иначе 401.
|
||
# 2) Юзер должен быть в roles.yaml — иначе 403 («неизвестный юзер ничего
|
||
# не видит» — decided 2026-05-25).
|
||
# 3) /api/v1/admin/* (= внешний /trade-in/api/v1/admin/* после Caddy
|
||
# `uri strip_prefix /trade-in`) — только role=admin, иначе 403.
|
||
# Public paths без auth (/health, /docs, /openapi.json) пропускаем —
|
||
# X-Authenticated-User там не приходит из Caddy.
|
||
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||
# #R2-H3: Caddy срезает внешний префикс /trade-in (uri strip_prefix) перед
|
||
# tradein-backend, а globs в roles.yaml — ВНЕШНИЕ (/trade-in/api/v1/**). Для
|
||
# scope-проверки восстанавливаем внешний путь.
|
||
_EXTERNAL_PREFIX = "/trade-in"
|
||
# Bootstrap-пути, доступные ЛЮБОМУ известному юзеру независимо от роли: /me отдаёт
|
||
# роль (expired → trial-экран), /brand/* — брендинг login/trial-экрана. Без них
|
||
# expired (roles.yaml paths:[] deny:/**) не получил бы роль и не увидел trial-экран.
|
||
_RBAC_BOOTSTRAP_EXEMPT = ("/api/v1/me", "/api/v1/brand")
|
||
|
||
|
||
async def rbac_guard(
|
||
request: Request,
|
||
call_next: Callable[[Request], Awaitable[Response]],
|
||
) -> Response:
|
||
path = request.url.path
|
||
if path in _PUBLIC_PATHS:
|
||
return await call_next(request)
|
||
|
||
username = request.headers.get("X-Authenticated-User")
|
||
if not username:
|
||
return JSONResponse(
|
||
status_code=401,
|
||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
||
)
|
||
|
||
# #2213 defense-in-depth: если общий секрет задан — запрос с X-Authenticated-User
|
||
# ОБЯЗАН нести валидный X-Internal-Auth-Secret (его добавляет Caddy из env).
|
||
# Иначе это подделка заголовка мимо Caddy (напр. изнутри gendesign_shared) → 401.
|
||
# Constant-time compare против timing-атак. Пусто = защита не активна (fail-open).
|
||
secret = settings.tradein_internal_auth_secret
|
||
if secret:
|
||
provided = request.headers.get("X-Internal-Auth-Secret", "")
|
||
if not secrets.compare_digest(provided, secret):
|
||
logger.warning(
|
||
"RBAC: X-Authenticated-User=%r без валидного X-Internal-Auth-Secret "
|
||
"на %s — возможная подделка заголовка мимо Caddy",
|
||
username,
|
||
path,
|
||
)
|
||
return JSONResponse(
|
||
status_code=401,
|
||
content={"detail": "invalid or missing internal auth secret"},
|
||
)
|
||
|
||
try:
|
||
role = get_role(username)
|
||
except KeyError:
|
||
logger.warning("RBAC: unknown user %r tried %s", username, path)
|
||
return JSONResponse(
|
||
status_code=403,
|
||
content={"detail": "user not in roles config"},
|
||
)
|
||
|
||
if _ADMIN_API_RE.match(path) and role != "admin":
|
||
logger.info("RBAC: blocked %s (role=%s) from %s", username, role, path)
|
||
return JSONResponse(
|
||
status_code=403,
|
||
content={"detail": "admin only"},
|
||
)
|
||
|
||
# #R2-H3: энфорсим roles.yaml scope (paths/deny) для ВСЕХ non-admin путей, а не
|
||
# только /admin/*. Иначе revoked (role=expired, paths:[] deny:/**) или узко-
|
||
# скоупленный аккаунт достаёт non-admin API (напр. POST /api/v1/search —
|
||
# экспорт листингов), который roles.yaml ему запрещает. Bootstrap-пути (/me,
|
||
# /brand) исключены выше по списку. roles.yaml globs внешние → восстанавливаем
|
||
# внешний путь (Caddy срезал /trade-in). На сбой парса — fail-open + громкий
|
||
# лог: не лочим платящего pilot из-за конфиг-бага (admin-гейт выше остаётся).
|
||
if not path.startswith(_RBAC_BOOTSTRAP_EXEMPT):
|
||
external_path = _EXTERNAL_PREFIX + path
|
||
try:
|
||
allowed = is_path_allowed(role, external_path)
|
||
except Exception:
|
||
logger.exception(
|
||
"RBAC scope-check raised for %s %s (ext=%s) — fail-open",
|
||
username,
|
||
path,
|
||
external_path,
|
||
)
|
||
allowed = True
|
||
if not allowed:
|
||
logger.info(
|
||
"RBAC: scope-blocked %s (role=%s) from %s (ext=%s)",
|
||
username,
|
||
role,
|
||
path,
|
||
external_path,
|
||
)
|
||
return JSONResponse(
|
||
status_code=403,
|
||
content={"detail": "forbidden for role"},
|
||
)
|
||
|
||
return await call_next(request)
|