fix(tradein/security): defense-in-depth trusted-header auth + rate-limiter fix (#2213)
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / changes (pull_request) Successful in 7s
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m43s
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / changes (pull_request) Successful in 7s
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m43s
- shared-secret Caddy↔tradein-backend: X-Internal-Auth-Secret (secrets.compare_digest, constant-time), fail-open до провижининга TRADEIN_INTERNAL_AUTH_SECRET (WARNING на старте); при заданном секрете подделка X-Authenticated-User изнутри docker-сети даёт 401 - rate-limiter: per-user/per-IP ключ вместо освобождения всех authenticated; client IP из rightmost XFF (один доверенный прокси Caddy, который аппендит real IP); leftmost XFF клиент-контролируем и игнорируется - Caddyfile: убран глобальный log_credentials (писал обратимые base64 basic-auth пароли в логи; glitchtip-forwarder деградирует gracefully) - тесты: secret-gate (401/200/backward-compat/wrong/no-user) + per-user rate-limit + XFF-spoof нейтрализован Провижининг (human, порядок критичен): значение секрета СНАЧАЛА в окружение Caddy-стека (+recreate caddy), ПОТОМ в рантайм-окружение tradein-backend (+restart). Обратный порядок = fail-closed 401 на всё приложение. Значение секрета в репо не попадает. Refs #2213
This commit is contained in:
parent
4ce0f51bb8
commit
07450aeed2
8 changed files with 347 additions and 41 deletions
24
Caddyfile
24
Caddyfile
|
|
@ -16,15 +16,13 @@
|
||||||
# (basic_auth runs before handle), making /health and /preview/* exclusions
|
# (basic_auth runs before handle), making /health and /preview/* exclusions
|
||||||
# ineffective. With route { }, handlers are matched top-to-bottom as written.
|
# ineffective. With route { }, handlers are matched top-to-bottom as written.
|
||||||
|
|
||||||
{
|
# #2213: global `log_credentials` УБРАН. Он заставлял Caddy писать raw
|
||||||
servers {
|
# Authorization header (Base64 `user:password`, тривиально обратимый) в ОБА лог-файла
|
||||||
# ВКЛЮЧАЕТ raw Authorization/Cookie headers в access log
|
# (gendsgn.ru.log + auth_audit.log). Даже при root-only доступе и коротком retention
|
||||||
# (по default Caddy 2.x редактирует их как "REDACTED").
|
# это plaintext-пароли пилотов на диске — недопустимый риск против скромной выгоды.
|
||||||
# Plain password будет в gendsgn.ru.log + auth_audit.log — оба root-only на VPS.
|
# Trade-off: glitchtip-auth-forwarder больше не извлекает `attempted_username` из 401
|
||||||
# См. forwarder.py _extract_attempted_username для использования.
|
# (тег деградирует в "(none)" — forwarder это уже обрабатывает gracefully, не падает).
|
||||||
log_credentials
|
# Событие basic_auth 401 (remote_ip / uri / method) по-прежнему уходит в GlitchTip.
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
gendsgn.ru {
|
gendsgn.ru {
|
||||||
encode zstd gzip
|
encode zstd gzip
|
||||||
|
|
@ -88,6 +86,11 @@ gendsgn.ru {
|
||||||
uri strip_prefix /trade-in
|
uri strip_prefix /trade-in
|
||||||
reverse_proxy tradein-backend:8000 {
|
reverse_proxy tradein-backend:8000 {
|
||||||
header_up X-Authenticated-User {http.auth.user.id}
|
header_up X-Authenticated-User {http.auth.user.id}
|
||||||
|
# #2213 defense-in-depth: общий секрет Caddy↔tradein-backend. header_up
|
||||||
|
# с value ПЕРЕЗАПИСЫВАЕТ (стирает) любой клиентский X-Internal-Auth-Secret —
|
||||||
|
# тот же механизм, что защищает X-Authenticated-User выше. Пусто пока
|
||||||
|
# TRADEIN_INTERNAL_AUTH_SECRET не задан в .env (fail-open, backend не проверяет).
|
||||||
|
header_up X-Internal-Auth-Secret {env.TRADEIN_INTERNAL_AUTH_SECRET}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -108,6 +111,9 @@ gendsgn.ru {
|
||||||
# Next.js basePath=/trade-in — фронт сам ждёт префикса в URL
|
# Next.js basePath=/trade-in — фронт сам ждёт префикса в URL
|
||||||
reverse_proxy tradein-frontend:3000 {
|
reverse_proxy tradein-frontend:3000 {
|
||||||
header_up X-Authenticated-User {http.auth.user.id}
|
header_up X-Authenticated-User {http.auth.user.id}
|
||||||
|
# #2213: симметрично с /trade-in/api/* — перезаписываем секрет из env
|
||||||
|
# (стирает клиентский), на случай SSR-forwardʼa фронтом в backend.
|
||||||
|
header_up X-Internal-Auth-Secret {env.TRADEIN_INTERNAL_AUTH_SECRET}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -270,6 +270,12 @@ services:
|
||||||
ports:
|
ports:
|
||||||
- "80:80"
|
- "80:80"
|
||||||
- "443:443"
|
- "443:443"
|
||||||
|
environment:
|
||||||
|
# #2213 defense-in-depth: общий секрет Caddy↔tradein-backend. Caddyfile
|
||||||
|
# подставляет его в header_up X-Internal-Auth-Secret на /trade-in/* роутах.
|
||||||
|
# Пустой дефолт = fail-open (backend не проверяет) → ничего не ломается до
|
||||||
|
# провижининга. Значение задаётся руками в /opt/gendesign/.env на VPS.
|
||||||
|
TRADEIN_INTERNAL_AUTH_SECRET: ${TRADEIN_INTERNAL_AUTH_SECRET:-}
|
||||||
volumes:
|
volumes:
|
||||||
- ./Caddyfile:/etc/caddy/Caddyfile:ro
|
- ./Caddyfile:/etc/caddy/Caddyfile:ro
|
||||||
- ./caddy/users.caddy.snippet:/etc/caddy/caddy/users.caddy.snippet:ro
|
- ./caddy/users.caddy.snippet:/etc/caddy/caddy/users.caddy.snippet:ro
|
||||||
|
|
|
||||||
|
|
@ -31,6 +31,23 @@ class Settings(BaseSettings):
|
||||||
cors_origins: list[str] = ["http://localhost", "http://localhost:3000", "http://localhost:8080"]
|
cors_origins: list[str] = ["http://localhost", "http://localhost:3000", "http://localhost:8080"]
|
||||||
environment: str = "dev"
|
environment: str = "dev"
|
||||||
|
|
||||||
|
# ── #2213: defense-in-depth для trusted-header auth ───────────────────────
|
||||||
|
# Backend доверяет заголовку X-Authenticated-User (его проставляет Caddy после
|
||||||
|
# basic_auth). Но backend сидит на общей docker-сети gendesign_shared вместе с
|
||||||
|
# обоими стеками — любой контейнер мог бы отправить поддельный
|
||||||
|
# `X-Authenticated-User: admin` напрямую на tradein-backend:8000, минуя Caddy,
|
||||||
|
# и получить админ-доступ. Общий секрет закрывает эту дыру: Caddy добавляет
|
||||||
|
# X-Internal-Auth-Secret из env, backend требует точного совпадения на любом
|
||||||
|
# запросе с X-Authenticated-User (constant-time compare).
|
||||||
|
#
|
||||||
|
# Дизайн — fail-open до провижининга: пусто (дефолт) → защита НЕ активна
|
||||||
|
# (поведение как раньше + один WARNING на старте). Секрет задаётся руками в
|
||||||
|
# .env.runtime ОБОИХ стедов (Caddy главного стека + tradein-backend) и НИКОГДА
|
||||||
|
# не коммитится. ENV: TRADEIN_INTERNAL_AUTH_SECRET.
|
||||||
|
tradein_internal_auth_secret: str = Field(
|
||||||
|
default="", validation_alias="TRADEIN_INTERNAL_AUTH_SECRET"
|
||||||
|
)
|
||||||
|
|
||||||
# Geocoder. Env var name `YANDEX_GEOCODER_API_KEY` — consistent с scripts/
|
# Geocoder. Env var name `YANDEX_GEOCODER_API_KEY` — consistent с scripts/
|
||||||
# backfill_house_coords.py + audit_address_mismatch.py + main backend
|
# backfill_house_coords.py + audit_address_mismatch.py + main backend
|
||||||
# OpenRouteService_API_KEY pattern. Renamed from YANDEX_GEOCODER_KEY (PR F).
|
# OpenRouteService_API_KEY pattern. Renamed from YANDEX_GEOCODER_KEY (PR F).
|
||||||
|
|
@ -53,12 +70,20 @@ class Settings(BaseSettings):
|
||||||
# Redis URL для hot-cache (Phase 3.2). Задаётся через env REDIS_URL.
|
# Redis URL для hot-cache (Phase 3.2). Задаётся через env REDIS_URL.
|
||||||
redis_url: str = "redis://localhost:6379/0"
|
redis_url: str = "redis://localhost:6379/0"
|
||||||
|
|
||||||
# Rate-limit публичного /api/* (per-IP sliding window). ENV: RATE_LIMIT,
|
# Rate-limit публичного /api/* (per-user / per-IP sliding window). ENV:
|
||||||
# RATE_LIMIT_WINDOW_S. Не более rate_limit запросов за rate_limit_window_s
|
# RATE_LIMIT, RATE_LIMIT_WINDOW_S. Не более rate_limit запросов за
|
||||||
# секунд с одного IP. Аутентифицированный трафик (X-Authenticated-User от
|
# rate_limit_window_s секунд на КЛЮЧ (ключ = authenticated username, либо
|
||||||
# Caddy basic_auth) не лимитируется — см. ratelimit.py (#655).
|
# client IP для анонимов). См. ratelimit.py (#655, #2213).
|
||||||
rate_limit: int = 300
|
rate_limit: int = 300
|
||||||
rate_limit_window_s: float = 60.0
|
rate_limit_window_s: float = 60.0
|
||||||
|
# #2213: аутентифицированный трафик БОЛЬШЕ не освобождается целиком (это было
|
||||||
|
# плацебо — заголовок X-Authenticated-User клиент-контролируем). Вместо этого
|
||||||
|
# per-user лимит = rate_limit × этот множитель. Живой пилот (kopylov/admin) не
|
||||||
|
# должен упираться — множитель щедрый (×5 = 1500/60с при дефолте), но подделка
|
||||||
|
# заголовка уже не даёт безлимит. ENV: RATE_LIMIT_AUTHENTICATED_MULTIPLIER.
|
||||||
|
rate_limit_authenticated_multiplier: int = Field(
|
||||||
|
default=5, validation_alias="RATE_LIMIT_AUTHENTICATED_MULTIPLIER"
|
||||||
|
)
|
||||||
|
|
||||||
# Password for tradein_fdw_reader role — used by backend startup to create/refresh
|
# Password for tradein_fdw_reader role — used by backend startup to create/refresh
|
||||||
# USER MAPPING for postgres_fdw → gendesign DB (gendesign_remote server).
|
# USER MAPPING for postgres_fdw → gendesign DB (gendesign_remote server).
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,24 @@
|
||||||
"""Простой in-memory rate-limiter для публичного API.
|
"""Простой in-memory rate-limiter для публичного API.
|
||||||
|
|
||||||
Per-IP sliding window. Достаточно для одного backend-инстанса (MVP).
|
Sliding window. Достаточно для одного backend-инстанса (MVP).
|
||||||
Защищает от абуза `/api/v1/*` — скрейп-эндпоинты уже за admin-токеном,
|
Защищает `/api/v1/*` от абуза — estimate/geocode/suggest публичны.
|
||||||
но estimate/geocode/suggest публичны.
|
|
||||||
|
|
||||||
Лимиты намеренно щедрые — обычный пользователь их не достигнет, режутся боты.
|
Ключ лимита (#2213):
|
||||||
|
- authenticated username (заголовок X-Authenticated-User от Caddy basic_auth) —
|
||||||
|
per-user лимит = rate_limit × rate_limit_authenticated_multiplier;
|
||||||
|
- для анонимов — client IP, лимит = rate_limit.
|
||||||
|
|
||||||
|
Раньше весь authenticated-трафик освобождался целиком (#655). Это было плацебо:
|
||||||
|
заголовок X-Authenticated-User клиент-контролируем (его ставит Caddy, но на общей
|
||||||
|
docker-сети запрос мог прийти и мимо Caddy). Теперь лимит применяется всегда, но
|
||||||
|
per-user порог щедрый — живой пилот его не достигнет.
|
||||||
|
|
||||||
|
Допущение по IP (#2213): перед backend ровно ОДИН доверенный прокси (Caddy).
|
||||||
|
Значит честный клиентский IP = ПОСЛЕДНИЙ (rightmost) элемент X-Forwarded-For,
|
||||||
|
добавленный самим Caddy. Leftmost-элементы клиент может подделать (X-Forwarded-For:
|
||||||
|
"1.2.3.4" в исходном запросе), поэтому брать leftmost — дыра (тривиальный обход
|
||||||
|
per-IP лимита сменой фейкового первого хопа). Если XFF пуст — remote_addr
|
||||||
|
соединения.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
@ -18,10 +32,6 @@ from starlette.middleware.base import BaseHTTPMiddleware
|
||||||
|
|
||||||
from app.core.config import settings
|
from app.core.config import settings
|
||||||
|
|
||||||
# Окно и лимит читаются из settings (env RATE_LIMIT / RATE_LIMIT_WINDOW_S):
|
|
||||||
# не более settings.rate_limit запросов за settings.rate_limit_window_s секунд
|
|
||||||
# с одного IP.
|
|
||||||
|
|
||||||
|
|
||||||
class RateLimitMiddleware(BaseHTTPMiddleware):
|
class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||||
"""Sliding-window rate limit на /api/v1/*. Health и статика — без лимита."""
|
"""Sliding-window rate limit на /api/v1/*. Health и статика — без лимита."""
|
||||||
|
|
@ -36,22 +46,24 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||||
if not path.startswith("/api/"):
|
if not path.startswith("/api/"):
|
||||||
return await call_next(request)
|
return await call_next(request)
|
||||||
|
|
||||||
# Аутентифицированный трафик не лимитируем: Caddy basic_auth ставит
|
# Ключ и порог: per-user для аутентифицированных, per-IP для анонимов.
|
||||||
# X-Authenticated-User на каждый запрос пилота. Лимит — только для
|
username = request.headers.get("x-authenticated-user")
|
||||||
# анонимного трафика (заголовок отсутствует/пуст). #655.
|
if username:
|
||||||
if request.headers.get("x-authenticated-user"):
|
key = f"user:{username}"
|
||||||
return await call_next(request)
|
limit = settings.rate_limit * settings.rate_limit_authenticated_multiplier
|
||||||
|
else:
|
||||||
|
key = f"ip:{_client_ip(request)}"
|
||||||
|
limit = settings.rate_limit
|
||||||
|
|
||||||
ip = _client_ip(request)
|
|
||||||
now = time.monotonic()
|
now = time.monotonic()
|
||||||
bucket = self._hits[ip]
|
bucket = self._hits[key]
|
||||||
|
|
||||||
# Выкидываем устаревшие отметки за пределами окна.
|
# Выкидываем устаревшие отметки за пределами окна.
|
||||||
cutoff = now - settings.rate_limit_window_s
|
cutoff = now - settings.rate_limit_window_s
|
||||||
while bucket and bucket[0] < cutoff:
|
while bucket and bucket[0] < cutoff:
|
||||||
bucket.popleft()
|
bucket.popleft()
|
||||||
|
|
||||||
if len(bucket) >= settings.rate_limit:
|
if len(bucket) >= limit:
|
||||||
retry = int(settings.rate_limit_window_s - (now - bucket[0])) + 1
|
retry = int(settings.rate_limit_window_s - (now - bucket[0])) + 1
|
||||||
return JSONResponse(
|
return JSONResponse(
|
||||||
status_code=429,
|
status_code=429,
|
||||||
|
|
@ -69,8 +81,16 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||||
|
|
||||||
|
|
||||||
def _client_ip(request: Request) -> str:
|
def _client_ip(request: Request) -> str:
|
||||||
"""Реальный IP за реверс-прокси (Caddy/nginx ставят X-Forwarded-For)."""
|
"""Честный клиентский IP при РОВНО ОДНОМ доверенном прокси (Caddy) перед нами.
|
||||||
|
|
||||||
|
Caddy добавляет свой хоп в конец X-Forwarded-For, поэтому берём ПОСЛЕДНИЙ
|
||||||
|
(rightmost) элемент — его нельзя подделать с клиента. Leftmost элементы
|
||||||
|
клиент-контролируемы и для ключа лимита НЕ используются. Пустой XFF (прямое
|
||||||
|
соединение) → remote_addr.
|
||||||
|
"""
|
||||||
xff = request.headers.get("x-forwarded-for")
|
xff = request.headers.get("x-forwarded-for")
|
||||||
if xff:
|
if xff:
|
||||||
return xff.split(",")[0].strip()
|
parts = [p.strip() for p in xff.split(",") if p.strip()]
|
||||||
|
if parts:
|
||||||
|
return parts[-1]
|
||||||
return request.client.host if request.client else "unknown"
|
return request.client.host if request.client else "unknown"
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ import asyncio
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
|
import secrets
|
||||||
from collections.abc import AsyncGenerator, Awaitable, Callable
|
from collections.abc import AsyncGenerator, Awaitable, Callable
|
||||||
from contextlib import asynccontextmanager
|
from contextlib import asynccontextmanager
|
||||||
|
|
||||||
|
|
@ -66,6 +67,16 @@ if settings.glitchtip_dsn:
|
||||||
|
|
||||||
@asynccontextmanager
|
@asynccontextmanager
|
||||||
async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
|
async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
|
||||||
|
# #2213 defense-in-depth: если общий секрет не задан — trusted-header auth
|
||||||
|
# уязвим к подделке X-Authenticated-User изнутри docker-сети gendesign_shared.
|
||||||
|
# Один явный WARNING на старте, чтобы это не осталось незамеченным в проде.
|
||||||
|
if not settings.tradein_internal_auth_secret:
|
||||||
|
logger.warning(
|
||||||
|
"defense-in-depth НЕ активен: X-Authenticated-User принимается без "
|
||||||
|
"проверки внутреннего секрета — задай TRADEIN_INTERNAL_AUTH_SECRET в "
|
||||||
|
".env.runtime ОБОИХ стеков (Caddy главного стека + tradein-backend)"
|
||||||
|
)
|
||||||
|
|
||||||
# FDW bootstrap: create/refresh USER MAPPING for gendesign_remote postgres_fdw server.
|
# FDW bootstrap: create/refresh USER MAPPING for gendesign_remote postgres_fdw server.
|
||||||
# Best-effort: failure does not abort startup, just logs.
|
# Best-effort: failure does not abort startup, just logs.
|
||||||
try:
|
try:
|
||||||
|
|
@ -129,6 +140,25 @@ async def rbac_guard(
|
||||||
content={"detail": "no authenticated user (Caddy basic_auth required)"},
|
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:
|
try:
|
||||||
role = get_role(username)
|
role = get_role(username)
|
||||||
except KeyError:
|
except KeyError:
|
||||||
|
|
@ -154,7 +184,7 @@ app.add_middleware(
|
||||||
allow_methods=["*"],
|
allow_methods=["*"],
|
||||||
allow_headers=["*"],
|
allow_headers=["*"],
|
||||||
)
|
)
|
||||||
# Rate-limit публичного API (per-IP sliding window) — защита от абуза.
|
# Rate-limit публичного API (per-user / per-IP sliding window) — защита от абуза.
|
||||||
app.add_middleware(RateLimitMiddleware)
|
app.add_middleware(RateLimitMiddleware)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
157
tradein-mvp/backend/tests/test_internal_auth_secret.py
Normal file
157
tradein-mvp/backend/tests/test_internal_auth_secret.py
Normal file
|
|
@ -0,0 +1,157 @@
|
||||||
|
"""Defense-in-depth: shared-secret gate поверх trusted-header auth (#2213).
|
||||||
|
|
||||||
|
Backend доверяет X-Authenticated-User (его ставит Caddy). На общей docker-сети
|
||||||
|
gendesign_shared любой контейнер мог бы отправить поддельный
|
||||||
|
`X-Authenticated-User: admin` напрямую на tradein-backend:8000. Общий секрет
|
||||||
|
X-Internal-Auth-Secret закрывает дыру: если TRADEIN_INTERNAL_AUTH_SECRET задан,
|
||||||
|
каждый запрос с X-Authenticated-User обязан нести валидный секрет (constant-time),
|
||||||
|
иначе 401. Пусто = fail-open (backward-compat до провижининга).
|
||||||
|
|
||||||
|
MIRROR of rbac_guard из app/main.py — включая #2213 secret-gate. Держим копию
|
||||||
|
здесь (как test_rbac.py), чтобы не тянуть тяжёлый app.main (lifespan/DB/scheduler).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||||
|
|
||||||
|
import re
|
||||||
|
import secrets
|
||||||
|
from collections.abc import Awaitable, Callable
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from fastapi import FastAPI, Request
|
||||||
|
from fastapi.responses import JSONResponse, Response
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
from app.core import auth as auth_mod
|
||||||
|
from app.core import config
|
||||||
|
|
||||||
|
_ADMIN_API_RE = re.compile(r"^/api/v1/admin/")
|
||||||
|
_PUBLIC_PATHS = frozenset({"/health", "/docs", "/redoc", "/openapi.json"})
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _reset_auth_cache() -> None:
|
||||||
|
auth_mod.reset_cache_for_tests()
|
||||||
|
|
||||||
|
|
||||||
|
def _build_test_app() -> FastAPI:
|
||||||
|
"""Копия rbac_guard из app/main.py (с #2213 secret-gate)."""
|
||||||
|
app = FastAPI()
|
||||||
|
|
||||||
|
@app.middleware("http")
|
||||||
|
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)"},
|
||||||
|
)
|
||||||
|
|
||||||
|
secret = config.settings.tradein_internal_auth_secret
|
||||||
|
if secret:
|
||||||
|
provided = request.headers.get("X-Internal-Auth-Secret", "")
|
||||||
|
if not secrets.compare_digest(provided, secret):
|
||||||
|
return JSONResponse(
|
||||||
|
status_code=401,
|
||||||
|
content={"detail": "invalid or missing internal auth secret"},
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
role = auth_mod.get_role(username)
|
||||||
|
except KeyError:
|
||||||
|
return JSONResponse(
|
||||||
|
status_code=403,
|
||||||
|
content={"detail": "user not in roles config"},
|
||||||
|
)
|
||||||
|
if _ADMIN_API_RE.match(path) and role != "admin":
|
||||||
|
return JSONResponse(status_code=403, content={"detail": "admin only"})
|
||||||
|
return await call_next(request)
|
||||||
|
|
||||||
|
@app.get("/api/v1/ping")
|
||||||
|
async def ping() -> dict:
|
||||||
|
return {"ok": True}
|
||||||
|
|
||||||
|
return app
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def client() -> TestClient:
|
||||||
|
return TestClient(_build_test_app())
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# (в) секрет НЕ задан → backward-compat: X-Authenticated-User достаточно.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_unset_backward_compat(client: TestClient, monkeypatch) -> None:
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "")
|
||||||
|
resp = client.get("/api/v1/ping", headers={"X-Authenticated-User": "admin"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json() == {"ok": True}
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_unset_ignores_provided_secret(client: TestClient, monkeypatch) -> None:
|
||||||
|
# Если секрет не задан — присланный клиентом X-Internal-Auth-Secret игнорируется.
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "")
|
||||||
|
resp = client.get(
|
||||||
|
"/api/v1/ping",
|
||||||
|
headers={"X-Authenticated-User": "admin", "X-Internal-Auth-Secret": "whatever"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# (а) секрет задан + заголовок юзера БЕЗ секрета → 401.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_set_missing_header_401(client: TestClient, monkeypatch) -> None:
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "s3cr3t-value")
|
||||||
|
resp = client.get("/api/v1/ping", headers={"X-Authenticated-User": "admin"})
|
||||||
|
assert resp.status_code == 401
|
||||||
|
assert "internal auth secret" in resp.json()["detail"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_set_wrong_secret_401(client: TestClient, monkeypatch) -> None:
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "s3cr3t-value")
|
||||||
|
resp = client.get(
|
||||||
|
"/api/v1/ping",
|
||||||
|
headers={"X-Authenticated-User": "admin", "X-Internal-Auth-Secret": "wrong"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 401
|
||||||
|
assert "internal auth secret" in resp.json()["detail"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# (б) секрет задан + оба заголовка корректны → 200 (существующее поведение).
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_set_correct_secret_200(client: TestClient, monkeypatch) -> None:
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "s3cr3t-value")
|
||||||
|
resp = client.get(
|
||||||
|
"/api/v1/ping",
|
||||||
|
headers={"X-Authenticated-User": "admin", "X-Internal-Auth-Secret": "s3cr3t-value"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json() == {"ok": True}
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_set_no_user_header_still_401(client: TestClient, monkeypatch) -> None:
|
||||||
|
# Секрет без X-Authenticated-User — no-auth 401 (secret-gate не ослабляет базу).
|
||||||
|
monkeypatch.setattr(config.settings, "tradein_internal_auth_secret", "s3cr3t-value")
|
||||||
|
resp = client.get("/api/v1/ping", headers={"X-Internal-Auth-Secret": "s3cr3t-value"})
|
||||||
|
assert resp.status_code == 401
|
||||||
|
assert "no authenticated user" in resp.json()["detail"].lower()
|
||||||
|
|
@ -1,8 +1,13 @@
|
||||||
"""Tests for RateLimitMiddleware — per-IP sliding window + auth bypass (#655).
|
"""Tests for RateLimitMiddleware — per-user / per-IP sliding window (#655, #2213).
|
||||||
|
|
||||||
Тестируем middleware в изоляции на минимальном FastAPI-приложении, чтобы не
|
Тестируем middleware в изоляции на минимальном FastAPI-приложении, чтобы не
|
||||||
тянуть тяжёлый app.main (DB / scheduler / sentry). Лимит/окно читаются из
|
тянуть тяжёлый app.main (DB / scheduler / sentry). Лимит/окно/множитель читаются
|
||||||
settings на каждый dispatch → monkeypatch'им их в маленькие значения.
|
из settings на каждый dispatch → monkeypatch'им их в маленькие значения.
|
||||||
|
|
||||||
|
#2213: аутентифицированный трафик больше НЕ освобождается целиком — он лимитируется
|
||||||
|
per-user с щедрым порогом (rate_limit × multiplier). Анонимный ключ = client IP,
|
||||||
|
взятый из RIGHTMOST элемента X-Forwarded-For (ровно один доверенный прокси Caddy),
|
||||||
|
поэтому подделка leftmost XFF не меняет ключ.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
|
@ -19,9 +24,10 @@ from app.core.ratelimit import RateLimitMiddleware
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def client(monkeypatch):
|
def client(monkeypatch):
|
||||||
"""Минимальное приложение с лимитом 3 запроса / 60 с (для быстрого 429)."""
|
"""Минимальное приложение: анонимный лимит 3 / 60 с, auth-множитель ×2 (→6)."""
|
||||||
monkeypatch.setattr(config.settings, "rate_limit", 3)
|
monkeypatch.setattr(config.settings, "rate_limit", 3)
|
||||||
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
|
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
|
||||||
|
monkeypatch.setattr(config.settings, "rate_limit_authenticated_multiplier", 2)
|
||||||
|
|
||||||
app = FastAPI()
|
app = FastAPI()
|
||||||
app.add_middleware(RateLimitMiddleware)
|
app.add_middleware(RateLimitMiddleware)
|
||||||
|
|
@ -43,16 +49,66 @@ def test_anonymous_throttled_past_limit(client):
|
||||||
assert "Retry-After" in resp.headers
|
assert "Retry-After" in resp.headers
|
||||||
|
|
||||||
|
|
||||||
def test_authenticated_user_bypasses_limit(client):
|
def test_authenticated_user_is_limited_per_user(client):
|
||||||
# X-Authenticated-User (Caddy basic_auth) → лимит не применяется.
|
# #2213: authenticated НЕ освобождается — лимит = rate_limit × multiplier = 6.
|
||||||
headers = {"X-Authenticated-User": "pilot@example.com"}
|
headers = {"X-Authenticated-User": "alice"}
|
||||||
for _ in range(10): # сильно больше rate_limit=3
|
for _ in range(6):
|
||||||
|
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||||||
|
# 7-й запрос того же юзера — 429.
|
||||||
|
assert client.get("/api/v1/ping", headers=headers).status_code == 429
|
||||||
|
|
||||||
|
|
||||||
|
def test_authenticated_limit_higher_than_anonymous(client):
|
||||||
|
# Auth-порог (6) строго выше анонимного (3): 4-й запрос юзера ещё проходит,
|
||||||
|
# тогда как анонимный на 4-м уже режется (см. test_anonymous_throttled_past_limit).
|
||||||
|
headers = {"X-Authenticated-User": "alice"}
|
||||||
|
for _ in range(4):
|
||||||
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_per_user_key_isolation(client):
|
||||||
|
# Разные юзеры — независимые корзины: alice упирается, bob не затронут.
|
||||||
|
alice = {"X-Authenticated-User": "alice"}
|
||||||
|
bob = {"X-Authenticated-User": "bob"}
|
||||||
|
for _ in range(6):
|
||||||
|
assert client.get("/api/v1/ping", headers=alice).status_code == 200
|
||||||
|
assert client.get("/api/v1/ping", headers=alice).status_code == 429
|
||||||
|
# bob со своим ключом проходит свободно.
|
||||||
|
assert client.get("/api/v1/ping", headers=bob).status_code == 200
|
||||||
|
|
||||||
|
|
||||||
def test_empty_auth_header_does_not_bypass(client):
|
def test_empty_auth_header_does_not_bypass(client):
|
||||||
# Пустой заголовок не должен no-op'ить лимит (header present но falsy).
|
# Пустой заголовок не должен no-op'ить лимит (header present но falsy) → per-IP.
|
||||||
headers = {"X-Authenticated-User": ""}
|
headers = {"X-Authenticated-User": ""}
|
||||||
for _ in range(3):
|
for _ in range(3):
|
||||||
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||||||
assert client.get("/api/v1/ping", headers=headers).status_code == 429
|
assert client.get("/api/v1/ping", headers=headers).status_code == 429
|
||||||
|
|
||||||
|
|
||||||
|
def test_leftmost_xff_does_not_affect_anon_key(client):
|
||||||
|
# Клиент подделывает leftmost XFF, но rightmost (добавленный Caddy) одинаков →
|
||||||
|
# один ключ ip:9.9.9.9 → лимит общий, подделкой его не обойти.
|
||||||
|
spoofed = [
|
||||||
|
{"X-Forwarded-For": "1.1.1.1, 9.9.9.9"},
|
||||||
|
{"X-Forwarded-For": "2.2.2.2, 9.9.9.9"},
|
||||||
|
{"X-Forwarded-For": "3.3.3.3, 9.9.9.9"},
|
||||||
|
]
|
||||||
|
for h in spoofed:
|
||||||
|
assert client.get("/api/v1/ping", headers=h).status_code == 200
|
||||||
|
# 4-й (снова другой leftmost, тот же rightmost) — уже за лимитом.
|
||||||
|
resp = client.get("/api/v1/ping", headers={"X-Forwarded-For": "4.4.4.4, 9.9.9.9"})
|
||||||
|
assert resp.status_code == 429
|
||||||
|
|
||||||
|
|
||||||
|
def test_anonymous_key_from_rightmost_xff(client):
|
||||||
|
# Разный rightmost = разные реальные клиенты = независимые корзины.
|
||||||
|
for _ in range(3):
|
||||||
|
assert (
|
||||||
|
client.get("/api/v1/ping", headers={"X-Forwarded-For": "8.8.8.8, 1.1.1.1"}).status_code
|
||||||
|
== 200
|
||||||
|
)
|
||||||
|
# Другой rightmost — своя корзина, не затронут лимитом первого.
|
||||||
|
assert (
|
||||||
|
client.get("/api/v1/ping", headers={"X-Forwarded-For": "8.8.8.8, 2.2.2.2"}).status_code
|
||||||
|
== 200
|
||||||
|
)
|
||||||
|
|
|
||||||
|
|
@ -88,6 +88,12 @@ services:
|
||||||
# Scheduler отключён в API-процессе — он живёт в tradein-scraper (#1182).
|
# Scheduler отключён в API-процессе — он живёт в tradein-scraper (#1182).
|
||||||
# Это предотвращает двойной запуск sweep'ов при наличии обоих контейнеров.
|
# Это предотвращает двойной запуск sweep'ов при наличии обоих контейнеров.
|
||||||
SCHEDULER_ENABLE: "false"
|
SCHEDULER_ENABLE: "false"
|
||||||
|
# #2213 defense-in-depth: общий секрет с Caddy. ЗАДАН → backend требует
|
||||||
|
# валидный X-Internal-Auth-Secret на каждом запросе с X-Authenticated-User
|
||||||
|
# (иначе 401). Пусто = защита не активна (fail-open до провижининга).
|
||||||
|
# Значение ДОЛЖНО совпадать с TRADEIN_INTERNAL_AUTH_SECRET в .env главного
|
||||||
|
# (Caddy) стека. Читается также из backend/.env.runtime (env_file выше).
|
||||||
|
TRADEIN_INTERNAL_AUTH_SECRET: "${TRADEIN_INTERNAL_AUTH_SECRET:-}"
|
||||||
depends_on:
|
depends_on:
|
||||||
browser:
|
browser:
|
||||||
condition: service_started
|
condition: service_started
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue