gendesign/tradein-mvp/backend/tests/test_estimate_idor.py
bot-backend 466774dbd8
All checks were successful
Deploy Trade-In / build-backend (push) Successful in 40s
Deploy Trade-In / deploy (push) Successful in 34s
Deploy Trade-In / changes (push) Successful in 4s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / test (push) Successful in 24s
fix(tradein): rehydrate POST-only fields on GET /estimate/{id} (#696) (#710)
Co-authored-by: bot-backend <bot-backend@gendsgn.local>
Co-committed-by: bot-backend <bot-backend@gendsgn.local>
2026-05-30 11:05:52 +00:00

526 lines
20 KiB
Python
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

"""Tests for GET /estimate/{id} and /estimate/{id}/pdf ownership scoping (#690 IDOR).
Closes cross-pilot data-leak: any authenticated pilot could read/download another
pilot's estimate (leaking client_name/client_phone). The endpoints now scope to the
owner (trade_in_estimates.created_by) OR admin, mirroring /history (#656).
Реальная БД не нужна: DB + get_role мокируются. Для 200-кейсов подменяем
_qc_geo_to_precision и generate_trade_in_pdf, чтобы не тащить тяжёлый estimator/PDF.
"""
from __future__ import annotations
import os
import sys
from types import SimpleNamespace
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
_ESTIMATE_ID = "11111111-1111-1111-1111-111111111111"
@pytest.fixture(autouse=True)
def _restore_get_role():
"""Restore app.core.auth.get_role after each test (mirror test_history_scope)."""
from app.core import auth as auth_mod
original = auth_mod.get_role
yield
auth_mod.get_role = original
@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_estimate_row(created_by: str | None) -> SimpleNamespace:
"""A trade_in_estimates row with the full column set the endpoints read."""
from datetime import UTC, datetime, timedelta
return SimpleNamespace(
id=_ESTIMATE_ID,
median_price=5_000_000,
range_low=4_500_000,
range_high=5_500_000,
median_price_per_m2=100_000,
confidence="medium",
confidence_explanation="ok",
n_analogs=7,
analogs=[],
actual_deals=[],
sources_used=["avito"],
data_freshness_minutes=10,
expires_at=datetime.now(tz=UTC) + timedelta(hours=12),
address="ул. Тестовая, 1",
lat=56.8,
lon=60.6,
area_m2=50.0,
rooms=2,
floor=3,
total_floors=9,
year_built=2010,
house_type="монолит",
repair_state="хороший",
has_balcony=True,
canonical_address="ул. Тестовая, 1",
house_cadnum=None,
house_fias_id=None,
dadata_qc_geo=0,
dadata_metro=[],
expected_sold_price=4_600_000,
expected_sold_range_low=4_200_000,
expected_sold_range_high=5_000_000,
expected_sold_per_m2=92_000,
asking_to_sold_ratio=0.85,
ratio_basis="per_rooms",
created_by=created_by,
created_at=datetime.now(tz=UTC),
)
def _make_db_mock(row: SimpleNamespace | None) -> MagicMock:
"""DB session mock returning *row* from .execute(...).fetchone()."""
db = MagicMock()
execute_result = MagicMock()
execute_result.fetchone.return_value = row
db.execute.return_value = execute_result
return db
def _client_with(app: FastAPI, db_mock: MagicMock, role: str | None) -> TestClient:
"""Override get_db with *db_mock*; patch get_role to return *role* (or raise KeyError)."""
from app.core.db import get_db
def _override_db():
yield db_mock
app.dependency_overrides[get_db] = _override_db
auth_mod = sys.modules["app.core.auth"]
if role is None:
def _raise_keyerror(_u: str):
raise KeyError(_u)
auth_mod.get_role = _raise_keyerror # type: ignore[assignment]
else:
auth_mod.get_role = lambda _u: role # type: ignore[assignment]
return TestClient(app)
@pytest.fixture(autouse=True)
def _stub_precision_and_pdf():
"""Stub _qc_geo_to_precision + generate_trade_in_pdf so 200-paths don't pull heavy deps."""
from app.api.v1 import trade_in as trade_in_module
# #696: GET-rehydrate pulls four POST-only helpers from estimator. Stub them to
# None by default (graceful "no data" path); the positive rehydrate test overrides
# them per-call with data-returning lambdas.
estimator_stub = SimpleNamespace(
_qc_geo_to_precision=lambda _qc: "house",
_fetch_price_trend=lambda *a, **k: None,
_fetch_dkp_corridor=lambda *a, **k: None,
_fetch_house_imv_anchor=lambda *a, **k: None,
)
real_estimator = sys.modules.get("app.services.estimator")
sys.modules["app.services.estimator"] = estimator_stub # type: ignore[assignment]
original_pdf = trade_in_module.generate_trade_in_pdf
trade_in_module.generate_trade_in_pdf = lambda *a, **k: b"%PDF-fake" # type: ignore[assignment]
# brand resolution also imports lazily — stub the module it imports from.
brand_stub = SimpleNamespace(get_brand=lambda _b, _db: SimpleNamespace(slug="generic"))
real_brand = sys.modules.get("app.services.brand")
sys.modules["app.services.brand"] = brand_stub # type: ignore[assignment]
yield
trade_in_module.generate_trade_in_pdf = original_pdf
if real_estimator is not None:
sys.modules["app.services.estimator"] = real_estimator
else:
sys.modules.pop("app.services.estimator", None)
if real_brand is not None:
sys.modules["app.services.brand"] = real_brand
else:
sys.modules.pop("app.services.brand", None)
# ── GET /estimate/{id} ───────────────────────────────────────────────────────
def test_get_estimate_owner_can_read(trade_in_app: FastAPI) -> None:
"""Owner pilot reads own estimate → 200."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
assert resp.json()["estimate_id"] == _ESTIMATE_ID
def test_get_estimate_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Non-owner pilot must NOT read someone else's estimate → 404 (hide existence)."""
db_mock = _make_db_mock(_make_estimate_row(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404
def test_get_estimate_admin_can_read_any(trade_in_app: FastAPI) -> None:
"""Admin reads any estimate regardless of owner → 200."""
db_mock = _make_db_mock(_make_estimate_row(created_by="someone_else"))
client = _client_with(trade_in_app, db_mock, role="admin")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "admin"},
)
assert resp.status_code == 200
def test_get_estimate_rehydrates_post_only_fields(trade_in_app: FastAPI) -> None:
"""#696: GET /{id} recomputes price_trend / avito_imv / dkp_corridor / last_scraped_at.
Previously these POST-only derived fields came back null on shared-link / PDF /
?id= restore. Stub the estimator helpers with data and assert they surface.
"""
est = sys.modules["app.services.estimator"]
est._fetch_price_trend = lambda *a, **k: [ # type: ignore[attr-defined]
{"month": "2026-03", "ppm2": 140_000},
{"month": "2026-04", "ppm2": 150_000},
{"month": "2026-05", "ppm2": 160_000},
]
est._fetch_house_imv_anchor = lambda *a, **k: { # type: ignore[attr-defined]
"recommended_price": 5_200_000,
"lower_price": 4_800_000,
"higher_price": 5_600_000,
"market_count": 42,
}
est._fetch_dkp_corridor = lambda *a, **k: { # type: ignore[attr-defined]
"count": 5,
"low_ppm2": 120_000,
"median_ppm2": 150_000,
"high_ppm2": 180_000,
"period_months": 12,
}
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
body = resp.json()
assert len(body["price_trend"]) == 3
assert body["avito_imv"]["recommended_price"] == 5_200_000
assert body["dkp_corridor"]["count"] == 5
# last_scraped_at = created_at data_freshness_minutes (both persisted) → non-null
assert body["last_scraped_at"] is not None
def test_get_estimate_requires_authenticated_user(trade_in_app: FastAPI) -> None:
"""No X-Authenticated-User header → 401."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}")
assert resp.status_code == 401
def test_get_estimate_unknown_user_gets_403(trade_in_app: FastAPI) -> None:
"""Authenticated via Caddy but missing from roles.yaml → 403."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role=None)
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "ghost"},
)
assert resp.status_code == 403
# ── GET /estimate/{id}/pdf ───────────────────────────────────────────────────
def test_pdf_owner_can_download(trade_in_app: FastAPI) -> None:
"""Owner pilot downloads own PDF → 200."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/pdf",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
assert resp.headers["content-type"] == "application/pdf"
def test_pdf_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Non-owner pilot must NOT download someone else's PDF → 404."""
db_mock = _make_db_mock(_make_estimate_row(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/pdf",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404
def test_pdf_admin_can_download_any(trade_in_app: FastAPI) -> None:
"""Admin downloads any PDF regardless of owner → 200."""
db_mock = _make_db_mock(_make_estimate_row(created_by="someone_else"))
client = _client_with(trade_in_app, db_mock, role="admin")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/pdf",
headers={"X-Authenticated-User": "admin"},
)
assert resp.status_code == 200
def test_pdf_requires_authenticated_user(trade_in_app: FastAPI) -> None:
"""No X-Authenticated-User header → 401."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/pdf")
assert resp.status_code == 401
def test_pdf_unknown_user_gets_403(trade_in_app: FastAPI) -> None:
"""/pdf: authenticated via Caddy but missing from roles.yaml → 403 (reviewer gap)."""
db_mock = _make_db_mock(_make_estimate_row(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role=None)
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/pdf",
headers={"X-Authenticated-User": "ghost"},
)
assert resp.status_code == 403
# ── Reviewer-named gap: legacy NULL created_by ───────────────────────────────
def test_get_estimate_legacy_null_owner_non_admin_gets_404(trade_in_app: FastAPI) -> None:
"""Legacy estimate with NULL created_by: non-admin pilot must NOT read it → 404."""
db_mock = _make_db_mock(_make_estimate_row(created_by=None))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 404
# ── Photo endpoints (#690 — raw apartment-interior bytes are enumerable PII) ──
def _make_photo_row() -> SimpleNamespace:
"""A minimal estimate_photos row for get_photo (content + content_type)."""
return SimpleNamespace(content=b"\x89PNG-fake", content_type="image/png")
def _make_db_mock_seq(*rows: SimpleNamespace | None) -> MagicMock:
"""DB session mock whose successive .execute(...).fetchone() yield *rows* in order.
Derived/photo routes run the guard SELECT (created_by) first, then their own
queries; configure side_effect so the guard row comes first.
"""
db = MagicMock()
def _execute(*_a, **_k):
result = MagicMock()
result.fetchone.return_value = next(_iter)
return result
_iter = iter(rows)
db.execute.side_effect = _execute
return db
def test_get_photo_owner_can_read(trade_in_app: FastAPI) -> None:
"""Owner pilot reads photo bytes → 200."""
db_mock = _make_db_mock_seq(
SimpleNamespace(created_by="kopylov"), # guard query
_make_photo_row(), # route query
)
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
assert resp.content == b"\x89PNG-fake"
def test_get_photo_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Non-owner pilot must NOT read someone else's photo → 404 (guard raises first)."""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404
def test_get_photo_admin_can_read_any(trade_in_app: FastAPI) -> None:
"""Admin reads any photo → 200."""
db_mock = _make_db_mock_seq(
SimpleNamespace(created_by="someone_else"),
_make_photo_row(),
)
client = _client_with(trade_in_app, db_mock, role="admin")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos/{_ESTIMATE_ID}",
headers={"X-Authenticated-User": "admin"},
)
assert resp.status_code == 200
def test_get_photo_requires_authenticated_user(trade_in_app: FastAPI) -> None:
"""get_photo: no X-Authenticated-User header → 401."""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="kopylov"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos/{_ESTIMATE_ID}",
)
assert resp.status_code == 401
def test_list_photos_owner_can_read(trade_in_app: FastAPI) -> None:
"""Owner pilot lists photos → 200 (empty list ok)."""
db = MagicMock()
guard_result = MagicMock()
guard_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
list_result = MagicMock()
list_result.mappings.return_value.all.return_value = []
db.execute.side_effect = [guard_result, list_result]
client = _client_with(trade_in_app, db, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
assert resp.json() == []
def test_list_photos_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Non-owner pilot must NOT list someone else's photos → 404."""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404
def test_upload_photo_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Write-side IDOR: non-owner pilot must NOT upload to someone else's estimate → 404.
Guard runs on the existence SELECT (created_by) before any write — so a single
fetchone returning created_by='admin' + a non-owner header is enough.
"""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="admin"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.post(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos",
headers={"X-Authenticated-User": "attacker"},
files={"file": ("x.png", b"\x89PNG-fake", "image/png")},
)
assert resp.status_code == 404
def test_upload_photo_owner_can_upload(trade_in_app: FastAPI, monkeypatch) -> None:
"""Owner pilot uploads → 200. Stub sanitize_image + the INSERT mapping row."""
from app.api.v1 import trade_in as trade_in_module
monkeypatch.setattr(
trade_in_module, "sanitize_image", lambda _c: (b"\x89PNG-clean", "image/png")
)
from datetime import UTC, datetime
db = MagicMock()
guard_result = MagicMock()
guard_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
count_result = MagicMock()
count_result.scalar_one.return_value = 0
insert_result = MagicMock()
insert_result.mappings.return_value.fetchone.return_value = {
"id": _ESTIMATE_ID,
"filename": "x.png",
"content_type": "image/png",
"size_bytes": 9,
"uploaded_at": datetime.now(tz=UTC),
}
db.execute.side_effect = [guard_result, count_result, insert_result]
client = _client_with(trade_in_app, db, role="pilot")
resp = client.post(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/photos",
headers={"X-Authenticated-User": "kopylov"},
files={"file": ("x.png", b"\x89PNG-fake", "image/png")},
)
assert resp.status_code == 200
# ── Derived analytics routes (representative: /houses, /imv-benchmark) ────────
def test_get_estimate_houses_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""Derived route: non-owner pilot blocked by guard → 404 (before any house query)."""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/houses",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404
def test_get_estimate_houses_owner_can_read(trade_in_app: FastAPI) -> None:
"""Derived route: owner pilot passes guard → 200. Stub target with no address/geo."""
db = MagicMock()
guard_result = MagicMock()
guard_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
target_result = MagicMock()
target_result.fetchone.return_value = SimpleNamespace(lat=None, lon=None, address=None)
db.execute.side_effect = [guard_result, target_result]
client = _client_with(trade_in_app, db, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/houses",
headers={"X-Authenticated-User": "kopylov"},
)
assert resp.status_code == 200
assert resp.json() == []
def test_get_estimate_imv_benchmark_other_pilot_gets_404(trade_in_app: FastAPI) -> None:
"""imv-benchmark: non-owner pilot blocked by guard → 404."""
db_mock = _make_db_mock_seq(SimpleNamespace(created_by="victim"))
client = _client_with(trade_in_app, db_mock, role="pilot")
resp = client.get(
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}/imv-benchmark",
headers={"X-Authenticated-User": "attacker"},
)
assert resp.status_code == 404