df943ea1c7
All checks were successful
CI Trade-In / changes (pull_request) Successful in 13s
CI / changes (pull_request) Successful in 13s
CI Trade-In / frontend-checks (pull_request) Has been skipped
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
CI Trade-In / backend-tests (pull_request) Successful in 2m18s
Deep-review seed'а (#2564) нашёл смежный дефект в уже смерженном коде (#2563): Python `$` матчит перед trailing newline (re.match(r'...\$', 'admin\n') -> True), а Postgres `~` в CHECK tradein_users_username_ascii_ck (миграция 193) - False. username="admin\n" проходил Pydantic-валидацию и падал уже в БД -> 500 вместо честного 422. `\Z` - конец строки без поблажки на trailing newline, совпадает с семантикой Postgres `~`. Grep по app/schemas/ (pattern=/regex=/re.compile/re.match/re.fullmatch) - других regex-валидаторов с `$` в схемах trade-in нет, team.py - единственный файл с regex-based полем.
1031 lines
42 KiB
Python
1031 lines
42 KiB
Python
"""Integration tests for #2554 team-management API — employees CRUD, quotas, history.
|
||
|
||
Same pattern as `tests/test_auth_api.py`: real `rbac_guard` + real `auth.router` /
|
||
`team.router` wired into an isolated FastAPI test app, with an in-memory fake DB
|
||
(`_Store`/`_FakeDB`) dispatching on SQL text standing in for `tradein_users` /
|
||
`tradein_sessions` / `account_quota_overrides` / `account_estimate_usage` /
|
||
`user_events` / `trade_in_estimates`.
|
||
|
||
`app.core.rbac.SessionLocal` (middleware, no FastAPI DI) and `app.core.db.get_db`
|
||
(auth.router / team.router `Depends(get_db)`) both point at the SAME `_Store`
|
||
instance per test — a session created via POST /login is immediately visible to
|
||
rbac_guard's own DB round trip AND to `current_team_actor`.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import os
|
||
from datetime import UTC, datetime, timedelta
|
||
from types import SimpleNamespace
|
||
from typing import Any
|
||
from uuid import uuid4
|
||
|
||
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.api.v1 import auth as auth_router
|
||
from app.api.v1 import team as team_router
|
||
from app.core import auth as auth_mod
|
||
from app.core import config
|
||
from app.core.db import get_db
|
||
from app.core.password import hash_password
|
||
from app.core.rbac import rbac_guard
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# Fake DB backing tradein_users / tradein_sessions / quota / user_events
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class _Store:
|
||
def __init__(self) -> None:
|
||
self.users: dict[str, dict[str, Any]] = {} # username -> user dict
|
||
self.sessions: dict[str, dict[str, Any]] = {}
|
||
self.quota_overrides: dict[str, dict[str, Any]] = {}
|
||
self.usage: dict[tuple[str, str], int] = {}
|
||
self.estimates: dict[str, dict[str, Any]] = {} # estimate_id -> result fields
|
||
self.events: list[dict[str, Any]] = [] # user_events rows (history source)
|
||
self._next_id = 1
|
||
self.query_count = 0 # db.execute() calls — N+1 regression guard (review PR #2563)
|
||
|
||
def add_user(
|
||
self,
|
||
username: str,
|
||
password_hash: str | None,
|
||
*,
|
||
role: str = "employee",
|
||
manager_id: int | None = None,
|
||
is_active: bool = True,
|
||
display_name: str | None = None,
|
||
org_name: str | None = None,
|
||
email: str | None = None,
|
||
created_at: datetime | None = None,
|
||
) -> int:
|
||
uid = self._next_id
|
||
self._next_id += 1
|
||
self.users[username] = {
|
||
"id": uid,
|
||
"username": username,
|
||
"password_hash": password_hash,
|
||
"role": role,
|
||
"manager_id": manager_id,
|
||
"display_name": display_name,
|
||
"org_name": org_name,
|
||
"email": email,
|
||
"is_active": is_active,
|
||
"created_at": created_at or datetime.now(UTC),
|
||
}
|
||
return uid
|
||
|
||
def user_by_id(self, uid: int) -> dict[str, Any] | None:
|
||
for u in self.users.values():
|
||
if u["id"] == uid:
|
||
return u
|
||
return None
|
||
|
||
def add_estimate_event(
|
||
self,
|
||
username: str,
|
||
*,
|
||
address: str | None = "ул. Ленина, 1",
|
||
area_m2: str | None = "45",
|
||
rooms: str | None = "2",
|
||
estimate_id: str | None = None,
|
||
median_price: int | None = None,
|
||
confidence: str | None = None,
|
||
n_analogs: int | None = None,
|
||
created_at: datetime | None = None,
|
||
) -> None:
|
||
eid = estimate_id or str(uuid4())
|
||
self.events.append(
|
||
{
|
||
"username": username,
|
||
"event_type": "estimate_request",
|
||
"estimate_id": eid,
|
||
"payload": {"address": address, "area_m2": area_m2, "rooms": rooms},
|
||
"created_at": created_at or datetime.now(UTC),
|
||
}
|
||
)
|
||
if median_price is not None or confidence is not None or n_analogs is not None:
|
||
self.estimates[eid] = {
|
||
"median_price": median_price,
|
||
"confidence": confidence,
|
||
"n_analogs": n_analogs,
|
||
}
|
||
|
||
|
||
class _Result:
|
||
"""Minimal cursor-result stand-in: `.fetchone()`/`.fetchall()` (attribute-style
|
||
Row) AND `.mappings().fetchone()`/`.all()` (dict-style RowMapping) — enough
|
||
surface for the SQL used by app.api.v1.team / app.api.v1.auth /
|
||
app.services.auth_session / app.services.account_quota."""
|
||
|
||
def __init__(self, rows: list[dict[str, Any]]) -> None:
|
||
self._rows = rows
|
||
|
||
def fetchone(self) -> SimpleNamespace | None:
|
||
return SimpleNamespace(**self._rows[0]) if self._rows else None
|
||
|
||
def fetchall(self) -> list[SimpleNamespace]:
|
||
return [SimpleNamespace(**r) for r in self._rows]
|
||
|
||
def mappings(self) -> _Mappings:
|
||
return _Mappings(self._rows)
|
||
|
||
|
||
class _Mappings:
|
||
def __init__(self, rows: list[dict[str, Any]]) -> None:
|
||
self._rows = rows
|
||
|
||
def fetchone(self) -> dict[str, Any] | None:
|
||
return dict(self._rows[0]) if self._rows else None
|
||
|
||
def all(self) -> list[dict[str, Any]]:
|
||
return [dict(r) for r in self._rows]
|
||
|
||
|
||
class _FakeDB:
|
||
"""Minimal Session stand-in dispatching on SQL text — see module docstring."""
|
||
|
||
def __init__(self, store: _Store) -> None:
|
||
self.store = store
|
||
|
||
def __enter__(self) -> _FakeDB:
|
||
return self
|
||
|
||
def __exit__(self, *exc: object) -> bool:
|
||
return False
|
||
|
||
def close(self) -> None:
|
||
pass
|
||
|
||
def commit(self) -> None:
|
||
pass
|
||
|
||
def rollback(self) -> None:
|
||
pass
|
||
|
||
def execute(self, stmt: object, params: dict[str, Any] | None = None) -> _Result:
|
||
sql = str(stmt)
|
||
p = params or {}
|
||
s = self.store
|
||
s.query_count += 1
|
||
|
||
# ---- tradein_sessions ----
|
||
if "INSERT INTO tradein_sessions" in sql:
|
||
now = datetime.now(UTC)
|
||
s.sessions[p["token"]] = {
|
||
"user_id": p["user_id"],
|
||
"expires_at": now + timedelta(hours=p["ttl_hours"]),
|
||
"last_seen_at": now,
|
||
}
|
||
return _Result([])
|
||
|
||
if "UPDATE tradein_sessions" in sql and "SET last_seen_at" in sql:
|
||
sess = s.sessions.get(p["token"])
|
||
if sess is not None:
|
||
now = datetime.now(UTC)
|
||
sess["last_seen_at"] = now
|
||
sess["expires_at"] = now + timedelta(hours=p["ttl_hours"])
|
||
return _Result([])
|
||
|
||
if "DELETE FROM tradein_sessions WHERE token" in sql:
|
||
s.sessions.pop(p["token"], None)
|
||
return _Result([])
|
||
|
||
if "DELETE FROM tradein_sessions WHERE user_id" in sql:
|
||
uid = p["user_id"]
|
||
for tok in [t for t, sess in s.sessions.items() if sess["user_id"] == uid]:
|
||
del s.sessions[tok]
|
||
return _Result([])
|
||
|
||
if "FROM tradein_sessions s" in sql and "JOIN tradein_users u" in sql:
|
||
sess = s.sessions.get(p["token"])
|
||
if sess is None:
|
||
return _Result([])
|
||
user = s.user_by_id(sess["user_id"])
|
||
if user is None:
|
||
return _Result([])
|
||
return _Result(
|
||
[
|
||
{
|
||
"user_id": sess["user_id"],
|
||
"expires_at": sess["expires_at"],
|
||
"last_seen_at": sess["last_seen_at"],
|
||
"username": user["username"],
|
||
"role": user["role"],
|
||
"display_name": user["display_name"],
|
||
"org_name": user["org_name"],
|
||
"email": user["email"],
|
||
"is_active": user["is_active"],
|
||
}
|
||
]
|
||
)
|
||
|
||
# ---- tradein_users: login lookup (get_user_by_username) ----
|
||
if "password_hash, role, is_active" in sql and "FROM tradein_users" in sql:
|
||
user = s.users.get(p["username"])
|
||
return _Result([user] if user is not None else [])
|
||
|
||
# ---- tradein_users: create ----
|
||
if "INSERT INTO tradein_users" in sql:
|
||
uid = s._next_id
|
||
s._next_id += 1
|
||
created_at = datetime.now(UTC)
|
||
row = {
|
||
"id": uid,
|
||
"username": p["username"],
|
||
"password_hash": p["password_hash"],
|
||
"role": "employee",
|
||
"manager_id": p["manager_id"],
|
||
"display_name": p["display_name"],
|
||
"org_name": p["org_name"],
|
||
"email": p["email"],
|
||
"is_active": True,
|
||
"created_at": created_at,
|
||
}
|
||
s.users[p["username"]] = row
|
||
return _Result([dict(row)])
|
||
|
||
# ---- tradein_users: manager_id validation ----
|
||
if "role = 'manager'" in sql:
|
||
user = s.user_by_id(p["id"])
|
||
match = user is not None and user["role"] == "manager"
|
||
return _Result([{"id": user["id"]}] if match else [])
|
||
|
||
# ---- tradein_users: list employees (has explicit ORDER BY) ----
|
||
if "role = 'employee'" in sql and "ORDER BY created_at DESC" in sql:
|
||
rows = [u for u in s.users.values() if u["role"] == "employee"]
|
||
if "manager_id" in p:
|
||
rows = [u for u in rows if u["manager_id"] == p["manager_id"]]
|
||
# Mirrors real SQL `ORDER BY created_at DESC, id DESC` — `id` tiebreak
|
||
# is REQUIRED for deterministic paging when created_at ties (follow-up
|
||
# review PR #2563 п.1, bulk-seed #2557 inserts many rows in one tx).
|
||
rows = sorted(rows, key=lambda u: (u["created_at"], u["id"]), reverse=True)
|
||
offset, limit = p.get("offset", 0), p.get("limit", len(rows))
|
||
rows = rows[offset : offset + limit]
|
||
return _Result(
|
||
[
|
||
{
|
||
"id": u["id"],
|
||
"username": u["username"],
|
||
"display_name": u["display_name"],
|
||
"org_name": u["org_name"],
|
||
"email": u["email"],
|
||
"is_active": u["is_active"],
|
||
"manager_id": u["manager_id"],
|
||
"created_at": u["created_at"],
|
||
}
|
||
for u in rows
|
||
]
|
||
)
|
||
|
||
# ---- tradein_users: fetch single employee by id ----
|
||
if "role = 'employee'" in sql:
|
||
user = s.user_by_id(p["id"])
|
||
if user is None or user["role"] != "employee":
|
||
return _Result([])
|
||
return _Result(
|
||
[
|
||
{
|
||
"id": user["id"],
|
||
"username": user["username"],
|
||
"display_name": user["display_name"],
|
||
"org_name": user["org_name"],
|
||
"email": user["email"],
|
||
"is_active": user["is_active"],
|
||
"manager_id": user["manager_id"],
|
||
"created_at": user["created_at"],
|
||
}
|
||
]
|
||
)
|
||
|
||
# ---- tradein_users: uniqueness pre-check ----
|
||
if sql.strip().startswith("SELECT id FROM tradein_users WHERE username"):
|
||
user = s.users.get(p["u"])
|
||
return _Result([{"id": user["id"]}] if user is not None else [])
|
||
|
||
# ---- tradein_users: update (PATCH) ----
|
||
if "UPDATE tradein_users" in sql and "SET display_name = COALESCE" in sql:
|
||
user = s.user_by_id(p["id"])
|
||
assert user is not None
|
||
if p.get("display_name") is not None:
|
||
user["display_name"] = p["display_name"]
|
||
if p.get("org_name") is not None:
|
||
user["org_name"] = p["org_name"]
|
||
if p.get("email") is not None:
|
||
user["email"] = p["email"]
|
||
if p.get("is_active") is not None:
|
||
user["is_active"] = p["is_active"]
|
||
if p.get("password_hash") is not None:
|
||
user["password_hash"] = p["password_hash"]
|
||
return _Result([])
|
||
|
||
# ---- account_quota_overrides upsert: unlimited always reset to False,
|
||
# note preserved (COALESCE) if a row already existed — mirrors real SQL.
|
||
if "INSERT INTO account_quota_overrides" in sql:
|
||
existing_override = s.quota_overrides.get(p["username"])
|
||
preserved_note = (
|
||
existing_override["note"] if existing_override is not None else None
|
||
) or p["note"]
|
||
s.quota_overrides[p["username"]] = {
|
||
"monthly_limit": p["monthly_limit"],
|
||
"unlimited": False,
|
||
"note": preserved_note,
|
||
}
|
||
return _Result([])
|
||
|
||
# ---- account_quota_overrides: batch (list_employees) ----
|
||
if "SELECT username, monthly_limit, unlimited" in sql:
|
||
rows = [
|
||
{"username": u, "monthly_limit": ov["monthly_limit"], "unlimited": ov["unlimited"]}
|
||
for u, ov in s.quota_overrides.items()
|
||
if u in p["usernames"]
|
||
]
|
||
return _Result(rows)
|
||
|
||
if "SELECT unlimited FROM account_quota_overrides" in sql:
|
||
override = s.quota_overrides.get(p["u"])
|
||
return _Result([{"unlimited": override["unlimited"]}] if override else [])
|
||
|
||
if "SELECT monthly_limit FROM account_quota_overrides" in sql:
|
||
override = s.quota_overrides.get(p["u"])
|
||
return _Result([{"monthly_limit": override["monthly_limit"]}] if override else [])
|
||
|
||
# ---- account_estimate_usage: batch (list_employees) ----
|
||
if "SELECT username, used" in sql:
|
||
rows = [
|
||
{"username": u, "used": used}
|
||
for (u, period), used in s.usage.items()
|
||
if u in p["usernames"] and period == p["period"]
|
||
]
|
||
return _Result(rows)
|
||
|
||
# ---- account_estimate_usage: single (account_quota.get_status) ----
|
||
if "SELECT used FROM account_estimate_usage" in sql:
|
||
used = s.usage.get((p["u"], p["p"]))
|
||
return _Result([{"used": used}] if used is not None else [])
|
||
|
||
# ---- user_events + trade_in_estimates (history) ----
|
||
if "FROM user_events ue" in sql:
|
||
matches = [
|
||
e
|
||
for e in s.events
|
||
if e["username"] == p["username"] and e["event_type"] == "estimate_request"
|
||
]
|
||
matches.sort(key=lambda e: e["created_at"], reverse=True)
|
||
page = matches[p["offset"] : p["offset"] + p["limit"]]
|
||
rows = []
|
||
for e in page:
|
||
extra = s.estimates.get(e["estimate_id"], {})
|
||
rows.append(
|
||
{
|
||
"estimate_id": e["estimate_id"],
|
||
"address": e["payload"].get("address"),
|
||
"area_m2": e["payload"].get("area_m2"),
|
||
"rooms": e["payload"].get("rooms"),
|
||
"median_price": extra.get("median_price"),
|
||
"confidence": extra.get("confidence"),
|
||
"n_analogs": extra.get("n_analogs"),
|
||
"created_at": e["created_at"],
|
||
}
|
||
)
|
||
return _Result(rows)
|
||
|
||
raise AssertionError(f"unhandled fake SQL in test_team_api: {sql!r}")
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# Test app
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def _build_test_app(store: _Store) -> FastAPI:
|
||
app = FastAPI()
|
||
app.middleware("http")(rbac_guard)
|
||
app.include_router(auth_router.router, prefix="/api/v1/auth", tags=["auth"])
|
||
app.include_router(team_router.router, prefix="/api/v1/team", tags=["team"])
|
||
|
||
def _override_get_db(): # generator dependency — matches app.core.db.get_db shape
|
||
yield _FakeDB(store)
|
||
|
||
app.dependency_overrides[get_db] = _override_get_db
|
||
return app
|
||
|
||
|
||
@pytest.fixture(autouse=True)
|
||
def _reset_state(monkeypatch: pytest.MonkeyPatch) -> None:
|
||
auth_mod.reset_cache_for_tests()
|
||
auth_router._LOGIN_LIMITER._hits.clear()
|
||
monkeypatch.setattr(config.settings, "auth_mode", "dual")
|
||
# team.py / auth.py events go through schedule_event (own SessionLocal(), fire-
|
||
# and-forget) — captured into a list instead of hitting a real DB.
|
||
monkeypatch.setattr(team_router, "schedule_event", lambda **kw: _EVENTS.append(kw))
|
||
monkeypatch.setattr(auth_router, "schedule_event", lambda **kw: None)
|
||
_EVENTS.clear()
|
||
|
||
|
||
_EVENTS: list[dict[str, Any]] = []
|
||
|
||
|
||
@pytest.fixture
|
||
def store() -> _Store:
|
||
return _Store()
|
||
|
||
|
||
@pytest.fixture
|
||
def client(store: _Store, monkeypatch: pytest.MonkeyPatch) -> TestClient:
|
||
monkeypatch.setattr("app.core.rbac.SessionLocal", lambda: _FakeDB(store))
|
||
# base_url=https:// — login sets a Secure cookie; see test_auth_api.py for why
|
||
# a plain-http TestClient would silently drop it.
|
||
return TestClient(_build_test_app(store), base_url="https://testserver")
|
||
|
||
|
||
def _login(client: TestClient, username: str, password: str) -> None:
|
||
resp = client.post("/api/v1/auth/login", json={"username": username, "password": password})
|
||
assert resp.status_code == 200, resp.text
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# POST /employees — happy path + validation
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_manager_creates_employee_forces_own_manager_id(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_alice", "password": "Secret123!", "display_name": "Алиса"},
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
body = resp.json()
|
||
assert body["username"] == "emp_alice"
|
||
assert body["manager_id"] == mgr_id
|
||
assert body["quota"]["limit"] > 0
|
||
assert store.users["emp_alice"]["password_hash"] is not None
|
||
assert any(e["event_type"] == "employee_created" for e in _EVENTS)
|
||
created_event = next(e for e in _EVENTS if e["event_type"] == "employee_created")
|
||
# Пароль никогда не попадает в аудит-событие.
|
||
assert "Secret123!" not in str(created_event)
|
||
|
||
|
||
def test_admin_creates_employee_with_explicit_manager_id(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
_login(client, "admin1", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_bob", "password": "Secret123!", "manager_id": mgr_id},
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
assert resp.json()["manager_id"] == mgr_id
|
||
|
||
|
||
def test_admin_creates_employee_without_manager_id(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
_login(client, "admin1", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees", json={"username": "emp_free", "password": "Secret123!"}
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
assert resp.json()["manager_id"] is None
|
||
|
||
|
||
def test_admin_create_employee_invalid_manager_id_422(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
_login(client, "admin1", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_x", "password": "Secret123!", "manager_id": 999},
|
||
)
|
||
assert resp.status_code == 422
|
||
|
||
|
||
def test_create_employee_non_ascii_username_422(client: TestClient, store: _Store) -> None:
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "сотрудник", "password": "Secret123!"},
|
||
)
|
||
assert resp.status_code == 422
|
||
|
||
|
||
@pytest.mark.parametrize("username", ["admin\n", "user1\n"])
|
||
def test_create_employee_trailing_newline_username_422_not_500(
|
||
client: TestClient, store: _Store, username: str
|
||
) -> None:
|
||
"""Deep-review seed #2564: Python `$` matches BEFORE a trailing newline
|
||
(`re.match(r'...\\$', 'admin\\n')` → True), but Postgres `~` (CHECK
|
||
tradein_users_username_ascii_ck, migration 193) does NOT — a username with a
|
||
trailing "\\n" used to pass Pydantic validation and crash in the DB (500)
|
||
instead of a clean 422. `_USERNAME_RE` now uses `\\Z`, matching Postgres `~`
|
||
semantics exactly."""
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": username, "password": "Secret123!"},
|
||
)
|
||
assert resp.status_code == 422, resp.text
|
||
|
||
|
||
def test_create_employee_duplicate_username_409(client: TestClient, store: _Store) -> None:
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
store.add_user("emp_dup", hash_password("Secret123!"), role="employee")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_dup", "password": "Secret123!"},
|
||
)
|
||
assert resp.status_code == 409
|
||
|
||
|
||
def test_create_employee_no_session_401(client: TestClient) -> None:
|
||
resp = client.post(
|
||
"/api/v1/team/employees", json={"username": "emp_x", "password": "Secret123!"}
|
||
)
|
||
assert resp.status_code == 401
|
||
|
||
|
||
def test_create_employee_employee_role_403(client: TestClient, store: _Store) -> None:
|
||
store.add_user("emp_a", hash_password("Secret123!"), role="employee")
|
||
_login(client, "emp_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees", json={"username": "emp_x", "password": "Secret123!"}
|
||
)
|
||
assert resp.status_code == 403
|
||
|
||
|
||
def test_employee_role_403_on_all_team_routes(client: TestClient, store: _Store) -> None:
|
||
store.add_user("emp_a", hash_password("Secret123!"), role="employee")
|
||
_login(client, "emp_a", "Secret123!")
|
||
|
||
assert client.get("/api/v1/team/employees").status_code == 403
|
||
assert client.patch("/api/v1/team/employees/1", json={}).status_code == 403
|
||
assert client.get("/api/v1/team/employees/1/history").status_code == 403
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# Org isolation — manager A vs manager B
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_manager_a_cannot_see_manager_b_employee(client: TestClient, store: _Store) -> None:
|
||
mgr_a = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
mgr_b = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
store.add_user("emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_a)
|
||
store.add_user("emp_b", hash_password("Secret123!"), role="employee", manager_id=mgr_b)
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.get("/api/v1/team/employees")
|
||
assert resp.status_code == 200
|
||
usernames = {e["username"] for e in resp.json()}
|
||
assert usernames == {"emp_a"}
|
||
|
||
|
||
def test_manager_a_patch_manager_b_employee_404(client: TestClient, store: _Store) -> None:
|
||
mgr_a = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
mgr_b = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
emp_b_id = store.add_user(
|
||
"emp_b", hash_password("Secret123!"), role="employee", manager_id=mgr_b
|
||
)
|
||
assert mgr_a # used only to seed manager_id != mgr_b
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_b_id}", json={"display_name": "hacked"})
|
||
assert resp.status_code == 404
|
||
|
||
|
||
def test_manager_a_history_manager_b_employee_404(client: TestClient, store: _Store) -> None:
|
||
mgr_a = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
mgr_b = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
emp_b_id = store.add_user(
|
||
"emp_b", hash_password("Secret123!"), role="employee", manager_id=mgr_b
|
||
)
|
||
assert mgr_a
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.get(f"/api/v1/team/employees/{emp_b_id}/history")
|
||
assert resp.status_code == 404
|
||
|
||
|
||
def test_manager_post_with_foreign_manager_id_creates_under_self(
|
||
client: TestClient, store: _Store
|
||
) -> None:
|
||
"""POST с чужим manager_id в теле от лица manager — ИГНОРИРУЕТСЯ, сотрудник
|
||
создаётся под ЕГО собственным manager_id, не под указанным чужим."""
|
||
mgr_a_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
mgr_b_id = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
assert mgr_a_id != mgr_b_id
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={
|
||
"username": "emp_spoof",
|
||
"password": "Secret123!",
|
||
"manager_id": mgr_b_id,
|
||
},
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
assert resp.json()["manager_id"] == mgr_a_id
|
||
assert store.users["emp_spoof"]["manager_id"] == mgr_a_id
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# PATCH /employees/{id} — block/unblock/quota/profile/password
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_block_employee_revokes_sessions(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("EmpSecret1!"), role="employee", manager_id=mgr_id
|
||
)
|
||
|
||
# Employee logs in first — real session created via the real auth flow.
|
||
emp_client = TestClient(client.app, base_url="https://testserver")
|
||
_login(emp_client, "emp_a", "EmpSecret1!")
|
||
assert any(sess["user_id"] == emp_id for sess in store.sessions.values())
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"is_active": False})
|
||
assert resp.status_code == 200, resp.text
|
||
assert resp.json()["is_active"] is False
|
||
|
||
# Blocking must have revoked ALL of emp_a's sessions.
|
||
assert not any(sess["user_id"] == emp_id for sess in store.sessions.values())
|
||
assert any(e["event_type"] == "employee_blocked" for e in _EVENTS)
|
||
|
||
|
||
def test_reset_password_revokes_old_sessions(client: TestClient, store: _Store) -> None:
|
||
"""HIGH (deep-review PR #2563): смена пароля обязана ревокать ВСЕ существующие
|
||
сессии сотрудника — иначе скомпрометированная/чужая сессия переживает reset
|
||
(sliding-refresh в auth_session.py продлевает её бесконечно, а не «до TTL»)."""
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("OldSecret1!"), role="employee", manager_id=mgr_id
|
||
)
|
||
|
||
emp_client = TestClient(client.app, base_url="https://testserver")
|
||
_login(emp_client, "emp_a", "OldSecret1!")
|
||
assert any(sess["user_id"] == emp_id for sess in store.sessions.values())
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"new_password": "NewSecret2!"})
|
||
assert resp.status_code == 200, resp.text
|
||
|
||
# Password reset must have revoked ALL of emp_a's pre-existing sessions —
|
||
# not just when is_active is explicitly set to False.
|
||
assert not any(sess["user_id"] == emp_id for sess in store.sessions.values())
|
||
|
||
|
||
def test_unblock_employee_event(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id, is_active=False
|
||
)
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"is_active": True})
|
||
assert resp.status_code == 200
|
||
assert resp.json()["is_active"] is True
|
||
assert any(e["event_type"] == "employee_unblocked" for e in _EVENTS)
|
||
|
||
|
||
def test_patch_monthly_limit_reflected_in_quota_status(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"monthly_limit": 30})
|
||
assert resp.status_code == 200, resp.text
|
||
assert resp.json()["quota"]["limit"] == 30
|
||
assert any(e["event_type"] == "quota_changed" for e in _EVENTS)
|
||
|
||
listing = client.get("/api/v1/team/employees")
|
||
assert listing.status_code == 200
|
||
entry = next(e for e in listing.json() if e["id"] == emp_id)
|
||
assert entry["quota"]["limit"] == 30
|
||
|
||
|
||
def test_patch_reset_password_no_password_in_events(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("OldSecret1!"), role="employee", manager_id=mgr_id
|
||
)
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"new_password": "NewSecret2!"})
|
||
assert resp.status_code == 200, resp.text
|
||
assert any(e["event_type"] == "employee_password_reset" for e in _EVENTS)
|
||
assert "NewSecret2!" not in str(_EVENTS)
|
||
|
||
emp_client = TestClient(client.app, base_url="https://testserver")
|
||
login_resp = emp_client.post(
|
||
"/api/v1/auth/login", json={"username": "emp_a", "password": "NewSecret2!"}
|
||
)
|
||
assert login_resp.status_code == 200
|
||
|
||
|
||
def test_patch_no_session_401(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"is_active": False})
|
||
assert resp.status_code == 401
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# GET /employees — list + admin filter
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_admin_sees_all_employees_and_filters_by_manager(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_a = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
mgr_b = store.add_user("mgr_b", hash_password("Secret123!"), role="manager")
|
||
store.add_user("emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_a)
|
||
store.add_user("emp_b", hash_password("Secret123!"), role="employee", manager_id=mgr_b)
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
resp_all = client.get("/api/v1/team/employees")
|
||
assert resp_all.status_code == 200
|
||
assert {e["username"] for e in resp_all.json()} == {"emp_a", "emp_b"}
|
||
|
||
resp_filtered = client.get("/api/v1/team/employees", params={"manager_id": mgr_a})
|
||
assert resp_filtered.status_code == 200
|
||
assert {e["username"] for e in resp_filtered.json()} == {"emp_a"}
|
||
|
||
|
||
def test_list_employees_query_count_is_not_n_plus_1(client: TestClient, store: _Store) -> None:
|
||
"""Medium2 (deep-review PR #2563): pre-fix measured 2N+3 = 23 SQL queries for
|
||
N=10 employees (per-employee `account_quota.get_status`). Batch quota lookup
|
||
(`_batch_quota_status`) must keep the query count constant regardless of N."""
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
for i in range(10):
|
||
store.add_user(f"emp_{i}", hash_password("Secret123!"), role="employee", manager_id=mgr_id)
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
store.query_count = 0 # reset after login's own DB traffic
|
||
resp = client.get("/api/v1/team/employees")
|
||
assert resp.status_code == 200
|
||
assert len(resp.json()) == 10
|
||
# rbac_guard session lookup (1) + current_team_actor session lookup (1) +
|
||
# list query (1) + 2 batch quota queries = 5, flat regardless of N.
|
||
assert store.query_count <= 5, f"expected O(1) queries for N=10, got {store.query_count}"
|
||
|
||
|
||
def test_list_employees_pagination(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
for i in range(5):
|
||
store.add_user(f"emp_{i}", hash_password("Secret123!"), role="employee", manager_id=mgr_id)
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
resp = client.get("/api/v1/team/employees", params={"limit": 2, "offset": 1})
|
||
assert resp.status_code == 200
|
||
assert len(resp.json()) == 2
|
||
|
||
|
||
def test_list_employees_limit_max_200(client: TestClient, store: _Store) -> None:
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
_login(client, "admin1", "Secret123!")
|
||
resp = client.get("/api/v1/team/employees", params={"limit": 500})
|
||
assert resp.status_code == 422
|
||
|
||
|
||
def test_list_employees_pagination_stable_with_identical_created_at(
|
||
client: TestClient, store: _Store
|
||
) -> None:
|
||
"""Follow-up review PR #2563 п.1: `created_at DEFAULT now()` — время ТРАНЗАКЦИИ,
|
||
bulk-seed (#2557) вставляет много юзеров одной транзакцией → идентичный
|
||
timestamp у N+ строк. Без `id DESC` тай-брейкера порядок между страницами
|
||
на PostgreSQL для строк-«близнецов» не гарантирован — сотрудники пропадали/
|
||
дублировались бы при постраничном листании. Вставляем 5 сотрудников с
|
||
ОДИНАКОВЫМ created_at, листаем limit=2 постранично — объединение страниц
|
||
обязано дать полный набор без дублей и пропусков."""
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
same_ts = datetime.now(UTC)
|
||
expected_usernames = set()
|
||
for i in range(5):
|
||
username = f"emp_tie_{i}"
|
||
store.add_user(
|
||
username,
|
||
hash_password("Secret123!"),
|
||
role="employee",
|
||
manager_id=mgr_id,
|
||
created_at=same_ts,
|
||
)
|
||
expected_usernames.add(username)
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
|
||
seen: list[str] = []
|
||
offset = 0
|
||
while True:
|
||
resp = client.get("/api/v1/team/employees", params={"limit": 2, "offset": offset})
|
||
assert resp.status_code == 200, resp.text
|
||
page = [e["username"] for e in resp.json()]
|
||
if not page:
|
||
break
|
||
seen.extend(page)
|
||
offset += 2
|
||
|
||
assert len(seen) == len(expected_usernames), (
|
||
f"page union has {len(seen)} entries (dupes or gaps), expected "
|
||
f"{len(expected_usernames)}: {seen}"
|
||
)
|
||
assert set(seen) == expected_usernames
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# CSRF defense-in-depth — Origin/Referer check on state-changing team routes
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_create_employee_origin_mismatch_403(client: TestClient, store: _Store) -> None:
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_evil", "password": "Secret123!"},
|
||
headers={"Origin": "https://evil.example"},
|
||
)
|
||
assert resp.status_code == 403
|
||
assert "emp_evil" not in store.users
|
||
|
||
|
||
def test_create_employee_origin_allowed(client: TestClient, store: _Store) -> None:
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
allowed_origin = config.settings.cors_origins[0]
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_ok", "password": "Secret123!"},
|
||
headers={"Origin": allowed_origin},
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
|
||
|
||
def test_create_employee_no_origin_no_referer_allowed(client: TestClient, store: _Store) -> None:
|
||
"""curl-смоук внутри контейнера не шлёт ни Origin, ни Referer — не должен ломаться."""
|
||
store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.post(
|
||
"/api/v1/team/employees",
|
||
json={"username": "emp_curl", "password": "Secret123!"},
|
||
)
|
||
assert resp.status_code == 201, resp.text
|
||
|
||
|
||
def test_patch_employee_origin_mismatch_403(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.patch(
|
||
f"/api/v1/team/employees/{emp_id}",
|
||
json={"display_name": "hacked"},
|
||
headers={"Referer": "https://evil.example/csrf.html"},
|
||
)
|
||
assert resp.status_code == 403
|
||
assert store.users["emp_a"]["display_name"] != "hacked"
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# _batch_quota_status unlimited semantics — must match account_quota.is_unlimited
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_batch_quota_unlimited_ignored_for_non_roles_yaml_username(
|
||
client: TestClient, store: _Store
|
||
) -> None:
|
||
"""Follow-up review PR #2563 п.2: `account_quota.is_unlimited` short-circuits
|
||
to False for a username NOT in roles.yaml — it never even reads
|
||
`account_quota_overrides.unlimited`. The batch quota status used by
|
||
GET /employees must agree, or the list would show "unlimited" for a quota
|
||
that real enforcement (check_and_raise/increment, same is_unlimited) does
|
||
NOT honor — a misleading display. `emp_ghost_unlimited` is a fresh DB-only
|
||
username guaranteed absent from roles.yaml."""
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
store.add_user(
|
||
"emp_ghost_unlimited", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
store.quota_overrides["emp_ghost_unlimited"] = {
|
||
"monthly_limit": 15,
|
||
"unlimited": True,
|
||
"note": "manual grant via SQL runbook (not through team-api)",
|
||
}
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
resp = client.get("/api/v1/team/employees")
|
||
assert resp.status_code == 200, resp.text
|
||
entry = next(e for e in resp.json() if e["username"] == "emp_ghost_unlimited")
|
||
# DB override says unlimited=true, but username is NOT in roles.yaml — real
|
||
# enforcement would never see it, so the list must NOT claim "unlimited".
|
||
assert entry["quota"]["unlimited"] is False
|
||
assert entry["quota"]["limit"] == 15
|
||
|
||
|
||
def test_batch_quota_unlimited_honored_for_roles_yaml_username(
|
||
client: TestClient, store: _Store
|
||
) -> None:
|
||
"""Symmetric positive case: a username actually present in roles.yaml
|
||
(non-admin role) — `account_quota_overrides.unlimited=true` IS honored, same
|
||
as `account_quota.is_unlimited`. Uses `kopylov` — real prod pilot-role entry
|
||
in auth/roles.yaml (see app/core/auth.py module docstring)."""
|
||
store.add_user("admin1", hash_password("Secret123!"), role="admin")
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
store.add_user("kopylov", hash_password("Secret123!"), role="employee", manager_id=mgr_id)
|
||
store.quota_overrides["kopylov"] = {
|
||
"monthly_limit": 999,
|
||
"unlimited": True,
|
||
"note": "existing prod grant",
|
||
}
|
||
|
||
_login(client, "admin1", "Secret123!")
|
||
resp = client.get("/api/v1/team/employees")
|
||
assert resp.status_code == 200, resp.text
|
||
entry = next(e for e in resp.json() if e["username"] == "kopylov")
|
||
assert entry["quota"]["unlimited"] is True
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# GET /employees/{id}/history
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
def test_employee_history_happy_path(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
now = datetime.now(UTC)
|
||
store.add_estimate_event(
|
||
"emp_a",
|
||
address="ул. Малышева, 10",
|
||
median_price=6_500_000,
|
||
confidence="high",
|
||
n_analogs=12,
|
||
created_at=now - timedelta(minutes=5),
|
||
)
|
||
store.add_estimate_event("emp_a", address="ул. Мамина-Сибиряка, 5", created_at=now)
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.get(f"/api/v1/team/employees/{emp_id}/history")
|
||
assert resp.status_code == 200, resp.text
|
||
body = resp.json()
|
||
assert len(body) == 2
|
||
assert body[0]["address"] == "ул. Мамина-Сибиряка, 5" # most recent first
|
||
priced = next(e for e in body if e["address"] == "ул. Малышева, 10")
|
||
assert priced["median_price"] == 6_500_000
|
||
assert priced["confidence"] == "high"
|
||
|
||
|
||
def test_employee_history_pagination(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
for i in range(5):
|
||
store.add_estimate_event("emp_a", address=f"адрес-{i}")
|
||
|
||
_login(client, "mgr_a", "Secret123!")
|
||
resp = client.get(f"/api/v1/team/employees/{emp_id}/history", params={"limit": 2, "offset": 1})
|
||
assert resp.status_code == 200
|
||
assert len(resp.json()) == 2
|
||
|
||
|
||
def test_employee_history_limit_max_200(client: TestClient, store: _Store) -> None:
|
||
mgr_id = store.add_user("mgr_a", hash_password("Secret123!"), role="manager")
|
||
emp_id = store.add_user(
|
||
"emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id
|
||
)
|
||
_login(client, "mgr_a", "Secret123!")
|
||
|
||
resp = client.get(f"/api/v1/team/employees/{emp_id}/history", params={"limit": 500})
|
||
assert resp.status_code == 422
|