lekss361/gendesign
1
0
Fork 0
Code Issues 57 Pull requests Projects Releases Packages Wiki Activity Actions

Merge remote-tracking branch 'forgejo/main' into fix/tradein-654-502-timeout

Browse source 
# Conflicts:
#	tradein-mvp/backend/app/api/v1/trade_in.py
#	tradein-mvp/backend/app/core/config.py
This commit is contained in:
lekss361 2026-05-29 19:10:58 +03:00
commit 55e1dd1303
13 changed files with 360 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
backend/app/api/v1/ping.py Normal file
View file

@ -0,0 +1,8 @@
from fastapi import APIRouter
router = APIRouter()
@router.get("/ping")
async def ping() -> dict[str, bool]:
return {"pong": True}

4
backend/app/main.py
View file

@ -30,6 +30,7 @@ from app.api.v1 import (
parcels,
photos,
pilot,
ping,
trade_in,
users,
)
@ -86,7 +87,7 @@ app = FastAPI(title="GenDesign API", version="0.1.0", lifespan=lifespan)
# 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"})
_PUBLIC_PATHS = frozenset({"/health", "/api/v1/ping", "/docs", "/redoc", "/openapi.json"})
@app.middleware("http")
@ -164,6 +165,7 @@ app.include_router(landing.router, prefix="/api/v1", tags=["landing"])
app.include_router(pilot.router, prefix="/api/v1/pilot", tags=["pilot"])
app.include_router(users.router, prefix="/api/v1", tags=["users"])
app.include_router(me.router, prefix="/api/v1", tags=["me"])
app.include_router(ping.router, prefix="/api/v1", tags=["ping"])
@app.get("/health")

10
backend/tests/test_ping.py Normal file
View file

@ -0,0 +1,10 @@
from fastapi.testclient import TestClient
from app.main import app
def test_ping() -> None:
client = TestClient(app)
response = client.get("/api/v1/ping")
assert response.status_code == 200
assert response.json() == {"pong": True}

47
tradein-mvp/backend/app/api/v1/trade_in.py
View file

@ -67,8 +67,9 @@ async def estimate(
# через logger.exception (→ GlitchTip/Sentry получает stack trace) и отдаём
# явный 503 — так любая БУДУЩАЯ реальная ошибка становится видимой, а не
# «глотается» шлюзом. HTTPException пробрасываем как есть (это не сбой).
# created_by (#656) прокидываем в estimate_quality для скоупа /history.
try:
result = await estimate_quality(payload, db)
result = await estimate_quality(payload, db, created_by=x_authenticated_user)
except HTTPException:
raise
except Exception:
@ -412,19 +413,57 @@ def get_photo(
def estimate_history(
db: Annotated[Session, Depends(get_db)],
limit: int = 50,
account: str | None = None,
x_authenticated_user: Annotated[str | None, Header(alias="X-Authenticated-User")] = None,
) -> list[dict[str, object]]:
"""История оценок (#399) — последние N записей trade_in_estimates."""
"""История оценок (#399) — последние N записей trade_in_estimates.
Скоупинг (#656 — закрывает cross-pilot data-leak): non-admin видит ТОЛЬКО
свои оценки (created_by = X-Authenticated-User); legacy NULL-строки без
владельца не попадают. Admin видит все строки, либо фильтрует по ?account=<user>.
401 если заголовок отсутствует (mirror /me — Caddy basic_auth обязателен).
"""
if not x_authenticated_user:
raise HTTPException(
status_code=401,
detail="no authenticated user (Caddy basic_auth required)",
)
from app.core.auth import get_role
try:
role = get_role(x_authenticated_user)
except KeyError:
logger.warning(
"user %r authenticated via Caddy but missing from roles.yaml",
x_authenticated_user,
)
raise HTTPException(status_code=403, detail="user not in roles config") from None
params: dict[str, object] = {"limit": min(max(limit, 1), 200)}
where = ""
if role == "admin":
# Admin: все строки, либо фильтр по конкретному аккаунту.
if account:
where = "WHERE created_by = :account"
params["account"] = account
else:
# Non-admin: жёстко скоупим на свои строки; ?account игнорируется.
where = "WHERE created_by = :owner"
params["owner"] = x_authenticated_user
rows = db.execute(
text(
"""
f"""
SELECT id, address, rooms, area_m2, median_price,
confidence, n_analogs, created_at
FROM trade_in_estimates
{where}
ORDER BY created_at DESC
LIMIT :limit
"""
),
{"limit": min(max(limit, 1), 200)},
params,
).mappings().all()
return [dict(r) for r in rows]

11
tradein-mvp/backend/app/core/config.py
View file

@ -34,6 +34,13 @@ class Settings(BaseSettings):
# Redis URL для hot-cache (Phase 3.2). Задаётся через env REDIS_URL.
redis_url: str = "redis://localhost:6379/0"
# Rate-limit публичного /api/* (per-IP sliding window). ENV: RATE_LIMIT,
# RATE_LIMIT_WINDOW_S. Не более rate_limit запросов за rate_limit_window_s
# секунд с одного IP. Аутентифицированный трафик (X-Authenticated-User от
# Caddy basic_auth) не лимитируется — см. ratelimit.py (#655).
rate_limit: int = 300
rate_limit_window_s: float = 60.0
# 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 не создаётся, gendesign_cad_buildings не работает (dev).
@ -62,5 +69,9 @@ class Settings(BaseSettings):
estimate_geocode_budget_s: float = 12.0
estimate_house_meta_timeout_s: float = 8.0
# Лимит успешных оценок trade-in за календарный месяц на аккаунт (#658).
# Конфигурируется через env ESTIMATE_QUOTA_LIMIT. Default 15.
estimate_quota_limit: int = 15
settings = Settings()

20
tradein-mvp/backend/app/core/ratelimit.py
View file

@ -16,9 +16,11 @@ from fastapi import Request
from fastapi.responses import JSONResponse
from starlette.middleware.base import BaseHTTPMiddleware
# Окно и лимит: не более RATE_LIMIT запросов за RATE_WINDOW секунд с одного IP.
RATE_WINDOW = 60.0
RATE_LIMIT = 90
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):
@ -34,17 +36,23 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
if not path.startswith("/api/"):
return await call_next(request)
# Аутентифицированный трафик не лимитируем: Caddy basic_auth ставит
# X-Authenticated-User на каждый запрос пилота. Лимит — только для
# анонимного трафика (заголовок отсутствует/пуст). #655.
if request.headers.get("x-authenticated-user"):
return await call_next(request)
ip = _client_ip(request)
now = time.monotonic()
bucket = self._hits[ip]
# Выкидываем устаревшие отметки за пределами окна.
cutoff = now - RATE_WINDOW
cutoff = now - settings.rate_limit_window_s
while bucket and bucket[0] < cutoff:
bucket.popleft()
if len(bucket) >= RATE_LIMIT:
retry = int(RATE_WINDOW - (now - bucket[0])) + 1
if len(bucket) >= settings.rate_limit:
retry = int(settings.rate_limit_window_s - (now - bucket[0])) + 1
return JSONResponse(
status_code=429,
content={"detail": "Слишком много запросов. Попробуйте позже."},

13
tradein-mvp/backend/app/services/account_quota.py
View file

@ -1,7 +1,8 @@
"""Сервис квоты оценок trade-in — 15 успешных оценок в месяц на аккаунт.
"""Сервис квоты оценок trade-in — N успешных оценок в месяц на аккаунт.
Правила:
- Лимит = 15 успешных оценок за календарный месяц (UTC, период 'YYYY-MM').
- Лимит = settings.estimate_quota_limit успешных оценок за календарный месяц
(UTC, период 'YYYY-MM'); конфигурируется через env ESTIMATE_QUOTA_LIMIT, default 15.
- Без лимита (unlimited): роль admin ИЛИ username == 'kopylov'.
- Учитываются ТОЛЬКО успешные оценки (инкремент ПОСЛЕ estimate_quality).
- Если заголовок X-Authenticated-User отсутствует (dev без Caddy) → unlimited,
@ -19,12 +20,16 @@ from sqlalchemy import text
from sqlalchemy.orm import Session
from app.core.auth import get_role
from app.core.config import settings
logger = logging.getLogger(__name__)
MONTHLY_LIMIT = 15
# Лимит успешных оценок за календарный месяц — конфигурируется через
# env ESTIMATE_QUOTA_LIMIT (core.config.Settings), default 15 (#658).
MONTHLY_LIMIT = settings.estimate_quota_limit
LIMIT_EXHAUSTED_MESSAGE = (
"Лимит из 15 оценок в этом месяце исчерпан. Свяжитесь с Артёмом Копыловым."
f"Лимит из {MONTHLY_LIMIT} оценок в этом месяце исчерпан. "
"За полной версией обращайтесь к Копылову."
)

14
tradein-mvp/backend/app/services/estimator.py
View file

@ -682,7 +682,9 @@ async def _with_budget(coro: Any, budget_s: float, *, label: str) -> Any:
# ── Public ───────────────────────────────────────────────────────────────────
async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> AggregatedEstimate:
async def estimate_quality(
payload: TradeInEstimateInput, db: Session, created_by: str | None = None
) -> AggregatedEstimate:
"""Главная функция — оценка квартиры по реальным данным.
PR M / #564 Phase 3: rosreestr_deals **included** в actual_deals output.
@ -708,7 +710,7 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
if geo is None:
# Без координат не можем искать через PostGIS. Возвращаем low confidence.
logger.warning("geocode failed for %s — returning low-confidence estimate", payload.address)
return _empty_estimate(payload, db, reason="address_not_geocoded")
return _empty_estimate(payload, db, reason="address_not_geocoded", created_by=created_by)
# 1b. DaData enrichment (PR Q1) — on-demand cleanup для target адреса.
# Best-effort: graceful None при отсутствии credentials / quota / fail.
@ -1066,6 +1068,7 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
expected_sold_price, expected_sold_range_low,
expected_sold_range_high, expected_sold_per_m2,
asking_to_sold_ratio, ratio_basis,
created_by,
expires_at
) VALUES (
CAST(:id AS uuid),
@ -1085,6 +1088,7 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
:expected_sold_price, :expected_sold_range_low,
:expected_sold_range_high, :expected_sold_per_m2,
:asking_to_sold_ratio, :ratio_basis,
:created_by,
:expires_at
)
"""
@ -1133,6 +1137,7 @@ async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> Aggreg
"expected_sold_per_m2": expected_sold_per_m2,
"asking_to_sold_ratio": asking_to_sold_ratio,
"ratio_basis": ratio_basis,
"created_by": created_by,
"expires_at": expires_at,
},
)
@ -2067,7 +2072,7 @@ def _deal_to_analog(row: dict[str, Any]) -> AnalogLot:
def _empty_estimate(
payload: TradeInEstimateInput, db: Session, *, reason: str
payload: TradeInEstimateInput, db: Session, *, reason: str, created_by: str | None = None
) -> AggregatedEstimate:
"""Fallback когда нет данных для оценки.
@ -2090,6 +2095,7 @@ def _empty_estimate(
confidence, confidence_explanation, n_analogs,
analogs, actual_deals,
sources_used,
created_by,
expires_at
) VALUES (
CAST(:id AS uuid), :address,
@ -2100,6 +2106,7 @@ def _empty_estimate(
'low', :explanation, 0,
'[]'::jsonb, '[]'::jsonb,
'[]'::jsonb,
:created_by,
:expires_at
)
"""
@ -2120,6 +2127,7 @@ def _empty_estimate(
"client_name": payload.client_name,
"client_phone": payload.client_phone,
"explanation": reason,
"created_by": created_by,
"expires_at": expires_at,
},
)

34
tradein-mvp/backend/data/sql/083_trade_in_estimates_created_by.sql Normal file
View file

@ -0,0 +1,34 @@
-- 083_trade_in_estimates_created_by.sql
-- #656 [P1, security/data-leak] — добавляем владельца оценки для скоупинга GET /history.
--
-- ПРОБЛЕМА: GET /history SELECT'ил последние оценки ВСЕХ пользователей (без фильтра)
-- → cross-pilot data leak (один пилот видел адреса/клиентов другого). trade_in_estimates
-- не имела колонки владельца. Caddy basic_auth прокидывает X-Authenticated-User в каждый
-- аутентифицированный запрос; RBAC middleware уже 401'ит неизвестных юзеров, так что
-- заголовок гарантированно присутствует для реальных вызовов.
--
-- ФИКС: created_by text (username из X-Authenticated-User). Estimator пишет его на INSERT.
-- GET /history скоупит: non-admin → WHERE created_by = :user; admin → все строки
-- (или ?account=<user> фильтр). Legacy-строки остаются created_by=NULL — они без
-- владельца, поэтому non-admin их НЕ видит (корректно: нет owner = не показываем).
--
-- Индекс (created_by, created_at DESC) обслуживает основной паттерн запроса /history
-- (ORDER BY created_at DESC LIMIT N, опционально WHERE created_by = :user).
-- Idempotent: ADD COLUMN IF NOT EXISTS / CREATE INDEX IF NOT EXISTS.
-- Apply after: 082_scrape_schedules_seed_ratio_refresh.sql
BEGIN;
ALTER TABLE trade_in_estimates
ADD COLUMN IF NOT EXISTS created_by text;
CREATE INDEX IF NOT EXISTS idx_trade_in_estimates_created_by_created_at
ON trade_in_estimates (created_by, created_at DESC);
COMMENT ON COLUMN trade_in_estimates.created_by IS
'Username (X-Authenticated-User из Caddy basic_auth), создавший оценку. '
'Используется для скоупинга GET /history (#656): non-admin видит только свои '
'строки. NULL для legacy-строк, созданных до миграции 083 — они без владельца '
'и в non-admin историю не попадают.';
COMMIT;

3
tradein-mvp/backend/tests/test_account_quota.py
View file

@ -147,7 +147,8 @@ def test_check_and_raise_pilot_blocked_exact_detail() -> None:
with pytest.raises(HTTPException) as exc_info:
check_and_raise(db, "user3")
assert exc_info.value.detail == (
"Лимит из 15 оценок в этом месяце исчерпан. Свяжитесь с Артёмом Копыловым."
f"Лимит из {MONTHLY_LIMIT} оценок в этом месяце исчерпан. "
"За полной версией обращайтесь к Копылову."
)

155
tradein-mvp/backend/tests/test_history_scope.py Normal file
View file

@ -0,0 +1,155 @@
"""Tests for GET /api/v1/trade-in/history scoping (#656).
Закрывает cross-pilot data-leak: non-admin видит только свои оценки
(created_by = X-Authenticated-User), admin видит все либо фильтрует по ?account.
Проверяем САМ скоупинг через перехват SQL/params, переданных в db.execute —
реальная БД не нужна (DB + get_role мокируются).
"""
from __future__ import annotations
import os
import sys
from unittest.mock import MagicMock
# psycopg v3 driver required; stub DATABASE_URL before any app import
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
# WeasyPrint requires GTK — not present in CI/Windows. Stub before any app import.
_wp_mock = MagicMock()
sys.modules.setdefault("weasyprint", _wp_mock)
sys.modules.setdefault("weasyprint.CSS", _wp_mock)
sys.modules.setdefault("weasyprint.HTML", _wp_mock)
import pytest # noqa: E402
from fastapi import FastAPI # noqa: E402
from fastapi.testclient import TestClient # noqa: E402
@pytest.fixture()
def trade_in_app() -> FastAPI:
"""Minimal FastAPI app mounting only the trade-in router."""
from app.api.v1 import trade_in as trade_in_module
application = FastAPI()
application.include_router(trade_in_module.router, prefix="/api/v1/trade-in")
return application
def _make_db_mock(rows: list[dict]) -> MagicMock:
"""DB session mock returning *rows* from .mappings().all()."""
db = MagicMock()
mapping_result = MagicMock()
mapping_result.all.return_value = rows
execute_result = MagicMock()
execute_result.mappings.return_value = mapping_result
db.execute.return_value = execute_result
return db
def _captured_sql_and_params(db_mock: MagicMock) -> tuple[str, dict]:
"""Extract the SQL text + params dict from the single db.execute call."""
args, _ = db_mock.execute.call_args
sql = str(args[0])
params = args[1]
return sql, params
def _client_with(app: FastAPI, db_mock: MagicMock, role: str) -> TestClient:
"""Override get_db with *db_mock* and patch get_role to return *role*."""
from app.api.v1 import trade_in as trade_in_module
from app.core.db import get_db
def _override_db():
yield db_mock
app.dependency_overrides[get_db] = _override_db
# estimate_history imports get_role from app.core.auth at call time → patch source.
trade_in_module_auth = sys.modules["app.core.auth"]
trade_in_module_auth.get_role = lambda _u: role # type: ignore[assignment]
_ = trade_in_module # keep router import side-effect explicit
return TestClient(app)
def test_history_requires_authenticated_user(trade_in_app: FastAPI) -> None:
"""No X-Authenticated-User header → 401 (mirror /me)."""
db_mock = _make_db_mock([])
from app.core.db import get_db
def _override_db():
yield db_mock
trade_in_app.dependency_overrides[get_db] = _override_db
client = TestClient(trade_in_app)
resp = client.get("/api/v1/trade-in/history")
assert resp.status_code == 401
# DB must not be touched when unauthenticated.
db_mock.execute.assert_not_called()
def test_non_admin_sees_only_own_rows(trade_in_app: FastAPI) -> None:
"""Pilot user → WHERE created_by = :owner with owner == header value."""
db_mock = _make_db_mock([{"id": "1", "address": "A", "rooms": 2,
"area_m2": 50, "median_price": 5_000_000,
"confidence": "medium", "n_analogs": 7,
"created_at": "2026-05-29T00:00:00"}])
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
"/api/v1/trade-in/history",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
sql, params = _captured_sql_and_params(db_mock)
assert "created_by = :owner" in sql
assert params["owner"] == "kopylov"
assert "account" not in params
def test_non_admin_account_param_is_ignored(trade_in_app: FastAPI) -> None:
"""Pilot passing ?account=victim must still be scoped to themselves."""
db_mock = _make_db_mock([])
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
"/api/v1/trade-in/history",
params={"account": "victim"},
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
sql, params = _captured_sql_and_params(db_mock)
assert "created_by = :owner" in sql
assert params["owner"] == "kopylov"
# The attacker-supplied account must NOT reach the query.
assert "account" not in params
assert "victim" not in params.values()
def test_admin_sees_all_rows(trade_in_app: FastAPI) -> None:
"""Admin without ?account → no WHERE filter (all rows)."""
db_mock = _make_db_mock([])
client = _client_with(trade_in_app, db_mock, role="admin")
resp = client.get(
"/api/v1/trade-in/history",
headers={"X-Authenticated-User": "admin"},
)
assert resp.status_code == 200
sql, params = _captured_sql_and_params(db_mock)
assert "WHERE" not in sql.upper()
assert "owner" not in params
assert "account" not in params
def test_admin_filters_by_account(trade_in_app: FastAPI) -> None:
"""Admin with ?account=kopylov → WHERE created_by = :account."""
db_mock = _make_db_mock([])
client = _client_with(trade_in_app, db_mock, role="admin")
resp = client.get(
"/api/v1/trade-in/history",
params={"account": "kopylov"},
headers={"X-Authenticated-User": "admin"},
)
assert resp.status_code == 200
sql, params = _captured_sql_and_params(db_mock)
assert "created_by = :account" in sql
assert params["account"] == "kopylov"
assert "owner" not in params

58
tradein-mvp/backend/tests/test_ratelimit.py Normal file
View file

@ -0,0 +1,58 @@
"""Tests for RateLimitMiddleware — per-IP sliding window + auth bypass (#655).
Тестируем middleware в изоляции на минимальном FastAPI-приложении, чтобы не
тянуть тяжёлый app.main (DB / scheduler / sentry). Лимит/окно читаются из
settings на каждый dispatch → monkeypatch'им их в маленькие значения.
"""
import os
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
import pytest
from fastapi import FastAPI
from fastapi.testclient import TestClient
from app.core import config
from app.core.ratelimit import RateLimitMiddleware
@pytest.fixture
def client(monkeypatch):
"""Минимальное приложение с лимитом 3 запроса / 60 с (для быстрого 429)."""
monkeypatch.setattr(config.settings, "rate_limit", 3)
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
app = FastAPI()
app.add_middleware(RateLimitMiddleware)
@app.get("/api/v1/ping")
def ping() -> dict[str, bool]:
return {"ok": True}
return TestClient(app)
def test_anonymous_throttled_past_limit(client):
# Первые rate_limit запросов проходят, следующий — 429.
for _ in range(3):
assert client.get("/api/v1/ping").status_code == 200
resp = client.get("/api/v1/ping")
assert resp.status_code == 429
assert resp.json() == {"detail": "Слишком много запросов. Попробуйте позже."}
assert "Retry-After" in resp.headers
def test_authenticated_user_bypasses_limit(client):
# X-Authenticated-User (Caddy basic_auth) → лимит не применяется.
headers = {"X-Authenticated-User": "pilot@example.com"}
for _ in range(10): # сильно больше rate_limit=3
assert client.get("/api/v1/ping", headers=headers).status_code == 200
def test_empty_auth_header_does_not_bypass(client):
# Пустой заголовок не должен no-op'ить лимит (header present но falsy).
headers = {"X-Authenticated-User": ""}
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 == 429

3
tradein-mvp/frontend/src/components/trade-in/EstimateForm.tsx
View file

@ -429,7 +429,8 @@ export function EstimateForm({
marginBottom: 8,
}}
>
Лимит из 15 оценок в этом месяце исчерпан. Свяжитесь с Артёмом Копыловым.
Лимит из {limit ?? 15} оценок в этом месяце исчерпан. За полной версией
обращайтесь к Копылову.
</div>
) : (
<>