diff --git a/tradein-mvp/backend/app/api/v1/auth.py b/tradein-mvp/backend/app/api/v1/auth.py index 9933bc2e..a839d936 100644 --- a/tradein-mvp/backend/app/api/v1/auth.py +++ b/tradein-mvp/backend/app/api/v1/auth.py @@ -6,9 +6,14 @@ это `/trade-in/api/v1/auth/*` снаружи. Security: - - Неверные creds (неизвестный username / неактивен / password_hash NULL / + - Неверные creds (неизвестный username / доступ закрыт / password_hash NULL / неверный пароль) → ОДИНАКОВЫЙ 401 с generic сообщением — не раскрываем, существует ли username (user-enumeration защита). + - Состояние доступа проверяется ТОЛЬКО ПОСЛЕ проверки пароля, и осмысленный + ответ (403 «пробный доступ закончился») получает исключительно тот, кто + пароль уже доказал. Ветвление ДО пароля превратило бы отдельный статус в + оракул существования логина: перебором можно было бы перечислить аккаунты, + не зная ни одного пароля (миграция data/sql/auth/004, WHY-2). - #2552 post-review Medium 2: `verify_password` ВСЕГДА вызывается ровно один раз — для несуществующего username / NULL password_hash сверяем против статичного dummy-хеша (`_DUMMY_PASSWORD_HASH`, сгенерирован один @@ -38,10 +43,10 @@ from pydantic import BaseModel from sqlalchemy.orm import Session from app.core.config import settings -from app.core.db import get_db from app.core.password import hash_password, verify_password from app.core.ratelimit import SlidingWindowLimiter, _client_ip from app.services.auth_session import create_session, get_user_by_username, revoke_session +from app.services.identity_store import AccessState, get_identity_db from app.services.user_events import schedule_event logger = logging.getLogger(__name__) @@ -67,6 +72,16 @@ _DUMMY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16)) _INVALID_CREDENTIALS_DETAIL = "неверный логин или пароль" +# Единственный ответ логина, который НЕ generic 401: пароль верный, но пробный +# период истёк. `code` — машиночитаемый контракт для фронта (текст можно менять, +# ветку по нему — нет). Потребитель: `loginErrorMessage` в +# tradein-mvp/frontend/src/app/login/page.tsx — читает `detail.code` из +# `HTTPError.body` (frontend/src/lib/api.ts отдаёт тело ответа как есть) и +# показывает экран про пробный период вместо generic «Проверьте подключение». +# Меняешь значение здесь — меняй и там. +_ACCESS_EXPIRED_CODE = "access_expired" +_ACCESS_EXPIRED_MESSAGE = "Пробный доступ закончился" + class LoginRequest(BaseModel): username: str @@ -82,7 +97,7 @@ async def login( body: LoginRequest, request: Request, response: Response, - db: Annotated[Session, Depends(get_db)], + db: Annotated[Session, Depends(get_identity_db)], ) -> LoginResponse: ip = _client_ip(request) user_agent = request.headers.get("user-agent") @@ -105,9 +120,44 @@ async def login( # ВСЕГДА вызывается — dummy-хеш при отсутствующем юзере/NULL password_hash # держит время ответа одинаковым независимо от существования аккаунта. password_ok = verify_password(body.password, hash_to_check) - credentials_ok = user is not None and user["is_active"] and password_ok - if not credentials_ok: + # Пароль проверен ВЫШЕ и безусловно — только теперь смотрим на состояние + # доступа. Порядок несущий, а не стилистический: см. модульный docstring. + if user is None or not password_ok: + schedule_event( + event_type="login_failed", + username=body.username, + ip=ip, + user_agent=user_agent, + path="/api/v1/auth/login", + method="POST", + ) + raise HTTPException(status_code=401, detail=_INVALID_CREDENTIALS_DETAIL) + + access_state = user["access_state"] + if access_state is AccessState.TRIAL_EXPIRED: + # Пароль верный, сессия НЕ создаётся. Единственный не-generic ответ: + # аккаунт существует и владелец это уже доказал паролем, так что + # осмысленный текст ничего не раскрывает постороннему. + # В режиме identity_store="tradein" эта ветка недостижима: булев + # is_active даёт только active/disabled (identity_store.to_access_state). + schedule_event( + event_type="login_blocked_expired", + username=user["username"], + ip=ip, + user_agent=user_agent, + path="/api/v1/auth/login", + method="POST", + ) + raise HTTPException( + status_code=403, + detail={"code": _ACCESS_EXPIRED_CODE, "message": _ACCESS_EXPIRED_MESSAGE}, + ) + + if not access_state.can_sign_in: + # disabled (и любое нераспознанное состояние — to_access_state fail-closed) + # → ТОТ ЖЕ generic 401 и то же событие, что при неверном пароле: + # заблокированный аккаунт неотличим от несуществующего. schedule_event( event_type="login_failed", username=body.username, @@ -118,7 +168,6 @@ async def login( ) raise HTTPException(status_code=401, detail=_INVALID_CREDENTIALS_DETAIL) - assert user is not None # narrowed by credentials_ok above token = create_session(db, user_id=user["user_id"], ip=ip, user_agent=user_agent) response.set_cookie( @@ -147,7 +196,7 @@ async def login( async def logout( request: Request, response: Response, - db: Annotated[Session, Depends(get_db)], + db: Annotated[Session, Depends(get_identity_db)], ) -> dict[str, bool]: token = request.cookies.get(settings.session_cookie_name) if token: diff --git a/tradein-mvp/backend/app/api/v1/me.py b/tradein-mvp/backend/app/api/v1/me.py index f5e74b22..ef1dac95 100644 --- a/tradein-mvp/backend/app/api/v1/me.py +++ b/tradein-mvp/backend/app/api/v1/me.py @@ -9,10 +9,15 @@ Caddy basic_auth пропускает `X-Authenticated-User: ` чер кому что показывать. #2552: session-first. Валидная DB-session cookie (см. app.services.auth_session) -отдаёт scope из tradein_users (role/display_name/org/email) БЕЗ похода в +отдаёт scope из реестра людей (role/display_name/org/email) БЕЗ похода в roles.yaml. Без cookie (или невалидная/истёкшая) — legacy X-Authenticated-User путь, БЕЗ ИЗМЕНЕНИЙ (regression недопустим — существующие тесты держат его бит-в-бит). + +Сессия БД берётся у `identity_store.get_identity_db` (реестр), а не у +`app.core.db.get_db` (продуктовая БД): при `IDENTITY_STORE=auth` люди и сессии +живут в другой БД. В дефолтном режиме это ТОТ ЖЕ объект `Session`, что отдал бы +`get_db`, — поведение прода не меняется. """ from __future__ import annotations @@ -25,8 +30,8 @@ from sqlalchemy.orm import Session from app.core.auth import UserScope, get_user_scope from app.core.config import settings -from app.core.db import get_db from app.services.auth_session import get_db_role_scope, get_session_user +from app.services.identity_store import get_identity_db logger = logging.getLogger(__name__) @@ -36,14 +41,15 @@ router = APIRouter() @router.get("/me") async def me( request: Request, - db: Annotated[Session, Depends(get_db)], + db: Annotated[Session, Depends(get_identity_db)], x_authenticated_user: Annotated[str | None, Header(alias="X-Authenticated-User")] = None, ) -> UserScope | dict[str, Any]: """Return the current user's RBAC scope (role + allowed/deny paths). Return type is a union (не только `UserScope`) — `UserScope.role` — это `Literal["admin","pilot","analyst","expired"]` (legacy roles.yaml names), - а DB-роли (tradein_users.role) — `"admin"/"manager"/"employee"`. FastAPI + а DB-роли (реестр: tradein_users.role / auth.users.role) — + `"admin"/"manager"/"employee"`. FastAPI строит response-схему из return-аннотации; жёсткий `UserScope` завернул бы "employee"/"manager" в ResponseValidationError. Итоговая JSON-форма ОДИНАКОВАЯ (те же 8 ключей) для обеих веток. diff --git a/tradein-mvp/backend/app/api/v1/team.py b/tradein-mvp/backend/app/api/v1/team.py index 592f4f13..d42bd250 100644 --- a/tradein-mvp/backend/app/api/v1/team.py +++ b/tradein-mvp/backend/app/api/v1/team.py @@ -15,10 +15,35 @@ Mounted at `/api/v1/team`; через Caddy `uri strip_prefix /trade-in` это - Роль должна быть `admin` или `manager` — иначе 403. Org-изоляция (главный инвариант фичи): manager видит/меняет ТОЛЬКО своих -employee (`tradein_users.manager_id = actor.user_id`). Чужой/несуществующий +employee (`<реестр>.manager_id = actor.user_id`). Чужой/несуществующий employee_id → 404 (НЕ 403) — не подтверждаем/не опровергаем существование чужого сотрудника перед manager'ом. См. `_authorize_employee`. +ДВЕ СЕССИИ БД, и это не дублирование: + - `identity_db` (`Depends(get_identity_db)`) — реестр людей: строка сотрудника + и его сессии. При `IDENTITY_STORE=auth` это ДРУГАЯ БД (`auth`). + - `db` (`Depends(get_db)`) — продуктовые таблицы «Меры», которые в общий + реестр не переезжают: `account_quota_overrides`, `account_estimate_usage`, + `user_events`, `trade_in_estimates`. +В дефолтном режиме (`IDENTITY_STORE=tradein`) это ОДИН И ТОТ ЖЕ объект `Session` +(см. `identity_store.get_identity_db`), поэтому всё по-прежнему коммитится одной +транзакцией — прод не меняется. В режиме `auth` транзакции физически две: +порядок коммитов выбран так, чтобы при сбое второго коммита оставалось менее +вредное состояние (см. комментарии у `db.commit()`), а `db is not identity_db` — +рантайм-признак «БД разные». + +Гранты роли `auth_app` (data/sql/auth/004, Часть 4) этот роутер соблюдает без +обходов: он ПИШЕТ только `password_hash, display_name, org_name, email, +access_state, updated_at` (ровно column-level GRANT UPDATE), вставляет строку +целиком (табличный GRANT INSERT) и НИКОГДА не пишет `role`/`manager_id` +UPDATE'ом и не делает DELETE по `users`. + +DELETE по `sessions` реестра — штатный и грантом предусмотрен (data/sql/auth/002, +GRANT DELETE на sessions): блокировка и смена пароля обязаны рвать живые сессии +немедленно, это `revoke_user_sessions` из `app.services.auth_session`, вызываемый +из `update_employee`. То есть периметр DELETE у этого роутера — ровно `sessions` +и ничего больше; грант DELETE на sessions не лишний. + Кого именно можно менять через этот роутер (`_MANAGEABLE_ROLES_BY_ACTOR`): - actor manager → только `role='employee'` И только своих (как было). - actor admin → `role IN ('employee','manager')`. @@ -51,6 +76,7 @@ from sqlalchemy import text from sqlalchemy.engine import RowMapping from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session +from sqlalchemy.sql.elements import TextClause from app.core.auth import get_role from app.core.config import settings @@ -65,6 +91,14 @@ from app.schemas.team import ( ) from app.services import account_quota from app.services.auth_session import get_session_user, revoke_user_sessions +from app.services.identity_store import ( + AccessState, + IdentitySchema, + access_state_param, + get_identity_db, + identity_schema, + to_access_state, +) from app.services.user_events import schedule_event logger = logging.getLogger(__name__) @@ -83,18 +117,19 @@ class TeamActor: async def current_team_actor( request: Request, - db: Annotated[Session, Depends(get_db)], + identity_db: Annotated[Session, Depends(get_identity_db)], ) -> TeamActor: """Dependency: session-only identity, роль admin|manager, иначе 401/403. Намеренно НЕ читает `X-Authenticated-User` — см. модульный docstring. + Сессия резолвится в БД РЕЕСТРА (см. про две сессии в модульном docstring). """ token = request.cookies.get(settings.session_cookie_name) if not token: raise HTTPException(status_code=401, detail="valid session required") try: - session_user = get_session_user(db, token) + session_user = get_session_user(identity_db, token) except Exception: logger.exception("team: session lookup failed") raise HTTPException(status_code=401, detail="valid session required") from None @@ -159,30 +194,40 @@ def _require_same_origin(request: Request) -> None: # --------------------------------------------------------------------------- +# Имена таблицы и колонки состояния доступа приходят из `identity_schema()` — +# фиксированный словарь в `app.services.identity_store`, единственный источник +# этих имён (в SQL-строку не попадает ничего пришедшего снаружи; значения +# по-прежнему биндятся параметрами). +# +# `AS access_state` в КАЖДОМ SELECT'е — не косметика: колонка называется +# по-разному в двух схемах, и без алиаса вызывающий код читал бы то `is_active`, +# то `access_state`, то есть завёл бы то самое второе представление состояния, +# которого быть не должно. Дальше значение всегда идёт через `to_access_state()`. +def _employee_columns(schema: IdentitySchema) -> str: + return ( + "id, username, role, display_name, org_name, email, " + f"{schema.access_state_column} AS access_state, manager_id, created_at" + ) + + # Два статических варианта — НЕ динамическая сборка WHERE (та же мотивация, что -# у `_LIST_EMPLOYEES_*_SQL` ниже: значения и так биндятся параметрами, но +# у `_list_employees_sql` ниже: значения и так биндятся параметрами, но # статические ветки не провоцируют будущие правки в сторону конкатенации SQL). # Роль 'admin' не встречается ни в одной ветке — см. модульный docstring. -_FETCH_MANAGED_EMPLOYEE_SQL = text( - """ - SELECT id, username, role, display_name, org_name, email, is_active, - manager_id, created_at - FROM tradein_users - WHERE id = :id AND role = 'employee' - """ -) - -_FETCH_MANAGED_ANY_SQL = text( - """ - SELECT id, username, role, display_name, org_name, email, is_active, - manager_id, created_at - FROM tradein_users - WHERE id = :id AND role IN ('employee', 'manager') - """ -) +def _fetch_employee_sql(actor_role: str) -> TextClause: + schema = identity_schema() + cols = _employee_columns(schema) + if actor_role == "admin": + return text( + f"SELECT {cols} FROM {schema.users_table} " + "WHERE id = :id AND role IN ('employee', 'manager')" + ) + return text(f"SELECT {cols} FROM {schema.users_table} WHERE id = :id AND role = 'employee'") -def _fetch_employee_row(db: Session, employee_id: int, actor: TeamActor) -> RowMapping | None: +def _fetch_employee_row( + identity_db: Session, employee_id: int, actor: TeamActor +) -> RowMapping | None: """Строка управляемого юзера в пределах прав *actor* — иначе None (→ 404). Фильтр по роли делается ЗДЕСЬ, в SQL, а не в `_authorize_employee` ниже: @@ -191,8 +236,8 @@ def _fetch_employee_row(db: Session, employee_id: int, actor: TeamActor) -> RowM тебе не по зубам») — тот же принцип, что и 404-вместо-403 в `_authorize_employee`: не палим существование чужой строки. """ - sql = _FETCH_MANAGED_ANY_SQL if actor.role == "admin" else _FETCH_MANAGED_EMPLOYEE_SQL - return db.execute(sql, {"id": employee_id}).mappings().fetchone() + sql = _fetch_employee_sql(actor.role) + return identity_db.execute(sql, {"id": employee_id}).mappings().fetchone() def _authorize_employee(actor: TeamActor, row: RowMapping | None) -> RowMapping: @@ -343,6 +388,13 @@ def _batch_quota_status(db: Session, usernames: list[str]) -> dict[str, dict[str def _employee_out(row: RowMapping, quota: dict[str, Any]) -> EmployeeOut: + """Строка реестра → ответ API. + + `is_active` в контракте API остаётся булевым (форма ответа не меняется — + фронт «Команды» не трогаем этим PR), и считается он ровно как «пустят ли + входить»: `trial_expired` показывается как заблокированный. Отдельное + отображение пробного периода в «Команде» — вопрос UI-PR'а, не этого. + """ return EmployeeOut( id=row["id"], username=row["username"], @@ -350,7 +402,7 @@ def _employee_out(row: RowMapping, quota: dict[str, Any]) -> EmployeeOut: display_name=row["display_name"], org_name=row["org_name"], email=row["email"], - is_active=row["is_active"], + is_active=to_access_state(row["access_state"]).can_sign_in, manager_id=row["manager_id"], created_at=row["created_at"], quota=QuotaStatusOut(**quota), @@ -367,6 +419,7 @@ async def create_employee( body: EmployeeCreateRequest, actor: Annotated[TeamActor, Depends(current_team_actor)], db: Annotated[Session, Depends(get_db)], + identity_db: Annotated[Session, Depends(get_identity_db)], _origin_check: Annotated[None, Depends(_require_same_origin)], ) -> EmployeeOut: """Создать сотрудника. Роль всегда `employee`. @@ -375,9 +428,13 @@ async def create_employee( значение из тела ИГНОРИРУЕТСЯ, org-изоляция инвариант #2554). Для actor.role == admin — опционально из тела, валидируется что указанный id существует и role='manager' (иначе 422). + + `identity_db` — реестр (строка сотрудника), `db` — продуктовая квота; + в дефолтном режиме это одна и та же сессия и одна транзакция. """ - existing = db.execute( - text("SELECT id FROM tradein_users WHERE username = :u"), + schema = identity_schema() + existing = identity_db.execute( + text(f"SELECT id FROM {schema.users_table} WHERE username = :u"), {"u": body.username}, ).fetchone() if existing is not None: @@ -396,8 +453,8 @@ async def create_employee( else: manager_id = body.manager_id if manager_id is not None: - mgr = db.execute( - text("SELECT id FROM tradein_users WHERE id = :id AND role = 'manager'"), + mgr = identity_db.execute( + text(f"SELECT id FROM {schema.users_table} WHERE id = :id AND role = 'manager'"), {"id": manager_id}, ).fetchone() if mgr is None: @@ -408,17 +465,16 @@ async def create_employee( try: row = ( - db.execute( + identity_db.execute( text( - """ - INSERT INTO tradein_users + f""" + INSERT INTO {schema.users_table} (username, password_hash, role, manager_id, display_name, org_name, - email, is_active) + email, {schema.access_state_column}) VALUES (:username, :password_hash, 'employee', :manager_id, :display_name, - :org_name, :email, true) - RETURNING id, username, role, display_name, org_name, email, is_active, - manager_id, created_at + :org_name, :email, :access_state) + RETURNING {_employee_columns(schema)} """ ), { @@ -428,6 +484,10 @@ async def create_employee( "display_name": body.display_name, "org_name": body.org_name, "email": body.email, + # Новый сотрудник заводится с открытым доступом — как и + # раньше (`is_active = true` литералом). Литерала здесь + # больше нет: тип колонки разный, знает о нём identity_store. + "access_state": access_state_param(AccessState.ACTIVE), }, ) .mappings() @@ -435,8 +495,8 @@ async def create_employee( ) except IntegrityError: # TOCTOU: два конкурентных POST с одинаковым username между pre-check - # выше и этим INSERT — UNIQUE-констрейнт на tradein_users.username ловит. - db.rollback() + # выше и этим INSERT — UNIQUE-констрейнт на username в реестре ловит. + identity_db.rollback() raise HTTPException(status_code=409, detail="username already exists") from None assert row is not None # RETURNING на успешный INSERT всегда отдаёт строку @@ -444,7 +504,15 @@ async def create_employee( if body.monthly_limit is not None: _upsert_quota_override(db, body.username, body.monthly_limit, actor.username) - db.commit() + # Реестр коммитится ПЕРВЫМ. В дефолтном режиме это один коммит на одну + # транзакцию (identity_db is db) — ровно как было. В режиме `auth` БД две, + # и порядок выбран по цене сбоя: не доехавшая квота — это сотрудник с + # глобальным лимитом (чинится повторным PATCH), тогда как не доехавшая + # строка сотрудника при уже сохранённой квоте — висящий override на + # несуществующего человека. + identity_db.commit() + if db is not identity_db: + db.commit() schedule_event( event_type="employee_created", @@ -471,6 +539,7 @@ async def update_employee( body: EmployeeUpdateRequest, actor: Annotated[TeamActor, Depends(current_team_actor)], db: Annotated[Session, Depends(get_db)], + identity_db: Annotated[Session, Depends(get_identity_db)], _origin_check: Annotated[None, Depends(_require_same_origin)], ) -> EmployeeOut: """Частичное обновление сотрудника — block/unblock, лимит, профиль, пароль. @@ -483,8 +552,14 @@ async def update_employee( КАЖДОМ запросе, так что скомпрометированная/чужая сессия живёт неограниченно долго, а не «до TTL». `revoke_user_sessions` сам называет смену пароля своим use-case — см. его докстринг. + + `is_active` в теле остаётся булевым (контракт API не меняется): true → + `active`, false → `disabled`. Перевести аккаунт В `trial_expired` этим + роутом нельзя — это состояние проставляется миграцией/владельцем, а + выразить его булевым полем нечем; is_active=true на таком аккаунте открывает + доступ (снимает пробное ограничение), is_active=false закрывает жёстко. """ - row = _fetch_employee_row(db, employee_id, actor) + row = _fetch_employee_row(identity_db, employee_id, actor) row = _authorize_employee(actor, row) new_password_hash: str | None = None @@ -494,14 +569,27 @@ async def update_employee( except ValueError as e: raise HTTPException(status_code=422, detail=str(e)) from None - db.execute( + schema = identity_schema() + # Пишутся РОВНО те колонки, на которые у auth_app есть column-level GRANT + # UPDATE (data/sql/auth/004, Часть 4): password_hash, display_name, org_name, + # email, access_state, updated_at. role и manager_id этим роутом не + # обновляются — не «пока не понадобилось», а сознательно: право на их запись + # роли приложения не выдано, и добавлять его в обход миграции нельзя. + # + # CAST обязателен из-за NULL-параметра (поле не пришло в PATCH → COALESCE + # оставляет текущее значение): у нетипизированного NULL Postgres не может + # вывести тип. Имя SQL-типа — из фиксированного словаря identity_store. + identity_db.execute( text( - """ - UPDATE tradein_users + f""" + UPDATE {schema.users_table} SET display_name = COALESCE(:display_name, display_name), org_name = COALESCE(:org_name, org_name), email = COALESCE(:email, email), - is_active = COALESCE(CAST(:is_active AS boolean), is_active), + {schema.access_state_column} = COALESCE( + CAST(:access_state AS {schema.access_state_sql_type}), + {schema.access_state_column} + ), password_hash = COALESCE(:password_hash, password_hash), updated_at = now() WHERE id = :id @@ -511,7 +599,13 @@ async def update_employee( "display_name": body.display_name, "org_name": body.org_name, "email": body.email, - "is_active": body.is_active, + "access_state": ( + None + if body.is_active is None + else access_state_param( + AccessState.ACTIVE if body.is_active else AccessState.DISABLED + ) + ), "password_hash": new_password_hash, "id": employee_id, }, @@ -523,13 +617,20 @@ async def update_employee( if body.is_active is False or body.new_password is not None: # Обязательно ПОСЛЕ UPDATE, ДО финального commit — revoke_user_sessions # коммитит сам (см. app.services.auth_session), это флашит и наш - # предшествующий UPDATE/quota-upsert в той же сессии. Self-lockout + # предшествующий UPDATE (а в дефолтном режиме, где сессия одна, — и + # quota-upsert). Сессии живут в БД реестра, вместе с пользователем, + # поэтому рвём их через `identity_db`: с чужой сессией здесь блокировка + # и смена пароля перестали бы действовать немедленно. Self-lockout # невозможен: _fetch_employee_row не отдаёт строки с role='admin' # НИКОМУ, а manager'у — ещё и только role='employee'; т.е. actor # (admin|manager) никогда не может патчить сам себя через этот роут. - revoke_user_sessions(db, employee_id) + revoke_user_sessions(identity_db, employee_id) - db.commit() + # Порядок и смысл — как в create_employee: реестр первым, продуктовая БД + # отдельным коммитом только если она физически другая. + identity_db.commit() + if db is not identity_db: + db.commit() changed_profile_fields = [ f @@ -573,7 +674,7 @@ async def update_employee( }, ) - updated_row = _fetch_employee_row(db, employee_id, actor) + updated_row = _fetch_employee_row(identity_db, employee_id, actor) assert updated_row is not None # только что успешно обновили эту же строку quota = account_quota.get_status(db, updated_row["username"]) return _employee_out(updated_row, quota) @@ -596,50 +697,48 @@ async def update_employee( # постраничном листании. `id` монотонно растёт (BIGINT IDENTITY) — детерминированный # tie-break без доп. индекса (созданные позже = бОльший id, тот же порядок что и # намерение DESC-сортировки по времени). -_LIST_EMPLOYEES_BY_MANAGER_SQL = text( - """ - SELECT id, username, role, display_name, org_name, email, is_active, manager_id, created_at - FROM tradein_users - WHERE role = 'employee' AND manager_id = :manager_id - ORDER BY created_at DESC, id DESC - LIMIT :limit OFFSET :offset - """ -) - -# Admin-ветка: сюда попадают И менеджеры (см. модульный docstring — иначе admin -# не видит в UI строку, которой должен уметь сбросить пароль). `role='admin'` -# по-прежнему невидим и неуправляем. Сортировка по (created_at, id) общая для -# обеих ролей — намеренно: seed (#2557) вставил всех одной транзакцией, так что -# группировка «сначала менеджеры» дала бы ложное ощущение иерархии там, где её -# в данных нет; роль показывается колонкой (`EmployeeOut.role`). -_LIST_EMPLOYEES_ALL_SQL = text( - """ - SELECT id, username, role, display_name, org_name, email, is_active, manager_id, created_at - FROM tradein_users - WHERE role IN ('employee', 'manager') - ORDER BY created_at DESC, id DESC - LIMIT :limit OFFSET :offset - """ -) +# +# Admin-ветка (`by_manager=False`): сюда попадают И менеджеры (см. модульный +# docstring — иначе admin не видит в UI строку, которой должен уметь сбросить +# пароль). `role='admin'` по-прежнему невидим и неуправляем. Сортировка по +# (created_at, id) общая для обеих веток — намеренно: seed (#2557) вставил всех +# одной транзакцией, так что группировка «сначала менеджеры» дала бы ложное +# ощущение иерархии там, где её в данных нет; роль показывается колонкой +# (`EmployeeOut.role`). +def _list_employees_sql(*, by_manager: bool) -> TextClause: + schema = identity_schema() + cols = _employee_columns(schema) + tail = "ORDER BY created_at DESC, id DESC LIMIT :limit OFFSET :offset" + if by_manager: + return text( + f"SELECT {cols} FROM {schema.users_table} " + f"WHERE role = 'employee' AND manager_id = :manager_id {tail}" + ) + return text( + f"SELECT {cols} FROM {schema.users_table} WHERE role IN ('employee', 'manager') {tail}" + ) @router.get("/employees", response_model=list[EmployeeOut]) async def list_employees( actor: Annotated[TeamActor, Depends(current_team_actor)], db: Annotated[Session, Depends(get_db)], + identity_db: Annotated[Session, Depends(get_identity_db)], manager_id: Annotated[int | None, Query()] = None, limit: Annotated[int, Query(ge=1, le=200)] = 50, offset: Annotated[int, Query(ge=0)] = 0, ) -> list[EmployeeOut]: """Список сотрудников. manager видит только своих; admin — всех, опц. ?manager_id=. - Квота — ОДИН батч-запрос на всю страницу (`_batch_quota_status`), не N+1 - (Medium2, review PR #2563: было 2N+3 SQL-запросов на N сотрудников). + Сотрудники читаются из реестра (`identity_db`), квоты — из продуктовой БД + (`db`): `account_quota_overrides`/`account_estimate_usage` в общий реестр не + переезжают. Квота — ОДИН батч-запрос на всю страницу (`_batch_quota_status`), + не N+1 (Medium2, review PR #2563: было 2N+3 SQL-запросов на N сотрудников). """ if actor.role == "manager": rows = ( - db.execute( - _LIST_EMPLOYEES_BY_MANAGER_SQL, + identity_db.execute( + _list_employees_sql(by_manager=True), {"manager_id": actor.user_id, "limit": limit, "offset": offset}, ) .mappings() @@ -647,8 +746,8 @@ async def list_employees( ) elif manager_id is not None: rows = ( - db.execute( - _LIST_EMPLOYEES_BY_MANAGER_SQL, + identity_db.execute( + _list_employees_sql(by_manager=True), {"manager_id": manager_id, "limit": limit, "offset": offset}, ) .mappings() @@ -656,7 +755,11 @@ async def list_employees( ) else: rows = ( - db.execute(_LIST_EMPLOYEES_ALL_SQL, {"limit": limit, "offset": offset}).mappings().all() + identity_db.execute( + _list_employees_sql(by_manager=False), {"limit": limit, "offset": offset} + ) + .mappings() + .all() ) quota_by_username = _batch_quota_status(db, [row["username"] for row in rows]) @@ -673,15 +776,17 @@ async def employee_history( employee_id: int, actor: Annotated[TeamActor, Depends(current_team_actor)], db: Annotated[Session, Depends(get_db)], + identity_db: Annotated[Session, Depends(get_identity_db)], limit: Annotated[int, Query(ge=1, le=200)] = 50, offset: Annotated[int, Query(ge=0)] = 0, ) -> list[EmployeeHistoryEntry]: """История оценок сотрудника (адрес/дата/результат) — из `user_events`, LEFT JOIN `trade_in_estimates` за фактическим результатом. - Та же org-проверка что и в PATCH: чужой employee_id → 404. + Та же org-проверка что и в PATCH: чужой employee_id → 404. Проверка идёт по + реестру (`identity_db`), сама история — продуктовые таблицы (`db`). """ - row = _fetch_employee_row(db, employee_id, actor) + row = _fetch_employee_row(identity_db, employee_id, actor) row = _authorize_employee(actor, row) rows = ( diff --git a/tradein-mvp/backend/app/core/auth_db.py b/tradein-mvp/backend/app/core/auth_db.py new file mode 100644 index 00000000..a7d1f5b4 --- /dev/null +++ b/tradein-mvp/backend/app/core/auth_db.py @@ -0,0 +1,125 @@ +"""Engine + session-factory для БД `auth` — общего реестра людей (эпик «единый вход»). + +Отдельный модуль, а не ещё пара строк в `app.core.db`, ровно по одной причине: +`app.core.db` создаёт engine НА ИМПОРТЕ (`create_engine(settings.database_url)` в +теле модуля). Сделай мы так же для БД `auth` — приложение начало бы падать на +старте везде, где `AUTH_DATABASE_URL` не задан, а не задан он сейчас ВЕЗДЕ: на +проде роль `auth_app` ещё без пароля, в тестах этой БД нет вовсе. Здесь engine +создаётся ЛЕНИВО, при первом реальном обращении. + +Контракт (⚠️ после мержа прод обязан работать ТОЧНО как сейчас): + + * `settings.identity_store == "tradein"` (дефолт) — в этот модуль не заходит + никто: `app.services.identity_store` берёт сессию из `app.core.db`. Пустой + `AUTH_DATABASE_URL` при этом не ошибка ни на импорте, ни в рантайме; ни одно + соединение с БД `auth` не открывается. + * `settings.identity_store == "auth"` + пустой DSN — первое же обращение + поднимает `AuthDatabaseNotConfiguredError` с внятным текстом. Именно + исключение, а НЕ тихий откат на tradein-таблицы и не пустой результат: + молчаливая деградация auth-пути означала бы «пользователь не найден» вместо + «конфигурация сломана», то есть массовый отказ входа под видом неверных + паролей — либо, в обратную сторону, анонимный доступ. + +`create_engine` сам по себе к серверу не ходит (connection pool ленивый), так что +даже после первого обращения реальный коннект открывается только на первом +запросе — но ошибку конфигурации мы обязаны отдать раньше, чем это станет +похоже на сетевую проблему. +""" + +from __future__ import annotations + +import threading +from collections.abc import Iterator +from contextlib import contextmanager + +from sqlalchemy import Engine, create_engine +from sqlalchemy.orm import Session, sessionmaker + +from app.core.config import settings + + +class AuthDatabaseNotConfiguredError(RuntimeError): + """`IDENTITY_STORE=auth`, но `AUTH_DATABASE_URL` пуст — идентичность негде читать.""" + + +_NOT_CONFIGURED_MSG = ( + "IDENTITY_STORE=auth, но AUTH_DATABASE_URL пуст: подключаться к общему реестру " + "людей (БД `auth`) не к чему. Задай DSN роли auth_app в .env.runtime — либо " + "верни IDENTITY_STORE=tradein (старое поведение на tradein_users/tradein_sessions)." +) + +# Кеш engine/factory + защита от гонки: rbac_guard резолвит сессию на каждом +# non-public запросе, а uvicorn обслуживает их из нескольких потоков (sync-роуты +# уходят в threadpool). Без лока два одновременных первых запроса создали бы два +# engine — то есть два независимых пула коннектов, один из которых потеряется. +_LOCK = threading.Lock() +_engine: Engine | None = None +_session_factory: sessionmaker[Session] | None = None + + +def _build() -> tuple[Engine, sessionmaker[Session]]: + """Создаёт engine + session-factory по текущему DSN. Пустой DSN → явная ошибка.""" + dsn = settings.auth_database_url.strip() + if not dsn: + raise AuthDatabaseNotConfiguredError(_NOT_CONFIGURED_MSG) + engine = create_engine(dsn, pool_pre_ping=True, future=True) + factory = sessionmaker(autocommit=False, autoflush=False, bind=engine, expire_on_commit=False) + return engine, factory + + +def _ensure_built() -> tuple[Engine, sessionmaker[Session]]: + global _engine, _session_factory + if _engine is not None and _session_factory is not None: + return _engine, _session_factory + with _LOCK: + if _engine is None or _session_factory is None: + _engine, _session_factory = _build() + return _engine, _session_factory + + +def get_auth_engine() -> Engine: + """Engine БД `auth` (создаётся при первом вызове). + + Raises: + AuthDatabaseNotConfiguredError: `AUTH_DATABASE_URL` пуст. + """ + engine, _ = _ensure_built() + return engine + + +def get_auth_session_factory() -> sessionmaker[Session]: + """Session-factory БД `auth` (создаётся при первом вызове). + + Raises: + AuthDatabaseNotConfiguredError: `AUTH_DATABASE_URL` пуст. + """ + _, factory = _ensure_built() + return factory + + +@contextmanager +def auth_session() -> Iterator[Session]: + """Сессия к БД `auth`, закрывается на выходе из блока. + + Прямой вызов из роутов/сервисов НЕ предполагается — ходи через + `app.services.identity_store.identity_session()`, он один знает, какая БД + сейчас является реестром. + """ + factory = get_auth_session_factory() + with factory() as db: + yield db + + +def reset_auth_db() -> None: + """Сбрасывает закешированные engine/factory (смена DSN в рантайме, тесты). + + Старый engine `dispose()`-ится вне лока: закрытие пула может блокировать, а + держать в это время лок незачем — ссылки на него уже сняты. + """ + global _engine, _session_factory + with _LOCK: + stale = _engine + _engine = None + _session_factory = None + if stale is not None: + stale.dispose() diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index b4a3f507..7ee6f9e5 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -71,6 +71,29 @@ class Settings(BaseSettings): default=300, validation_alias="LOGIN_RATE_LIMIT_WINDOW_S" ) + # ── Эпик «единый вход»: общий реестр людей в БД `auth` ───────────────────── + # DSN БД `auth` (роль auth_app) — единый реестр людей «Меры» (trade-in) и + # «Птицы» (Site Finder); схема — data/sql/auth/001-004. + # + # ПУСТО ПО УМОЛЧАНИЮ, И ЭТО НЕ ОШИБКА. На проде пароль роли auth_app ещё не + # заведён (переменной AUTH_DATABASE_URL там нет), данные (хеши/роли/живые + # сессии) в `auth` ещё не скопированы. Пока identity_store="tradein" (дефолт) + # к этой БД не обращается ни одна строка кода: engine не создаётся, + # соединение не открывается, пустой DSN на старте ничего не роняет — см. + # app.core.auth_db (ленивое создание engine). ENV: AUTH_DATABASE_URL. + auth_database_url: str = Field(default="", validation_alias="AUTH_DATABASE_URL") + # Где живут identity (люди + сессии): + # "tradein" (ДЕФОЛТ) — БД tradein, таблицы tradein_users/tradein_sessions + # (ровно сегодняшний прод, поведение не меняется); + # "auth" — БД auth, таблицы users/sessions (единый реестр). + # Переключать ТОЛЬКО после того, как на проде заведён пароль auth_app и + # перенесены данные. Дефолт = старое поведение: включить новый путь можно + # исключительно явной сменой этого флага. Единственный потребитель — + # app.services.identity_store. ENV: IDENTITY_STORE. + identity_store: Literal["tradein", "auth"] = Field( + default="tradein", validation_alias="IDENTITY_STORE" + ) + # для User-Agent в Nominatim (Nominatim Usage Policy) contact_email: str = "erginrajpopxbe@outlook.com" diff --git a/tradein-mvp/backend/app/core/rbac.py b/tradein-mvp/backend/app/core/rbac.py index eb9ec090..c391c366 100644 --- a/tradein-mvp/backend/app/core/rbac.py +++ b/tradein-mvp/backend/app/core/rbac.py @@ -10,13 +10,19 @@ so a regression in that check would NOT have failed CI. This module holds the real guard. Historically it had "no DB/lifespan/scheduler side effects" beyond ``app.core.auth``/``app.core.config`` (both side-effect-free at import time). #2552 (dual-mode DB-session auth) adds a conditional per-request -DB round trip via ``app.core.db.SessionLocal`` — но ТОЛЬКО когда запрос реально -несёт session-cookie (``request.cookies.get(settings.session_cookie_name)``); -без cookie (весь существующий тестовый трафик, legacy Caddy trusted-header -запросы) ветка не выполняется — ноль новых DB-побочных эффектов для старых -путей. ``app/main.py`` and the test apps both import THIS module, so tests -exercise the exact production code path instead of a copy that can silently -fall out of sync. +DB round trip via ``app.services.identity_store.identity_session`` — но ТОЛЬКО +когда запрос реально несёт session-cookie +(``request.cookies.get(settings.session_cookie_name)``); без cookie (весь +существующий тестовый трафик, legacy Caddy trusted-header запросы) ветка не +выполняется — ноль новых DB-побочных эффектов для старых путей. ``app/main.py`` +and the test apps both import THIS module, so tests exercise the exact +production code path instead of a copy that can silently fall out of sync. + +Сессия открывается через ``identity_session()``, а не через +``app.core.db.SessionLocal`` напрямую: guard — middleware, FastAPI-DI здесь нет, +а реестр людей при ``IDENTITY_STORE=auth`` лежит в другой БД. В дефолтном режиме +``identity_session()`` открывает ровно ``app.core.db.SessionLocal()`` — тот же +коннект-пул и то же поведение, что до эпика «единый вход». """ from __future__ import annotations @@ -32,8 +38,8 @@ from fastapi.responses import JSONResponse, Response from app.core.auth import get_role, is_path_allowed from app.core.config import settings -from app.core.db import SessionLocal from app.services.auth_session import get_db_role_scope, get_session_user +from app.services.identity_store import identity_session logger = logging.getLogger(__name__) @@ -178,9 +184,23 @@ async def rbac_guard( if token: session_user: dict[str, Any] | None = None try: - with SessionLocal() as db: + with identity_session() as db: session_user = get_session_user(db, token) except Exception: + # Сюда попадает и AuthDatabaseNotConfiguredError (IDENTITY_STORE=auth + # без AUTH_DATABASE_URL): резолв сессии не состоялся, дальше работает + # тот же путь, что и при любом сбое БД, — auth_mode решает, пускать ли + # legacy trusted-header. + # + # ⚠️ Этот except НЕ должен быть тем, что ловит сломанный DSN: молча + # деградировать в legacy trusted-header означало бы раздавать права + # из roles.yaml в обход реестра (включая аккаунты с access_state + # 'disabled'/'trial_expired'), причём сутками — продуктовая БД жива, + # приложение работоспособно, сигнал только в логах. Поэтому + # конфигурацию проверяет lifespan (app/main.py): при + # IDENTITY_STORE=auth пустой DSN роняет СТАРТ. Здесь остаётся второй + # рубеж — реестр, отвалившийся уже после успешного старта, не имеет + # права отдавать 500. logger.exception("RBAC: session lookup failed for %s", path) if session_user is not None: username = session_user["username"] diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 0a45d24c..bea049b8 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -34,6 +34,7 @@ from app.api.v1 import ( team, trade_in, ) +from app.core.auth_db import get_auth_engine from app.core.config import settings from app.core.db import SessionLocal from app.core.fdw import ensure_fdw_user_mapping @@ -121,6 +122,27 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: "которым подпись реально нужна" ) + # Эпик «единый вход»: при IDENTITY_STORE=auth реестр людей обязан быть + # СКОНФИГУРИРОВАН — иначе стартуем сломанными. Ошибка DSN не похожа на «БД + # недоступна»: продуктовая БД жива, приложение полностью работоспособно и + # может так работать сутками, а rbac_guard ловит AuthDatabaseNotConfiguredError + # вместе с любым другим сбоем резолва сессии и падает в legacy + # trusted-header ветку (auth_mode='dual'). То есть любой, кого пропустил + # Caddy basic_auth, молча получал бы права из roles.yaml — даже аккаунт с + # access_state='disabled'/'trial_expired' в реестре. Пусть лучше сломанный + # деплой не поднимется вообще, чем сутки раздаёт доступ мимо реестра. + # + # На ДЕФОЛТНЫЙ режим не влияет: при identity_store="tradein" (прод сегодня) + # ветка не выполняется, engine БД `auth` не создаётся, пустой + # AUTH_DATABASE_URL по-прежнему не ошибка. + if settings.identity_store == "auth": + # Наружу летит AuthDatabaseNotConfiguredError с внятным текстом + # (app.core.auth_db); create_engine к серверу не ходит, так что это + # проверка КОНФИГУРАЦИИ, а не доступности БД — недоступный сервер + # по-прежнему не мешает старту. + get_auth_engine() + logger.info("identity_store=auth: DSN общего реестра людей (БД `auth`) сконфигурирован") + # FDW bootstrap: create/refresh USER MAPPING for gendesign_remote postgres_fdw server. # Best-effort: failure does not abort startup, just logs. try: diff --git a/tradein-mvp/backend/app/services/auth_session.py b/tradein-mvp/backend/app/services/auth_session.py index 385444dc..38071bea 100644 --- a/tradein-mvp/backend/app/services/auth_session.py +++ b/tradein-mvp/backend/app/services/auth_session.py @@ -1,15 +1,26 @@ """Session-сервис для DB-backed auth (#2552, эпик #2549 — auth-core). -Схема: `tradein_users` + `tradein_sessions` (migration `192_tradein_users_auth.sql`). +Схема НЕ зашита: имена таблиц и имя колонки состояния доступа берутся из +`app.services.identity_store.identity_schema()` — эпик «единый вход» переводит +реестр людей с `tradein_users`/`tradein_sessions` (migration +`192_tradein_users_auth.sql`, БД tradein) на `users`/`sessions` (БД `auth`, +миграции data/sql/auth/001-004) флагом `IDENTITY_STORE`, дефолт которого = +сегодняшнее прод-поведение. Никаких других отличий между режимами у этого +модуля нет: SQL один и тот же, подставляются только имена из фиксированного +словаря `identity_store._SCHEMAS`. + Опаковые (`secrets.token_urlsafe`) токены-сессии — не JWT, не подписаны: валидность -проверяется исключительно наличием + `expires_at`/`is_active` строкой в БД, поэтому -`SESSION_SECRET` НЕ обязателен для работы этого модуля (зарезервирован на будущее, -см. `app.core.config.Settings.session_secret` docstring). +проверяется исключительно наличием строки + `expires_at` + состоянием доступа +юзера в БД, поэтому `SESSION_SECRET` НЕ обязателен для работы этого модуля +(зарезервирован на будущее, см. `app.core.config.Settings.session_secret` docstring). Все функции здесь принимают уже открытую `db: Session` — сами НЕ открывают -`SessionLocal()` (вызывающая сторона решает время жизни транзакции: `rbac_guard` -и `app.core.db.get_db()`-роуты открывают её по-разному). Это делает модуль -тривиально unit-тестируемым без патчинга `SessionLocal` — тесты просто передают +сессию (вызывающая сторона решает время жизни транзакции: `rbac_guard` и +роуты открывают её по-разному). ⚠️ Это ОБЯЗАНА быть сессия РЕЕСТРА +(`identity_store.identity_session()` / `Depends(get_identity_db)`), а не +`app.core.db.get_db`: при `IDENTITY_STORE=auth` запрос уйдёт в БД tradein, +где таблиц `users`/`sessions` нет. В дефолтном режиме это один и тот же объект. +Модуль остаётся тривиально unit-тестируемым — тесты просто передают fake/real `Session`. Ни одна функция не должна ронять вызывающий HTTP-запрос: DB-ошибки логируются @@ -30,6 +41,7 @@ from sqlalchemy import text from sqlalchemy.orm import Session from app.core.config import settings +from app.services.identity_store import identity_schema, to_access_state logger = logging.getLogger(__name__) @@ -52,11 +64,13 @@ def create_session( `expires_at = now() + settings.session_ttl_hours`. Коммитит сам (self-contained, как `app.services.user_events.record_event`). """ + schema = identity_schema() token = secrets.token_urlsafe(_TOKEN_BYTES) db.execute( text( - """ - INSERT INTO tradein_sessions (token, user_id, expires_at, ip_address, user_agent) + f""" + INSERT INTO {schema.sessions_table} + (token, user_id, expires_at, ip_address, user_agent) VALUES ( :token, :user_id, now() + make_interval(hours => CAST(:ttl_hours AS integer)), @@ -78,7 +92,16 @@ def create_session( def get_session_user(db: Session, token: str) -> dict[str, Any] | None: """Резолвит сессионный токен в данные юзера, или None если сессия - невалидна (не найдена / истекла / юзер деактивирован). + невалидна (не найдена / истекла / доступ юзера не `active`). + + Состояние доступа: пропускает ТОЛЬКО `AccessState.ACTIVE`. Любое другое + (`disabled`, `trial_expired`, а также нераспознанное — `to_access_state` + fail-closed'ит его в `disabled`) делает уже выданную сессию недействительной + немедленно, без ожидания TTL. Это то же решение, что и в булевой схеме + (`is_active = false` → None), просто теперь состояний больше одного: + «пробный период истёк» гасит живую сессию так же, как блокировка — иначе + сотрудник, залогиненный до истечения пробного доступа, продолжал бы + работать, а sliding-refresh продлевал бы ему сессию бесконечно. Sliding refresh: если с последнего `last_seen_at` прошло >=5 минут — продлевает `expires_at`/`last_seen_at` ОДНИМ UPDATE. Сбой refresh @@ -88,13 +111,15 @@ def get_session_user(db: Session, token: str) -> dict[str, Any] | None: if not token: return None + schema = identity_schema() row = db.execute( text( - """ + f""" SELECT s.user_id, s.expires_at, s.last_seen_at, - u.username, u.role, u.display_name, u.org_name, u.email, u.is_active - FROM tradein_sessions s - JOIN tradein_users u ON u.id = s.user_id + u.username, u.role, u.display_name, u.org_name, u.email, + u.{schema.access_state_column} AS access_state + FROM {schema.sessions_table} s + JOIN {schema.users_table} u ON u.id = s.user_id WHERE s.token = :token """ ), @@ -107,15 +132,16 @@ def get_session_user(db: Session, token: str) -> dict[str, Any] | None: now = datetime.now(UTC) if row.expires_at is None or row.expires_at <= now: return None - if not row.is_active: + access_state = to_access_state(row.access_state) + if not access_state.can_sign_in: return None if row.last_seen_at is None or (now - row.last_seen_at) >= _SLIDING_REFRESH_INTERVAL: try: db.execute( text( - """ - UPDATE tradein_sessions + f""" + UPDATE {schema.sessions_table} SET last_seen_at = now(), expires_at = now() + make_interval(hours => CAST(:ttl_hours AS integer)) WHERE token = :token @@ -137,23 +163,35 @@ def get_session_user(db: Session, token: str) -> dict[str, Any] | None: "display_name": row.display_name, "org_name": row.org_name, "email": row.email, - "is_active": row.is_active, + # Всегда AccessState.ACTIVE — не-active сюда не доходит (см. выше). + # Ключ оставлен вместо прежнего `is_active`, чтобы состояние доступа во + # ВСЁМ коде называлось и выражалось одинаково. + "access_state": access_state, } def get_user_by_username(db: Session, username: str) -> dict[str, Any] | None: - """Возвращает строку `tradein_users` по username, или None если не найден. + """Возвращает строку реестра по username, или None если не найден. Используется login-флоу (`app.api.v1.auth.login`) для password-проверки. Отдаёт `password_hash` как есть (может быть NULL — переходный период, см. migration 192 docstring) — вызывающая сторона решает, что с ним делать. + + `access_state` — уже `AccessState` (не сырое значение колонки): решение + «пускать / не пускать / показать экран пробного периода» принимает login, + и принимать его он обязан по ОДНОМУ понятию, а не по boolean в одном режиме + и строке в другом. Отсутствие юзера состоянием НЕ выражается (None остаётся + None) — иначе login потерял бы разницу между «нет такого логина» и + «заблокирован», а она нужна ему для выбора события аудита. """ + schema = identity_schema() row = db.execute( text( - """ - SELECT id, username, password_hash, role, is_active, + f""" + SELECT id, username, password_hash, role, + {schema.access_state_column} AS access_state, display_name, org_name, email - FROM tradein_users + FROM {schema.users_table} WHERE username = :username """ ), @@ -168,7 +206,7 @@ def get_user_by_username(db: Session, username: str) -> dict[str, Any] | None: "username": row.username, "password_hash": row.password_hash, "role": row.role, - "is_active": row.is_active, + "access_state": to_access_state(row.access_state), "display_name": row.display_name, "org_name": row.org_name, "email": row.email, @@ -177,14 +215,19 @@ def get_user_by_username(db: Session, username: str) -> dict[str, Any] | None: def revoke_session(db: Session, token: str) -> None: """Удаляет одну сессию по токену (logout). No-op если токен не найден.""" - db.execute(text("DELETE FROM tradein_sessions WHERE token = :token"), {"token": token}) + schema = identity_schema() + db.execute(text(f"DELETE FROM {schema.sessions_table} WHERE token = :token"), {"token": token}) db.commit() def revoke_user_sessions(db: Session, user_id: int) -> None: - """Удаляет ВСЕ сессии юзера (напр. смена пароля / принудительный logout всех - устройств — не используется этим PR напрямую, задел для будущих admin-действий).""" - db.execute(text("DELETE FROM tradein_sessions WHERE user_id = :user_id"), {"user_id": user_id}) + """Удаляет ВСЕ сессии юзера — смена пароля и блокировка обязаны рвать + активные сессии немедленно (см. `app.api.v1.team.update_employee`).""" + schema = identity_schema() + db.execute( + text(f"DELETE FROM {schema.sessions_table} WHERE user_id = :user_id"), + {"user_id": user_id}, + ) db.commit() @@ -192,7 +235,9 @@ def revoke_user_sessions(db: Session, user_id: int) -> None: # DB-role → RBAC scope (paths/deny) — #2552 dual-mode. # --------------------------------------------------------------------------- # -# tradein_users.role ('admin'|'manager'|'employee', CHECK-констрейнт migration 192) +# Роли реестра ('admin'|'manager'|'employee' — CHECK-констрейнт: tradein м.192 для +# tradein_users.role, auth м.004 для auth.users.role; наборы значений совпадают +# намеренно, чтобы код «Меры» переехал на общий реестр без правок в проверках роли) # НЕ являются ключами auth/roles.yaml (тот файл — legacy Caddy trusted-header путь, # который этот эпик намеренно не трогает). Маппинг ниже даёт DB-ролям тот же # paths/deny-смысл, что и legacy-ролям, БЕЗ правки roles.yaml: diff --git a/tradein-mvp/backend/app/services/identity_store.py b/tradein-mvp/backend/app/services/identity_store.py new file mode 100644 index 00000000..f20ec47e --- /dev/null +++ b/tradein-mvp/backend/app/services/identity_store.py @@ -0,0 +1,291 @@ +"""Единственное место, знающее, В КАКОЙ БД и В КАКИХ ТАБЛИЦАХ живёт identity. + +Эпик «единый вход»: люди «Меры» (trade-in) и «Птицы» (Site Finder) переезжают в +общую БД `auth` (`users` / `sessions`, миграции data/sql/auth/001-004), а +`tradein_users` в итоге удаляется. Переезд идёт под флагом +`settings.identity_store`, дефолт которого = СТАРОЕ поведение: + + "tradein" (ДЕФОЛТ) — БД tradein, tradein_users / tradein_sessions; + "auth" — БД auth, users / sessions. + +Смысл модуля: во всём остальном коде не должно быть ни одного упоминания +конкретной БД, конкретных имён таблиц и того, каким столбцом выражено состояние +доступа. Кто хочет читать/писать людей и сессии — спрашивает здесь. + +Что модуль отдаёт вызывающему: + * `identity_session()` / `get_identity_db()` — сессия ТОЙ БД, которая сейчас + является реестром (для "tradein" это ровно `app.core.db.SessionLocal`, то + есть сегодняшний прод-путь без единого лишнего коннекта); + * `identity_schema()` — имена таблиц users/sessions и имя колонки состояния + доступа; + * `AccessState` + `to_access_state()` — ОДНО понятие «состояние доступа» для + обеих схем. + +Схемы `tradein_users` и `auth.users` совпадают, кроме состояния доступа: +`tradein_users.is_active` — boolean, `auth.users.access_state` — text из трёх +значений (`active` / `trial_expired` / `disabled`, семантика — в COMMENT'е +миграции 004). Вызывающий код обязан работать с ОДНИМ понятием: он читает +колонку `schema.access_state_column` и прогоняет значение через +`to_access_state()`. Второго представления состояния в коде быть не должно — +`if row.is_active` вне этого модуля больше не пишем. + +Как СПРАШИВАТЬ состояние доступа (канонический вызов): + + schema = identity_schema() + with identity_session() as db: + row = db.execute( + text( + f"SELECT u.id, u.username, u.role, " + f" u.{schema.access_state_column} AS access_state " + f" FROM {schema.users_table} u " + f" WHERE u.username = :username" + ), + {"username": username}, + ).fetchone() + state = to_access_state(row.access_state) + if not state.can_sign_in: + ... # 401 для disabled, отдельный 403 для AccessState.TRIAL_EXPIRED + +Значение подставляется bind-параметром (`:username`), имя таблицы и имя колонки — +из `schema`, то есть из фиксированного словаря; в SQL-строку не попадает ничего, +пришедшего снаружи. + +Как ПИСАТЬ состояние доступа (обратное направление, `access_state_param()`): + + db.execute( + text( + f"UPDATE {schema.users_table} " + f" SET {schema.access_state_column} = :access_state " + f" WHERE id = :id" + ), + {"access_state": access_state_param(AccessState.DISABLED), "id": user_id}, + ) + +Литералов `True` / `'active'` по месту быть не должно: тип колонки разный, и +единственное место, знающее какой, — этот модуль. + +⚠️ SQL-инъекция по имени таблицы: имена таблиц/колонок в SQL нельзя передать +bind-параметром, поэтому они подставляются в строку запроса. Единственный +допустимый источник — фиксированный словарь `_SCHEMAS` НИЖЕ. Никакой +конкатенации с внешним вводом (заголовок, тело запроса, переменная окружения, +имя роли) — значение `settings.identity_store` ограничено `Literal` в pydantic, +и лукап по нему делается только здесь. +""" + +from __future__ import annotations + +import logging +from collections.abc import Generator, Iterator +from contextlib import contextmanager +from dataclasses import dataclass +from enum import StrEnum +from typing import Annotated + +from fastapi import Depends +from sqlalchemy.orm import Session + +from app.core import auth_db +from app.core.config import settings +from app.core.db import SessionLocal, get_db + +logger = logging.getLogger(__name__) + + +class AccessState(StrEnum): + """Состояние доступа аккаунта — ЕДИНОЕ понятие для обеих схем. + + Значения дословно совпадают с `auth.users.access_state` (CHECK-констрейнт + `users_access_state_ck`, миграция 004); булев `tradein_users.is_active` + приводится сюда в `to_access_state()`. + + Семантика (COMMENT миграции 004, решение владельца от 2026-07-31): + active — вход разрешён; + trial_expired — пароль ВЕРНЫЙ, но пробный период истёк: отдельный 403 и + экран «пробный доступ закончился», сессия не выдаётся; + disabled — доступ закрыт: generic 401, для пользователя неотличимо от + неверного пароля. + Неверный пароль в ЛЮБОМ состоянии → generic 401, иначе отдельный ответ для + trial_expired превращается в оракул существования логина. + """ + + ACTIVE = "active" + TRIAL_EXPIRED = "trial_expired" + DISABLED = "disabled" + + @property + def can_sign_in(self) -> bool: + """True только для `active` — единственная проверка «пускать ли». + + Вынесена в свойство, чтобы вызывающий не писал `state == "active"`: + добавится четвёртое состояние — оно по умолчанию окажется «не пускать», + а не «пускать, потому что не disabled». + """ + return self is AccessState.ACTIVE + + +@dataclass(frozen=True, slots=True) +class IdentitySchema: + """Где физически лежит identity при текущем значении флага. + + Attributes: + store: значение `settings.identity_store`, которому соответствует схема. + users_table: имя таблицы людей. + sessions_table: имя таблицы сессий. + access_state_column: имя колонки состояния доступа. Значение из неё + ОБЯЗАНО пройти через `to_access_state()` — тип отличается между + схемами (boolean против text). + access_state_sql_type: SQL-тип этой колонки для `CAST(:param AS ...)`. + Нужен там, где параметр может быть NULL (`COALESCE(CAST(:x AS T), col)` + в PATCH «Команды»): без явного типа Postgres не может вывести тип + NULL-параметра. Значение — литерал из `_SCHEMAS`, в SQL-строку + снаружи ничего не попадает. + """ + + store: str + users_table: str + sessions_table: str + access_state_column: str + access_state_sql_type: str + + +# Фиксированный словарь — ЕДИНСТВЕННЫЙ источник имён таблиц/колонок для SQL. +# Ключи = допустимые значения settings.identity_store (Literal в pydantic). +_SCHEMAS: dict[str, IdentitySchema] = { + "tradein": IdentitySchema( + store="tradein", + users_table="tradein_users", + sessions_table="tradein_sessions", + access_state_column="is_active", + access_state_sql_type="boolean", + ), + "auth": IdentitySchema( + store="auth", + # В БД `auth` таблицы лежат без префикса продукта — реестр общий + # (data/sql/auth/001_identity_schema.sql). + users_table="users", + sessions_table="sessions", + access_state_column="access_state", + access_state_sql_type="text", + ), +} + + +def identity_schema() -> IdentitySchema: + """Схема реестра для текущего значения `settings.identity_store`. + + Читается на КАЖДОМ вызове, а не кешируется на импорте: тесты и + переключение флага не должны требовать перезагрузки модулей. + """ + schema = _SCHEMAS.get(settings.identity_store) + if schema is None: + # Недостижимо через настройки (Literal валидируется pydantic), но + # молчаливый fallback здесь означал бы поход не в ту БД. + raise ValueError(f"неизвестный identity_store={settings.identity_store!r}") + return schema + + +@contextmanager +def identity_session() -> Iterator[Session]: + """Сессия БД, в которой сейчас живёт identity. + + "tradein" → `app.core.db.SessionLocal` (та же БД и тот же пул, что у всего + остального приложения — сегодняшнее поведение прода без изменений). + "auth" → ленивый engine `app.core.auth_db`; пустой `AUTH_DATABASE_URL` + здесь поднимет `AuthDatabaseNotConfiguredError`, а не отдаст пустой + результат. + """ + if settings.identity_store == "auth": + with auth_db.auth_session() as db: + yield db + else: + with SessionLocal() as db: + yield db + + +def get_identity_db( + db: Annotated[Session, Depends(get_db)], +) -> Generator[Session, None, None]: + """FastAPI-зависимость: `db: Annotated[Session, Depends(get_identity_db)]`. + + Аналог `app.core.db.get_db`, но для реестра людей. Роуты, работающие с + identity, обязаны брать сессию отсюда — иначе при `identity_store="auth"` + они уйдут запросом в БД tradein, где нужных таблиц уже не будет. + + ⚠️ При `identity_store="tradein"` отдаётся РОВНО ТОТ ЖЕ объект `Session`, + что и у `Depends(get_db)` — не новая сессия к той же БД. Это не экономия + коннекта, а требование «прод обязан работать точно как сейчас»: роуты + «Команды» пишут в ОДНОЙ транзакции строку сотрудника (реестр) и его квоту + (`account_quota_overrides`, продуктовая таблица). Две сессии = две + транзакции = состояние «сотрудник создан, квота нет» на ровном месте. + FastAPI кеширует результат `Depends(get_db)` в пределах запроса, поэтому + роут, объявивший ОБЕ зависимости, в этом режиме получает один и тот же + объект, и `db is identity_db` — честный рантайм-признак «одна БД». + + При `identity_store="auth"` это разные БД физически, и одной транзакции + быть не может (двухфазный коммит здесь не заводим): вызывающий код обязан + коммитить обе сессии и понимать порядок — см. `app.api.v1.team`. + Зависимость `get_db` при этом всё равно резолвится, но `Session` ленив — + без единого запроса он коннект не открывает, так что лишнего соединения с + БД tradein не появляется. + """ + if settings.identity_store != "auth": + yield db + return + with auth_db.auth_session() as identity_db: + yield identity_db + + +def to_access_state(value: object) -> AccessState: + """Приводит значение колонки состояния доступа к `AccessState`. + + ЕДИНСТВЕННОЕ место, где булев `tradein_users.is_active` превращается в + трёхзначное состояние: True → `active`, False → `disabled` (жёсткая + блокировка, generic 401 — ровно то, что булева схема и означала). + `trial_expired` в булевой схеме выразить нечем: состояния там не + существовало, и на tradein-пути оно не появится. + + Fail-closed: неизвестная строка, NULL и любой неожиданный тип → `disabled` + + WARNING. Обратный выбор (пускать всё, что не `disabled`) означал бы, что + новое состояние, добавленное миграцией раньше кода, молча раздаёт доступ. + """ + if isinstance(value, bool): + return AccessState.ACTIVE if value else AccessState.DISABLED + if isinstance(value, str): + try: + return AccessState(value) + except ValueError: + logger.warning( + "identity_store: неизвестное состояние доступа %r → трактую как disabled", value + ) + return AccessState.DISABLED + logger.warning( + "identity_store: состояние доступа %r неожиданного типа %s → трактую как disabled", + value, + type(value).__name__, + ) + return AccessState.DISABLED + + +def access_state_param(state: AccessState) -> bool | str: + """Значение для ЗАПИСИ в `schema.access_state_column` — обратная к `to_access_state()`. + + Тип колонки разный (boolean против text), поэтому конверсию нельзя оставить + вызывающему: он бы неизбежно писал `True`/`'active'` по месту, и это ровно + то второе представление состояния, которого в коде быть не должно. + + Для булевой схемы `trial_expired` невыразим — там существуют только «пустят» + и «не пустят», и попытка записать промежуточное состояние молча стала бы + жёсткой блокировкой (клиент увидел бы «неверный пароль» вместо экрана + пробного периода). Поэтому это ошибка вызывающего, а не тихое приведение: + писать `trial_expired` можно только при `identity_store="auth"`. + """ + schema = identity_schema() + if schema.access_state_sql_type == "boolean": + if state is AccessState.TRIAL_EXPIRED: + raise ValueError( + f"состояние {state.value!r} невыразимо в схеме {schema.store!r} " + f"(колонка {schema.access_state_column} — boolean): доступны только " + f"{AccessState.ACTIVE.value!r} и {AccessState.DISABLED.value!r}" + ) + return state.can_sign_in + return state.value diff --git a/tradein-mvp/backend/tests/support/identity_modes.py b/tradein-mvp/backend/tests/support/identity_modes.py new file mode 100644 index 00000000..73c5672a --- /dev/null +++ b/tradein-mvp/backend/tests/support/identity_modes.py @@ -0,0 +1,200 @@ +"""Помощники для тестов, зависящих от того, В КАКОМ РЕЕСТРЕ живут люди. + +Эпик «единый вход»: `settings.identity_store` переключает код «Меры» между +`tradein_users`/`tradein_sessions` (БД tradein — ДЕФОЛТ, сегодняшнее поведение +прода) и `users`/`sessions` (БД `auth`). Различаются имена таблиц И тип колонки +состояния доступа (`is_active boolean` против `access_state text`). + +⚠️ ЗАЧЕМ ЭТОТ МОДУЛЬ (главная ловушка этих тестов). Интеграционные тесты +`test_auth_api.py` / `test_team_api.py` используют fake-DB, который диспатчит по +ТЕКСТУ SQL. Если ветка такого fake'а сравнивает с литералом «tradein_users», то +при `identity_store="auth"` она просто перестаёт матчиться — fake вернёт пустой +результат вместо строки, а тест останется ЗЕЛЁНЫМ на сломанном коде. Поэтому: + + * имена для матчинга берутся из `identity_schema()` (`sql_names()` ниже) — + ровно оттуда же, откуда их берёт продакшн-код; + * непонятый SQL в fake'ах ОБЯЗАН падать `AssertionError`, а не возвращать + пустоту (см. `raise AssertionError(f"unhandled fake SQL ...")` в обоих + файлах) — это то, что превращает «ветка отвалилась» в красный тест. + +`column_value()` — намеренно ЛИТЕРАЛЬНАЯ таблица «состояние → значение +колонки», а НЕ вызов `identity_store.access_state_param()`. Fake обязан хранить +то, что реально лежало бы в Postgres; если бы он звал ту же production-функцию, +что и проверяемый код, её инверсия (`active` ↔ `disabled`) прошла бы round-trip +через fake незамеченной, и тест бы не покраснел. +""" + +from __future__ import annotations + +import re +from collections.abc import Callable, Iterator +from contextlib import contextmanager +from dataclasses import dataclass +from typing import Any + +import pytest + +from app.core import auth_db, config +from app.services import identity_store +from app.services.identity_store import AccessState, identity_schema + +# Оба допустимых значения `IDENTITY_STORE` (Literal в pydantic-настройках). +# "tradein" ПЕРВЫЙ — это дефолт и путь прода; при чтении вывода pytest'а первый +# параметр всегда «как сейчас», второй — «после переезда». +IDENTITY_MODES = ("tradein", "auth") + +# Состояние доступа → значение, которое реально лежит в колонке реестра. +# Литералы, независимые от production-кода (см. модульный docstring). +# `trial_expired` в булевой схеме ОТСУТСТВУЕТ: состояния «пробный период истёк» +# там не существовало, выразить его нечем — тесты про него имеют смысл только в +# режиме `auth`, поэтому здесь явная ошибка вместо тихого приведения к False. +_COLUMN_VALUE: dict[tuple[str, AccessState], bool | str] = { + ("tradein", AccessState.ACTIVE): True, + ("tradein", AccessState.DISABLED): False, + ("auth", AccessState.ACTIVE): "active", + ("auth", AccessState.TRIAL_EXPIRED): "trial_expired", + ("auth", AccessState.DISABLED): "disabled", +} + + +def column_value(state: AccessState) -> bool | str: + """Значение состояния *state* в колонке реестра для ТЕКУЩЕГО режима.""" + store = config.settings.identity_store + try: + return _COLUMN_VALUE[(store, state)] + except KeyError: + raise AssertionError( + f"состояние {state.value!r} не существует в схеме {store!r} — " + f"такой тест имеет смысл только при identity_store='auth'" + ) from None + + +@dataclass(frozen=True, slots=True) +class SqlNames: + """Имена, по которым fake-DB узнаёт запрос в ТЕКУЩЕМ режиме.""" + + users: str + sessions: str + access_state_column: str + access_state_sql_type: str + + +def sql_names() -> SqlNames: + """Имена таблиц/колонки из `identity_schema()` — источник тот же, что у кода.""" + schema = identity_schema() + return SqlNames( + users=schema.users_table, + sessions=schema.sessions_table, + access_state_column=schema.access_state_column, + access_state_sql_type=schema.access_state_sql_type, + ) + + +def assert_reads_access_state(sql: str, names: SqlNames) -> None: + """Запрос, читающий состояние доступа, ОБЯЗАН брать колонку ТЕКУЩЕГО режима. + + Ставится в те ветки fake-DB, которые отдают строку человека. Без неё fake + остаётся ЗЕЛЁНЫМ на захардкоженном `is_active AS access_state`: строку он + собирает из `_Store`, где ключ УЖЕ называется `access_state`, и про имя + колонки в SELECT'е ничего не знает — то есть запрос, невозможный на реальном + Postgres (`column "is_active" does not exist` в БД `auth`), проехал бы молча. + + Измерено мутацией: захардкодить колонку в `team._employee_columns` — без + этой проверки все 48 тестов «Команды» остаются зелёными; с ней ветка + перестаёт матчиться, SQL доезжает до `raise AssertionError` в конце + `execute` и тесты краснеют. + + Алиас проверяется отдельно от имени колонки: без `AS access_state` + вызывающий код читал бы то `is_active`, то `access_state`, то есть завёл бы + второе представление состояния — ровно то, чего эпик не допускает. + """ + expected = f"{names.access_state_column} AS access_state" + if expected not in sql: + raise AssertionError( + f"запрос к реестру не читает колонку состояния текущего режима " + f"({expected!r}): {sql!r}" + ) + + +def assert_insert_writes_access_state(sql: str, names: SqlNames) -> None: + """INSERT в реестр обязан перечислять колонку состояния ТЕКУЩЕГО режима. + + Проверяется именно СПИСОК КОЛОНОК, а не наличие подстроки: bind-параметр + называется `:access_state` в обоих режимах, поэтому `... , :access_state)` + в VALUES матчился бы всегда — и `INSERT INTO users (..., is_active)` + (невозможный в БД `auth`) проехал бы молча. Измерено мутацией. + """ + match = re.search(rf"INSERT INTO\s+{re.escape(names.users)}\s*\(([^)]*)\)", sql) + if match is None: + raise AssertionError(f"не разобрал список колонок INSERT'а в реестр: {sql!r}") + columns = {c.strip() for c in match.group(1).split(",")} + if names.access_state_column not in columns: + raise AssertionError( + f"INSERT в реестр не пишет колонку состояния текущего режима " + f"({names.access_state_column!r}); в списке: {sorted(columns)}" + ) + + +def assert_update_writes_access_state(sql: str, names: SqlNames) -> None: + """UPDATE реестра обязан присваивать колонку состояния ТЕКУЩЕГО режима — и + кастовать параметр в ЕЁ тип. + + CAST здесь несущий: параметр может быть NULL («поле не пришло в PATCH» → + `COALESCE(CAST(:x AS T), col)`), и без явного типа Postgres тип NULL-параметра + не выведет. Захардкоженный `boolean` в текстовой схеме — ошибка уровня БД, + которую fake иначе не увидел бы. + """ + assignment = f"{names.access_state_column} = COALESCE(" + if assignment not in sql: + raise AssertionError( + f"UPDATE реестра не присваивает колонку состояния текущего режима " + f"({assignment!r}): {sql!r}" + ) + cast = f"CAST(:access_state AS {names.access_state_sql_type})" + if cast not in sql: + raise AssertionError( + f"UPDATE реестра кастует состояние не в тип текущей схемы ({cast!r}): {sql!r}" + ) + + +def use_identity_mode(monkeypatch: pytest.MonkeyPatch, mode: str) -> str: + """Переключает реестр на *mode* на время теста. + + `reset_auth_db()` — на случай, если предыдущий тест успел построить engine + БД `auth`: закешированный engine пережил бы monkeypatch настроек (он живёт в + module-global, а не в `settings`) и утёк бы сюда. + """ + auth_db.reset_auth_db() + monkeypatch.setattr(config.settings, "identity_store", mode) + return mode + + +def patch_identity_sessions(monkeypatch: pytest.MonkeyPatch, make_db: Callable[[], Any]) -> None: + """Подменяет ОБА источника сессии реестра так, чтобы работал РЕАЛЬНЫЙ + `identity_store.identity_session()` / `get_identity_db()`, а не их копия + в тесте. + + Точки подмены выбраны настолько «низко», насколько возможно: + * `identity_store.SessionLocal` — то, что открывает `identity_session()` + в режиме "tradein" (импортирован по имени, поэтому патчим в + `identity_store`, а не в `app.core.db`); + * `auth_db.auth_session` — то, что открывают `identity_session()` и + `get_identity_db()` в режиме "auth" (`identity_store` держит ссылку на + МОДУЛЬ `auth_db`, поэтому подмена атрибута модуля видна ему сразу). + + Благодаря этому ветвление по режиму остаётся на production-коде: тест не + повторяет его у себя, и регрессия в `get_identity_db` (например, если он + перестанет отдавать в режиме "tradein" тот же объект `Session`, что и + `get_db`) не сможет спрятаться за тестовым дублёром. + + *make_db* вызывается БЕЗ аргументов и обязан отдавать новый fake-Session, + поддерживающий `with ... as db` (как настоящая `Session`). + """ + + @contextmanager + def _fake_auth_session() -> Iterator[Any]: + with make_db() as db: + yield db + + monkeypatch.setattr(identity_store, "SessionLocal", make_db) + monkeypatch.setattr(auth_db, "auth_session", _fake_auth_session) diff --git a/tradein-mvp/backend/tests/test_auth_api.py b/tradein-mvp/backend/tests/test_auth_api.py index 23997f98..6f3d3da1 100644 --- a/tradein-mvp/backend/tests/test_auth_api.py +++ b/tradein-mvp/backend/tests/test_auth_api.py @@ -3,19 +3,36 @@ and rbac_guard session-cookie resolution. Uses the REAL `rbac_guard` (app.core.rbac) + REAL `auth.router` / `me.router` wired into an isolated FastAPI test app (same pattern as tests/test_rbac.py), with an -in-memory fake DB standing in for `tradein_users`/`tradein_sessions`: - - `app.core.rbac.SessionLocal` is monkeypatched (rbac_guard opens its own session, - it's middleware — no FastAPI DI available there). - - `app.core.db.get_db` is overridden via `app.dependency_overrides` (auth.py / - me.py use `Depends(get_db)`, the idiomatic FastAPI-testable path). +in-memory fake DB standing in for the identity registry: + - сессия РЕЕСТРА подменяется на самом низком уровне — `identity_store.SessionLocal` + и `auth_db.auth_session` (см. `tests.support.identity_modes.patch_identity_sessions`), + так что и `identity_session()` (rbac_guard — middleware, FastAPI-DI там нет), и + `Depends(get_identity_db)` (auth.py / me.py) выполняются РЕАЛЬНЫЕ, вместе со своим + ветвлением по `settings.identity_store`; + - `app.core.db.get_db` переопределён через `app.dependency_overrides` — это + продуктовая БД (в дефолтном режиме она же и реестр). -Both point at the SAME `_Store` instance per test, so a session created by POST -/login is immediately visible to rbac_guard's own DB round trip on the next request. +Все они смотрят в ОДИН `_Store` на тест, поэтому сессия, созданная POST /login, +сразу видна собственному DB-раунд-трипу rbac_guard'а на следующем запросе. + +⚠️ ДВА РЕЖИМА РЕЕСТРА И ЛОВУШКА FAKE-DB. `_FakeDB` диспатчит по ТЕКСТУ SQL, а +эпик «единый вход» переименовывает таблицы (`tradein_users`/`tradein_sessions` → +`users`/`sessions`) и меняет тип колонки состояния доступа. Литерал +«tradein_users» в диспатчере означал бы, что при `IDENTITY_STORE=auth` ветка +молча перестаёт матчиться, fake отдаёт пустоту, а тест остаётся ЗЕЛЁНЫМ на +сломанном коде. Поэтому имена берутся из `sql_names()` (= `identity_schema()`, +тот же словарь, что у продакшн-кода), а непонятый SQL падает `AssertionError`, +а не возвращает пустой результат. + +Дефолт (`identity_store="tradein"`) — сегодняшний прод; тесты без фикстуры +`auth_store` идут именно в нём. Тесты про режим `auth` (в т.ч. про состояние +`trial_expired`, невыразимое булевым `is_active`) — в конце файла. """ from __future__ import annotations import os +import re from datetime import UTC, datetime, timedelta from types import SimpleNamespace from typing import Annotated, Any @@ -29,13 +46,21 @@ from fastapi.testclient import TestClient from app.api.v1 import auth as auth_router from app.api.v1 import me as me_router from app.core import auth as auth_mod -from app.core import config +from app.core import auth_db, config from app.core.db import get_db from app.core.password import hash_password from app.core.rbac import rbac_guard +from app.services.identity_store import AccessState +from tests.support.identity_modes import ( + assert_reads_access_state, + column_value, + patch_identity_sessions, + sql_names, + use_identity_mode, +) # --------------------------------------------------------------------------- -# Fake DB backing tradein_users / tradein_sessions +# Fake DB backing the identity registry (users/sessions таблицы текущего режима) # --------------------------------------------------------------------------- @@ -43,6 +68,7 @@ class _Store: def __init__(self) -> None: self.users: dict[str, dict[str, Any]] = {} self.sessions: dict[str, dict[str, Any]] = {} + self.sql_log: list[str] = [] # весь SQL, доехавший до «БД» — см. тесты режимов self._next_id = 1 def add_user( @@ -51,7 +77,7 @@ class _Store: password_hash: str | None, *, role: str = "employee", - is_active: bool = True, + access_state: AccessState = AccessState.ACTIVE, display_name: str | None = "Alice A.", org_name: str | None = "Org LLC", email: str | None = "alice@example.com", @@ -63,13 +89,19 @@ class _Store: "username": username, "password_hash": password_hash, "role": role, - "is_active": is_active, + # СЫРОЕ значение колонки текущего режима (boolean либо text) — ровно + # то, что вернул бы драйвер; в AccessState его превращает код. + "access_state": column_value(access_state), "display_name": display_name, "org_name": org_name, "email": email, } return uid + def set_access_state(self, username: str, state: AccessState) -> None: + """Меняет состояние доступа уже заведённого юзера (как сделал бы админ/миграция).""" + self.users[username]["access_state"] = column_value(state) + def user_by_id(self, uid: int) -> dict[str, Any] | None: for u in self.users.values(): if u["id"] == uid: @@ -109,8 +141,12 @@ class _FakeDB: def execute(self, stmt: object, params: dict[str, Any] | None = None) -> SimpleNamespace: sql = str(stmt) p = params or {} + # Имена таблиц берутся ИЗ КОДА (identity_schema), а не из литералов — + # см. «ЛОВУШКА FAKE-DB» в модульном docstring. + names = sql_names() + self.store.sql_log.append(sql) - if "INSERT INTO tradein_sessions" in sql: + if f"INSERT INTO {names.sessions}" in sql: now = datetime.now(UTC) self.store.sessions[p["token"]] = { "user_id": p["user_id"], @@ -119,7 +155,7 @@ class _FakeDB: } return SimpleNamespace(fetchone=lambda: None) - if "UPDATE tradein_sessions" in sql and "SET last_seen_at" in sql: + if f"UPDATE {names.sessions}" in sql and "SET last_seen_at" in sql: sess = self.store.sessions.get(p["token"]) if sess is not None: now = datetime.now(UTC) @@ -127,23 +163,27 @@ class _FakeDB: sess["expires_at"] = now + timedelta(hours=p["ttl_hours"]) return SimpleNamespace(fetchone=lambda: None) - if "DELETE FROM tradein_sessions WHERE token" in sql: + if f"DELETE FROM {names.sessions} WHERE token" in sql: self.store.sessions.pop(p["token"], None) return SimpleNamespace(fetchone=lambda: None) - if "DELETE FROM tradein_sessions WHERE user_id" in sql: + if f"DELETE FROM {names.sessions} WHERE user_id" in sql: uid = p["user_id"] for tok in [t for t, s in self.store.sessions.items() if s["user_id"] == uid]: del self.store.sessions[tok] return SimpleNamespace(fetchone=lambda: None) - if "FROM tradein_sessions s" in sql and "JOIN tradein_users u" in sql: + if f"FROM {names.sessions} s" in sql and f"JOIN {names.users} u" in sql: + assert_reads_access_state(sql, names) sess = self.store.sessions.get(p["token"]) if sess is None: return SimpleNamespace(fetchone=lambda: None) user = self.store.user_by_id(sess["user_id"]) if user is None: return SimpleNamespace(fetchone=lambda: None) + # Колонка состояния приезжает под алиасом `access_state` в ОБОИХ + # режимах (`u.<колонка> AS access_state` в реальном SELECT'е); + # значение — сырое, типа своей схемы. row = SimpleNamespace( user_id=sess["user_id"], expires_at=sess["expires_at"], @@ -153,11 +193,12 @@ class _FakeDB: display_name=user["display_name"], org_name=user["org_name"], email=user["email"], - is_active=user["is_active"], + access_state=user["access_state"], ) return SimpleNamespace(fetchone=lambda: row) - if "FROM tradein_users" in sql: + if f"FROM {names.users}" in sql and "WHERE username = :username" in sql: + assert_reads_access_state(sql, names) user = self.store.users.get(p["username"]) if user is None: return SimpleNamespace(fetchone=lambda: None) @@ -214,6 +255,9 @@ 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") + # Каждый тест стартует в ДЕФОЛТНОМ режиме реестра (сегодняшний прод), даже + # если предыдущий переключался на `auth`. + use_identity_mode(monkeypatch, "tradein") @pytest.fixture @@ -221,9 +265,23 @@ def store() -> _Store: return _Store() +@pytest.fixture +def auth_store(store: _Store, monkeypatch: pytest.MonkeyPatch) -> _Store: + """Тот же `store`, но реестр — БД `auth` (`users`/`sessions`, text-состояние). + + Запрашивай ПЕРЕД `client` в списке аргументов теста: `client` строится уже с + учётом режима (`_build_test_app` читает его лениво, но `store.add_user` + сохраняет значение колонки по режиму НА МОМЕНТ ВЫЗОВА). + """ + use_identity_mode(monkeypatch, "auth") + return store + + @pytest.fixture def client(store: _Store, monkeypatch: pytest.MonkeyPatch) -> TestClient: - monkeypatch.setattr("app.core.rbac.SessionLocal", lambda: _FakeDB(store)) + # Подменяем сессию РЕЕСТРА на обоих её источниках сразу, а не ветвление по + # режиму: `identity_session()` / `get_identity_db()` остаются настоящими. + patch_identity_sessions(monkeypatch, lambda: _FakeDB(store)) # base_url=https:// — login sets the session cookie with Secure=True (real prod # behaviour, not weakened for tests); httpx's cookie jar silently drops Secure # cookies on a plain-http connection, so a plain http://testserver client would @@ -280,7 +338,9 @@ def test_login_unknown_username_401_generic_message(client: TestClient) -> None: def test_login_inactive_user_401(client: TestClient, store: _Store) -> None: - store.add_user("bob", hash_password("Secret123!"), role="employee", is_active=False) + store.add_user( + "bob", hash_password("Secret123!"), role="employee", access_state=AccessState.DISABLED + ) resp = client.post("/api/v1/auth/login", json={"username": "bob", "password": "Secret123!"}) assert resp.status_code == 401 @@ -611,3 +671,193 @@ def test_cyrillic_username_session_propagation_does_not_500( # latin-1 "replace" гарантированно не крашит — точное значение (что именно # получится из non-latin1 байт) не является контрактом, важно отсутствие 500. assert resp.json()["user"] is not None + + +# --------------------------------------------------------------------------- +# Эпик «единый вход»: режим IDENTITY_STORE=auth (общий реестр в БД `auth`). +# +# Всё выше идёт в ДЕФОЛТНОМ режиме — он же прод — и служит регрессионным +# доказательством «после мержа работает точно как сейчас». Ниже — поведение, +# которое появляется ТОЛЬКО после переезда: трёхзначное состояние доступа +# (`active` / `trial_expired` / `disabled`) вместо булева `is_active`. +# --------------------------------------------------------------------------- + + +def test_default_mode_talks_to_tradein_tables_only(client: TestClient, store: _Store) -> None: + """Дефолт трогает РОВНО сегодняшние таблицы — и ни одной таблицы реестра `auth`. + + Пин на случай, если флаг когда-нибудь начнёт «протекать» (например, дефолт + поменяют или ветвление уедет не туда): расхождение здесь означало бы, что + прод после мержа пошёл в другую БД. + """ + store.add_user("alice", hash_password("Secret123!"), role="employee") + client.post("/api/v1/auth/login", json={"username": "alice", "password": "Secret123!"}) + assert client.get("/api/v1/me").status_code == 200 + + joined = "\n".join(store.sql_log) + assert "tradein_users" in joined + assert "tradein_sessions" in joined + # Ни один запрос не адресован таблицам общего реестра. + assert not re.search(r"\b(FROM|INTO|UPDATE|JOIN)\s+users\b", joined) + assert not re.search(r"\b(FROM|INTO|UPDATE|JOIN)\s+sessions\b", joined) + # И engine БД `auth` даже не создавался (AUTH_DATABASE_URL на проде пуст — + # ленивое построение обязано не случиться, иначе запрос упал бы). + assert auth_db._engine is None + + +def test_auth_mode_talks_to_shared_registry_tables(auth_store: _Store, client: TestClient) -> None: + """Зеркало предыдущего: при IDENTITY_STORE=auth запросы уходят в users/sessions.""" + auth_store.add_user("alice", hash_password("Secret123!"), role="employee") + resp = client.post("/api/v1/auth/login", json={"username": "alice", "password": "Secret123!"}) + assert resp.status_code == 200, resp.text + assert client.get("/api/v1/me").status_code == 200 + + joined = "\n".join(auth_store.sql_log) + assert "tradein_users" not in joined + assert "tradein_sessions" not in joined + assert re.search(r"FROM\s+users\b", joined) + assert re.search(r"INSERT INTO\s+sessions\b", joined) + + +def test_login_trial_expired_403_with_code_and_no_session( + auth_store: _Store, client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ВЕРНЫЙ пароль + `trial_expired` → 403 с машиночитаемым кодом, сессии НЕТ. + + Единственный не-generic ответ логина: аккаунт существует и владелец это уже + доказал паролем, так что осмысленный текст постороннему ничего не выдаёт. + """ + auth_store.add_user( + "trialguy", + hash_password("Secret123!"), + role="employee", + access_state=AccessState.TRIAL_EXPIRED, + ) + events: list[dict[str, Any]] = [] + monkeypatch.setattr(auth_router, "schedule_event", lambda **kw: events.append(kw)) + + resp = client.post( + "/api/v1/auth/login", json={"username": "trialguy", "password": "Secret123!"} + ) + + assert resp.status_code == 403, resp.text + detail = resp.json()["detail"] + # Контракт для фронта — `code`, а не текст сообщения. + assert detail["code"] == "access_expired" + assert detail["message"] + # Сессия не выдана: ни куки, ни строки в реестре. + assert config.settings.session_cookie_name not in resp.cookies + assert auth_store.sessions == {} + assert [e["event_type"] for e in events] == ["login_blocked_expired"] + + +def test_login_wrong_password_on_trial_expired_is_generic_401( + auth_store: _Store, client: TestClient +) -> None: + """НЕверный пароль на `trial_expired` → тот же generic 401, что у чужого логина. + + Иначе отдельный 403 превращается в оракул существования аккаунта: перебором + можно было бы перечислить логины, не зная ни одного пароля. + """ + auth_store.add_user( + "trialguy", + hash_password("Secret123!"), + role="employee", + access_state=AccessState.TRIAL_EXPIRED, + ) + + wrong_pw = client.post("/api/v1/auth/login", json={"username": "trialguy", "password": "nope"}) + ghost = client.post("/api/v1/auth/login", json={"username": "ghost", "password": "nope"}) + + assert wrong_pw.status_code == 401 + # Побайтово тот же ответ, что и на несуществующий логин. + assert wrong_pw.json() == ghost.json() + assert auth_store.sessions == {} + + +def test_login_disabled_is_generic_401_not_403(auth_store: _Store, client: TestClient) -> None: + """`disabled` + верный пароль → generic 401, НЕ 403: заблокированный аккаунт + для пользователя неотличим от несуществующего (в отличие от `trial_expired`, + у которого есть свой экран).""" + auth_store.add_user( + "blocked", + hash_password("Secret123!"), + role="employee", + access_state=AccessState.DISABLED, + ) + + blocked = client.post( + "/api/v1/auth/login", json={"username": "blocked", "password": "Secret123!"} + ) + ghost = client.post("/api/v1/auth/login", json={"username": "ghost", "password": "x"}) + + assert blocked.status_code == 401 + assert blocked.json() == ghost.json() + assert auth_store.sessions == {} + + +def test_unknown_access_state_is_fail_closed_401(auth_store: _Store, client: TestClient) -> None: + """Состояние, которого код не знает (миграция уехала вперёд кода), НЕ пускает.""" + auth_store.add_user("newbie", hash_password("Secret123!"), role="employee") + auth_store.users["newbie"]["access_state"] = "pending_review" + + resp = client.post("/api/v1/auth/login", json={"username": "newbie", "password": "Secret123!"}) + + assert resp.status_code == 401 + assert auth_store.sessions == {} + + +@pytest.mark.parametrize("state", [AccessState.TRIAL_EXPIRED, AccessState.DISABLED]) +def test_live_session_dies_when_access_state_leaves_active( + auth_store: _Store, client: TestClient, state: AccessState +) -> None: + """Уже выданная сессия перестаёт работать СРАЗУ, как только состояние != active. + + Без этого sliding-refresh (`get_session_user` продлевает expires_at на каждом + запросе) держал бы сессию истёкшего/заблокированного бесконечно долго. + """ + auth_store.add_user("alice", hash_password("Secret123!"), role="employee") + login = client.post("/api/v1/auth/login", json={"username": "alice", "password": "Secret123!"}) + assert login.status_code == 200 + assert client.get("/api/v1/trade-in/dummy").status_code == 200 + + auth_store.set_access_state("alice", state) + + # auth_mode=dual, но legacy-заголовка нет → сессия больше не резолвится → 401. + assert client.get("/api/v1/trade-in/dummy").status_code == 401 + assert client.get("/api/v1/me").status_code == 401 + + +def test_session_identity_wins_over_spoofed_header_auth_store( + auth_store: _Store, client: TestClient +) -> None: + """Перезапись X-Authenticated-User в ASGI-scope работает и на общем реестре. + + Тот же CRITICAL, что и в дефолтном режиме (см. выше): подделанный клиентом + заголовок не должен выигрывать у резолвленной сессии ни в одном режиме — эти + ~15 downstream-хендлеров читают сырой заголовок и про режим ничего не знают. + """ + auth_store.add_user("alice", hash_password("Secret123!"), role="employee") + auth_store.add_user("victim", hash_password("Secret123!"), role="employee") + client.post("/api/v1/auth/login", json={"username": "alice", "password": "Secret123!"}) + + resp = client.get("/api/v1/trade-in/whoami", headers={"X-Authenticated-User": "victim"}) + + assert resp.status_code == 200 + assert resp.json()["user"] == "alice" + + +def test_auth_mode_role_scope_and_logout(auth_store: _Store, client: TestClient) -> None: + """Роль/скоуп и logout на общем реестре ведут себя как в дефолтном режиме.""" + auth_store.add_user("mgr", hash_password("Secret123!"), role="manager") + login = client.post("/api/v1/auth/login", json={"username": "mgr", "password": "Secret123!"}) + token = login.cookies[config.settings.session_cookie_name] + assert token in auth_store.sessions + + body = client.get("/api/v1/me").json() + assert body["role"] == "manager" + assert "/api/v1/team/**" in body["allowed_paths"] + assert "/trade-in/sale-share/**" in body["deny_paths"] + + assert client.post("/api/v1/auth/logout").status_code == 200 + assert token not in auth_store.sessions diff --git a/tradein-mvp/backend/tests/test_auth_session.py b/tradein-mvp/backend/tests/test_auth_session.py index b45ef98a..4595b6b9 100644 --- a/tradein-mvp/backend/tests/test_auth_session.py +++ b/tradein-mvp/backend/tests/test_auth_session.py @@ -2,15 +2,28 @@ Coverage: - create_session: INSERT with CAST(...) (never `:x::type`), commit, unique tokens. - - get_session_user: valid/expired/inactive/missing-row + sliding refresh (only when + - get_session_user: valid/expired/не-active/missing-row + sliding refresh (only when last_seen_at is stale, best-effort — a refresh failure still returns the user). - - get_user_by_username: found/not-found. + - get_user_by_username: found/not-found + состояние доступа как `AccessState`. - revoke_session / revoke_user_sessions: DELETE + commit. - get_db_role_scope: employee/manager/admin/unknown mapping. All functions here take `db: Session` as a plain argument (no SessionLocal() opened internally) — unit tests just pass a hand-rolled fake, mirroring the `_FakeSession` pattern from tests/test_user_events.py but adapted for `.fetchone()`-based reads. + +⚠️ ОБА РЕЖИМА РЕЕСТРА. Эпик «единый вход» вынес имена таблиц и имя/тип колонки +состояния доступа в `identity_store.identity_schema()`. Тесты, которые вообще +трогают SQL, прогоняются в ОБОИХ режимах (фикстура `identity_mode`): "tradein" +(дефолт, сегодняшний прод — `tradein_users`/`tradein_sessions`, boolean +`is_active`) и "auth" (`users`/`sessions`, text `access_state`). Ожидаемые имена +в ассертах берутся из `identity_schema()` — из того же словаря, что и у кода, +поэтому переименование таблиц не «разъезжает» тест с реальностью тихо; +поломка запроса ловится тем, что fake отдаёт строку ТОЛЬКО на ожидаемый SQL, +а сам SQL проверяется явными ассертами ниже. + +Тесты БЕЗ фикстуры `identity_mode` намеренно идут в дефолтном режиме +(`_default_identity_mode` autouse) — это чистая логика без SQL. """ from __future__ import annotations @@ -23,7 +36,34 @@ from typing import Any os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") +import pytest + from app.services import auth_session as svc +from app.services.identity_store import AccessState, identity_schema +from tests.support.identity_modes import IDENTITY_MODES, column_value, use_identity_mode + +# --------------------------------------------------------------------------- +# Режим реестра +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _default_identity_mode(monkeypatch: pytest.MonkeyPatch) -> None: + """Каждый тест стартует в ДЕФОЛТНОМ режиме, даже если предыдущий его менял.""" + use_identity_mode(monkeypatch, "tradein") + + +@pytest.fixture(params=IDENTITY_MODES) +def identity_mode(request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch) -> str: + """Тест прогоняется дважды: "tradein" (прод) и "auth" (после переезда).""" + return use_identity_mode(monkeypatch, request.param) + + +@pytest.fixture +def auth_mode(monkeypatch: pytest.MonkeyPatch) -> str: + """Только режим "auth" — для состояний, невыразимых булевой колонкой.""" + return use_identity_mode(monkeypatch, "auth") + # --------------------------------------------------------------------------- # Fake DB session @@ -58,6 +98,11 @@ class _FakeDB: self.rolled_back += 1 +# Часовой «аргумент не передан» — None здесь занят (это валидное сырое значение +# колонки: NULL, который to_access_state обязан трактовать как disabled). +_MISSING = object() + + def _session_row( *, user_id: int = 1, @@ -65,8 +110,17 @@ def _session_row( last_seen_at: datetime | None = None, username: str = "alice", role: str = "employee", - is_active: bool = True, + access_state: AccessState = AccessState.ACTIVE, + raw_access_state: object = _MISSING, ) -> SimpleNamespace: + """Строка JOIN'а sessions×users, как её отдал бы драйвер. + + Колонка состояния всегда приезжает под алиасом `access_state` (`AS access_state` + в реальном SELECT'е), а ЗНАЧЕНИЕ в ней — то, что лежит в БД текущего режима: + boolean для `tradein_users.is_active`, text для `auth.users.access_state`. + *raw_access_state* — обход таблицы состояний для проверки fail-closed на + значении, которого код не знает. + """ now = datetime.now(UTC) return SimpleNamespace( user_id=user_id, @@ -77,7 +131,9 @@ def _session_row( display_name="Alice A.", org_name="Org LLC", email="alice@example.com", - is_active=is_active, + access_state=( + column_value(access_state) if raw_access_state is _MISSING else raw_access_state + ), ) @@ -87,14 +143,14 @@ def _user_row( username: str = "alice", password_hash: str | None = "hash", role: str = "employee", - is_active: bool = True, + access_state: AccessState = AccessState.ACTIVE, ) -> SimpleNamespace: return SimpleNamespace( id=user_id, username=username, password_hash=password_hash, role=role, - is_active=is_active, + access_state=column_value(access_state), display_name="Alice A.", org_name="Org LLC", email="alice@example.com", @@ -106,14 +162,14 @@ def _user_row( # --------------------------------------------------------------------------- -def test_create_session_inserts_and_commits() -> None: +def test_create_session_inserts_and_commits(identity_mode: str) -> None: db = _FakeDB() token = svc.create_session(db, user_id=42, ip="1.2.3.4", user_agent="pytest") assert db.committed == 1 assert len(db.executed) == 1 sql, params = db.executed[0] - assert "INSERT INTO tradein_sessions" in sql + assert f"INSERT INTO {identity_schema().sessions_table}" in sql assert params is not None assert params["user_id"] == 42 assert params["ip"] == "1.2.3.4" @@ -123,7 +179,7 @@ def test_create_session_inserts_and_commits() -> None: assert len(token) >= 32 -def test_create_session_cast_not_doublecolon() -> None: +def test_create_session_cast_not_doublecolon(identity_mode: str) -> None: db = _FakeDB() svc.create_session(db, user_id=1) sql, _ = db.executed[0] @@ -150,16 +206,20 @@ def test_get_session_user_no_token_returns_none() -> None: assert db.executed == [] -def test_get_session_user_missing_row_returns_none() -> None: +def test_get_session_user_missing_row_returns_none(identity_mode: str) -> None: + schema = identity_schema() db = _FakeDB(rows=[None]) assert svc.get_session_user(db, "tok") is None sql, params = db.executed[0] - assert "FROM tradein_sessions s" in sql - assert "JOIN tradein_users u" in sql + assert f"FROM {schema.sessions_table} s" in sql + assert f"JOIN {schema.users_table} u" in sql + # Колонка состояния — под именем текущей схемы и обязательно с алиасом: + # без него вызывающий код читал бы то `is_active`, то `access_state`. + assert f"u.{schema.access_state_column} AS access_state" in sql assert params == {"token": "tok"} -def test_get_session_user_expired_returns_none() -> None: +def test_get_session_user_expired_returns_none(identity_mode: str) -> None: now = datetime.now(UTC) db = _FakeDB(rows=[_session_row(expires_at=now - timedelta(minutes=1))]) assert svc.get_session_user(db, "tok") is None @@ -167,13 +227,36 @@ def test_get_session_user_expired_returns_none() -> None: assert len(db.executed) == 1 -def test_get_session_user_inactive_returns_none() -> None: - db = _FakeDB(rows=[_session_row(is_active=False)]) +def test_get_session_user_disabled_returns_none(identity_mode: str) -> None: + """Жёстко заблокированный аккаунт — сессия недействительна в обеих схемах.""" + db = _FakeDB(rows=[_session_row(access_state=AccessState.DISABLED)]) assert svc.get_session_user(db, "tok") is None assert len(db.executed) == 1 -def test_get_session_user_valid_recent_no_refresh() -> None: +def test_get_session_user_trial_expired_returns_none(auth_mode: str) -> None: + """Пробный период истёк — УЖЕ ВЫДАННАЯ сессия гасится немедленно. + + Иначе сотрудник, залогиненный до истечения пробного доступа, продолжал бы + работать, а sliding-refresh продлевал бы ему `expires_at` бесконечно — + состояние `trial_expired` не наступило бы для него никогда. + """ + db = _FakeDB(rows=[_session_row(access_state=AccessState.TRIAL_EXPIRED)]) + assert svc.get_session_user(db, "tok") is None + # Ни UPDATE (sliding refresh), ни commit — сессия не продлевается. + assert len(db.executed) == 1 + assert db.committed == 0 + + +def test_get_session_user_unknown_state_returns_none(auth_mode: str) -> None: + """Fail-closed: состояние, которого код не знает (миграция впереди кода), + НЕ пускает. Обратный выбор молча раздавал бы доступ по новому значению.""" + db = _FakeDB(rows=[_session_row(raw_access_state="pending_review")]) + assert svc.get_session_user(db, "tok") is None + assert len(db.executed) == 1 + + +def test_get_session_user_valid_recent_no_refresh(identity_mode: str) -> None: """last_seen_at свежий (<5 мин) — sliding refresh НЕ триггерится.""" now = datetime.now(UTC) db = _FakeDB(rows=[_session_row(last_seen_at=now - timedelta(minutes=1))]) @@ -186,12 +269,15 @@ def test_get_session_user_valid_recent_no_refresh() -> None: assert result["org_name"] == "Org LLC" assert result["email"] == "alice@example.com" assert result["user_id"] == 1 + # Состояние доступа приезжает ЕДИНЫМ понятием, а не boolean/str по режимам; + # сюда доходит только ACTIVE (не-active отсеян выше). + assert result["access_state"] is AccessState.ACTIVE # Только 1 execute (SELECT) — никакого UPDATE. assert len(db.executed) == 1 assert db.committed == 0 -def test_get_session_user_stale_last_seen_triggers_refresh() -> None: +def test_get_session_user_stale_last_seen_triggers_refresh(identity_mode: str) -> None: """last_seen_at старше 5 минут — один UPDATE (sliding refresh) + commit.""" now = datetime.now(UTC) db = _FakeDB(rows=[_session_row(last_seen_at=now - timedelta(minutes=10))]) @@ -200,7 +286,7 @@ def test_get_session_user_stale_last_seen_triggers_refresh() -> None: assert result is not None assert len(db.executed) == 2 update_sql, update_params = db.executed[1] - assert "UPDATE tradein_sessions" in update_sql + assert f"UPDATE {identity_schema().sessions_table}" in update_sql assert "SET last_seen_at" in update_sql assert not re.search(r":\w+::\w", update_sql) assert "CAST(:ttl_hours AS integer)" in update_sql @@ -208,7 +294,7 @@ def test_get_session_user_stale_last_seen_triggers_refresh() -> None: assert db.committed == 1 -def test_get_session_user_refresh_failure_is_swallowed() -> None: +def test_get_session_user_refresh_failure_is_swallowed(identity_mode: str) -> None: """Sliding-refresh UPDATE падает — всё равно возвращаем валидного юзера (best-effort refresh, не часть решения "валидна ли сессия").""" now = datetime.now(UTC) @@ -228,7 +314,8 @@ def test_get_session_user_refresh_failure_is_swallowed() -> None: # --------------------------------------------------------------------------- -def test_get_user_by_username_found() -> None: +def test_get_user_by_username_found(identity_mode: str) -> None: + schema = identity_schema() db = _FakeDB(rows=[_user_row()]) user = svc.get_user_by_username(db, "alice") @@ -236,13 +323,38 @@ def test_get_user_by_username_found() -> None: assert user["username"] == "alice" assert user["password_hash"] == "hash" assert user["role"] == "employee" - assert user["is_active"] is True + assert user["access_state"] is AccessState.ACTIVE sql, params = db.executed[0] - assert "FROM tradein_users" in sql + assert f"FROM {schema.users_table}" in sql + assert f"{schema.access_state_column} AS access_state" in sql assert params == {"username": "alice"} -def test_get_user_by_username_not_found() -> None: +def test_get_user_by_username_disabled_state_is_reported_not_hidden(identity_mode: str) -> None: + """Строка отдаётся ВСЕГДА, состояние — отдельным полем. + + Login обязан отличать «нет такого логина» (None) от «есть, но доступ закрыт» + (строка + не-ACTIVE): от этого зависит выбор события аудита, а прятать + заблокированного за None означало бы потерять эту разницу. + """ + db = _FakeDB(rows=[_user_row(access_state=AccessState.DISABLED)]) + user = svc.get_user_by_username(db, "alice") + + assert user is not None + assert user["access_state"] is AccessState.DISABLED + assert user["access_state"].can_sign_in is False + + +def test_get_user_by_username_trial_expired_state(auth_mode: str) -> None: + db = _FakeDB(rows=[_user_row(access_state=AccessState.TRIAL_EXPIRED)]) + user = svc.get_user_by_username(db, "alice") + + assert user is not None + assert user["access_state"] is AccessState.TRIAL_EXPIRED + assert user["access_state"].can_sign_in is False + + +def test_get_user_by_username_not_found(identity_mode: str) -> None: db = _FakeDB(rows=[None]) assert svc.get_user_by_username(db, "ghost") is None @@ -252,24 +364,24 @@ def test_get_user_by_username_not_found() -> None: # --------------------------------------------------------------------------- -def test_revoke_session_deletes_and_commits() -> None: +def test_revoke_session_deletes_and_commits(identity_mode: str) -> None: db = _FakeDB() svc.revoke_session(db, "tok") assert db.committed == 1 sql, params = db.executed[0] - assert "DELETE FROM tradein_sessions" in sql + assert f"DELETE FROM {identity_schema().sessions_table}" in sql assert "token" in sql assert params == {"token": "tok"} -def test_revoke_user_sessions_deletes_and_commits() -> None: +def test_revoke_user_sessions_deletes_and_commits(identity_mode: str) -> None: db = _FakeDB() svc.revoke_user_sessions(db, 7) assert db.committed == 1 sql, params = db.executed[0] - assert "DELETE FROM tradein_sessions" in sql + assert f"DELETE FROM {identity_schema().sessions_table}" in sql assert "user_id" in sql assert params == {"user_id": 7} diff --git a/tradein-mvp/backend/tests/test_identity_store.py b/tradein-mvp/backend/tests/test_identity_store.py new file mode 100644 index 00000000..59eba9f5 --- /dev/null +++ b/tradein-mvp/backend/tests/test_identity_store.py @@ -0,0 +1,481 @@ +"""Tests for app.services.identity_store + app.core.auth_db — эпик «единый вход». + +`identity_store` — единственное место, знающее, В КАКОЙ БД и В КАКИХ ТАБЛИЦАХ +живёт identity. Всё остальное (auth_session, rbac, роуты) спрашивает у него, и +поэтому ошибка ЗДЕСЬ — это ошибка сразу везде. + +Главное, что пинят эти тесты (⚠️ ограничение PR: после мержа прод обязан +работать ТОЧНО как сейчас): + + 1. ДЕФОЛТ = старое поведение. `IDENTITY_STORE` не задан → `tradein_users` / + `tradein_sessions`, boolean-колонка, сессия из `app.core.db.SessionLocal`. + 2. При дефолте код НЕ ТРОГАЕТ БД `auth` вообще: engine не строится, пустой + `AUTH_DATABASE_URL` не ошибка. На проде роль `auth_app` ещё без пароля и + DSN не заведён — любое обращение туда было бы отказом входа. + 3. `IDENTITY_STORE=auth` + пустой DSN → ЯВНАЯ `AuthDatabaseNotConfiguredError`, + а не тихий фолбэк на tradein-таблицы и не пустой результат. Молчаливая + деградация auth-пути читалась бы как «неверный пароль» у всех сразу. + 4. `get_identity_db` в дефолтном режиме отдаёт ТОТ ЖЕ объект `Session`, что и + `get_db` — «Команда» пишет строку сотрудника и его квоту одной транзакцией. + Регрессия здесь дала бы состояние «сотрудник создан, квота нет». + 5. Литералы значений состояния (`True`/`'active'`/...) — пин по таблице + значений, а не round-trip через `to_access_state`: инверсия + `access_state_param` обязана быть видна. +""" + +from __future__ import annotations + +import os +from typing import Annotated, Any + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import pytest +from fastapi import Depends, FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import Engine + +from app.core import auth_db, config +from app.core.db import get_db +from app.core.rbac import rbac_guard +from app.services import identity_store +from app.services.identity_store import ( + AccessState, + access_state_param, + get_identity_db, + identity_schema, + identity_session, + to_access_state, +) +from tests.support.identity_modes import IDENTITY_MODES, use_identity_mode + +_FAKE_AUTH_DSN = "postgresql+psycopg://auth_app:secret@localhost:5432/auth" + + +@pytest.fixture(autouse=True) +def _clean_identity_state(monkeypatch: pytest.MonkeyPatch): + """Дефолтный режим + пустой DSN + сброшенный engine до И после теста. + + Engine БД `auth` живёт в module-global, а не в `settings`, поэтому + monkeypatch его не откатывает — держим сброс явно с обеих сторон, иначе + построенный здесь engine утёк бы в любой следующий тест сьюта. + """ + auth_db.reset_auth_db() + monkeypatch.setattr(config.settings, "identity_store", "tradein") + monkeypatch.setattr(config.settings, "auth_database_url", "") + yield + auth_db.reset_auth_db() + + +class _FakeSession: + """Session-заглушка: тестам здесь важна ИДЕНТИЧНОСТЬ объекта, не поведение.""" + + def __enter__(self) -> _FakeSession: + return self + + def __exit__(self, *exc: object) -> bool: + return False + + def close(self) -> None: + pass + + +# --------------------------------------------------------------------------- +# identity_schema — имена, попадающие прямо в SQL +# --------------------------------------------------------------------------- + + +def test_settings_defaults_are_legacy_mode(monkeypatch: pytest.MonkeyPatch) -> None: + """⚠️ ГЛАВНЫЙ ИНВАРИАНТ PR, пин НАПРЯМУЮ по классу настроек. + + Все остальные identity-тесты работают под autouse-фикстурой, которая + ПРИНУДИТЕЛЬНО выставляет `identity_store="tradein"` — то есть проверяют + поведение при уже выбранном режиме, а не сам дефолт. Перевернись + `Field(default=...)` в config.py — они бы этого не заметили, и прод молча + ушёл бы в БД `auth`, где ещё нет ни пароля роли `auth_app`, ни данных. + + Поэтому здесь настройки конструируются заново, минуя `config.settings`: + * `_env_file=None` — не читать локальный `.env` (дев-машина или CI могут + держать там свои значения; пиним ДЕФОЛТ КОДА, а не окружение); + * `delenv` обеих переменных — то же самое для переменных процесса. + Останется ровно то, что записано литералом в `Settings`. + """ + monkeypatch.delenv("IDENTITY_STORE", raising=False) + monkeypatch.delenv("AUTH_DATABASE_URL", raising=False) + + fresh = config.Settings(_env_file=None) # type: ignore[call-arg] + + assert fresh.identity_store == "tradein", ( + "дефолт IDENTITY_STORE обязан остаться 'tradein': прод после мержа должен " + "работать ТОЧНО как сейчас, на tradein_users/tradein_sessions" + ) + assert fresh.auth_database_url == "", ( + "AUTH_DATABASE_URL обязан быть пуст по умолчанию: на проде DSN роли " + "auth_app ещё не заведён, и пустое значение не должно ронять старт" + ) + + +def test_default_schema_is_todays_production(monkeypatch: pytest.MonkeyPatch) -> None: + """Без переменной окружения — ровно сегодняшние таблицы «Меры».""" + schema = identity_schema() + assert schema.store == "tradein" + assert schema.users_table == "tradein_users" + assert schema.sessions_table == "tradein_sessions" + assert schema.access_state_column == "is_active" + assert schema.access_state_sql_type == "boolean" + + +def test_auth_schema_points_at_shared_registry(monkeypatch: pytest.MonkeyPatch) -> None: + """В БД `auth` таблицы без префикса продукта — реестр общий на «Меру» и «Птицу».""" + use_identity_mode(monkeypatch, "auth") + schema = identity_schema() + assert schema.store == "auth" + assert schema.users_table == "users" + assert schema.sessions_table == "sessions" + assert schema.access_state_column == "access_state" + assert schema.access_state_sql_type == "text" + + +def test_schema_is_read_per_call_not_cached_at_import(monkeypatch: pytest.MonkeyPatch) -> None: + """Флаг читается на КАЖДОМ вызове: переключение не требует перезагрузки модулей.""" + assert identity_schema().users_table == "tradein_users" + use_identity_mode(monkeypatch, "auth") + assert identity_schema().users_table == "users" + + +def test_unknown_store_raises_instead_of_silent_fallback(monkeypatch: pytest.MonkeyPatch) -> None: + """Значение вне словаря — ошибка, а не «ну возьмём tradein». + + Недостижимо через настройки (`Literal` валидируется pydantic), но молчаливый + фолбэк здесь означал бы поход не в ту БД. + """ + monkeypatch.setattr(config.settings, "identity_store", "elsewhere") + with pytest.raises(ValueError, match="elsewhere"): + identity_schema() + + +@pytest.mark.parametrize("mode", IDENTITY_MODES) +def test_table_names_never_come_from_outside(monkeypatch: pytest.MonkeyPatch, mode: str) -> None: + """Имена таблиц — только из фиксированного словаря (защита от SQL-инъекции по имени). + + Имя таблицы нельзя передать bind-параметром, оно склеивается в строку запроса, + поэтому единственный допустимый источник — `_SCHEMAS`. Тест пинит, что весь + набор значений конечен и не содержит ничего, кроме идентификаторов. + """ + use_identity_mode(monkeypatch, mode) + schema = identity_schema() + for name in (schema.users_table, schema.sessions_table, schema.access_state_column): + assert name.replace("_", "").isalnum(), name + assert schema.access_state_sql_type in ("boolean", "text") + + +# --------------------------------------------------------------------------- +# AccessState / to_access_state — ОДНО понятие состояния на обе схемы +# --------------------------------------------------------------------------- + + +def test_only_active_can_sign_in() -> None: + assert AccessState.ACTIVE.can_sign_in is True + assert AccessState.TRIAL_EXPIRED.can_sign_in is False + assert AccessState.DISABLED.can_sign_in is False + + +def test_boolean_column_maps_to_active_disabled() -> None: + """Булев `tradein_users.is_active` — ровно два состояния, `trial_expired` там нет.""" + assert to_access_state(True) is AccessState.ACTIVE + assert to_access_state(False) is AccessState.DISABLED + + +def test_text_column_maps_by_value() -> None: + assert to_access_state("active") is AccessState.ACTIVE + assert to_access_state("trial_expired") is AccessState.TRIAL_EXPIRED + assert to_access_state("disabled") is AccessState.DISABLED + + +@pytest.mark.parametrize("value", ["", "ACTIVE", "pending_review", None, 1, 0, object()]) +def test_unrecognized_state_is_fail_closed(value: object) -> None: + """Неизвестное значение / NULL / неожиданный тип → `disabled`. + + Обратный выбор («пускать всё, что не disabled») означал бы, что состояние, + добавленное миграцией РАНЬШЕ кода, молча раздаёт доступ. NB: `1`/`0` — это + int, а не bool, и в булевой схеме они не появляются; сюда они попадают как + «неожиданный тип» и тоже блокируются. + """ + assert to_access_state(value) is AccessState.DISABLED + + +# --------------------------------------------------------------------------- +# access_state_param — обратное направление (ЗАПИСЬ) +# --------------------------------------------------------------------------- + + +def test_write_value_in_boolean_schema() -> None: + """Литералы, а не round-trip: инверсия функции обязана быть видна прямо здесь.""" + assert access_state_param(AccessState.ACTIVE) is True + assert access_state_param(AccessState.DISABLED) is False + + +def test_write_value_in_text_schema(monkeypatch: pytest.MonkeyPatch) -> None: + use_identity_mode(monkeypatch, "auth") + assert access_state_param(AccessState.ACTIVE) == "active" + assert access_state_param(AccessState.TRIAL_EXPIRED) == "trial_expired" + assert access_state_param(AccessState.DISABLED) == "disabled" + + +def test_trial_expired_is_not_silently_downgraded_in_boolean_schema() -> None: + """`trial_expired` в булевой схеме — ошибка вызывающего, НЕ тихий `False`. + + Тихое приведение превратило бы «пробный период истёк» в жёсткую блокировку: + клиент увидел бы «неверный логин или пароль» вместо экрана пробного периода. + """ + with pytest.raises(ValueError, match="trial_expired"): + access_state_param(AccessState.TRIAL_EXPIRED) + + +# --------------------------------------------------------------------------- +# Где физически берётся сессия реестра +# --------------------------------------------------------------------------- + + +def test_default_mode_uses_product_session_and_never_builds_auth_engine( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Дефолт: та же `SessionLocal`, что у всего приложения; БД `auth` не трогается. + + Это буквально «прод после мержа работает как сейчас»: `AUTH_DATABASE_URL` на + проде пуст, и его отсутствие не должно ни ронять старт, ни всплывать в + рантайме. + """ + opened: list[_FakeSession] = [] + + def _session_local() -> _FakeSession: + s = _FakeSession() + opened.append(s) + return s + + monkeypatch.setattr(identity_store, "SessionLocal", _session_local) + + with identity_session() as db: + assert db is opened[0] + + assert len(opened) == 1 + assert auth_db._engine is None + assert auth_db._session_factory is None + + +def test_auth_mode_without_dsn_raises_instead_of_silent_fallback( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`IDENTITY_STORE=auth` + пустой DSN → явная ошибка, и НИ ОДНОГО запроса в tradein. + + Тихий фолбэк на `tradein_users` был бы худшим исходом: вход бы «работал», но + в реестре, который к тому моменту считается неактуальным. + """ + use_identity_mode(monkeypatch, "auth") + + def _must_not_be_called() -> _FakeSession: + raise AssertionError("режим auth не имеет права открывать сессию БД tradein") + + monkeypatch.setattr(identity_store, "SessionLocal", _must_not_be_called) + + with pytest.raises(auth_db.AuthDatabaseNotConfiguredError, match="AUTH_DATABASE_URL"): + with identity_session(): + pass + + +def test_auth_engine_is_lazy_cached_and_resettable(monkeypatch: pytest.MonkeyPatch) -> None: + """Engine строится при ПЕРВОМ обращении, кешируется, сбрасывается `reset_auth_db`. + + `create_engine` к серверу не ходит (пул ленивый), поэтому тест не требует + живой БД — проверяется именно кеширование, из-за которого два одновременных + первых запроса иначе создали бы два независимых пула. + """ + use_identity_mode(monkeypatch, "auth") + monkeypatch.setattr(config.settings, "auth_database_url", _FAKE_AUTH_DSN) + + assert auth_db._engine is None # ленивость: до первого обращения ничего нет + engine = auth_db.get_auth_engine() + assert isinstance(engine, Engine) + assert auth_db.get_auth_engine() is engine + assert auth_db.get_auth_session_factory() is auth_db.get_auth_session_factory() + + auth_db.reset_auth_db() + assert auth_db._engine is None + assert auth_db.get_auth_engine() is not engine + + +def test_blank_dsn_is_not_configured(monkeypatch: pytest.MonkeyPatch) -> None: + """DSN из одних пробелов = не задан (иначе `create_engine('')` дал бы мутную ошибку).""" + use_identity_mode(monkeypatch, "auth") + monkeypatch.setattr(config.settings, "auth_database_url", " ") + with pytest.raises(auth_db.AuthDatabaseNotConfiguredError): + auth_db.get_auth_engine() + + +# --------------------------------------------------------------------------- +# get_identity_db — FastAPI-зависимость: ОДНА транзакция в дефолте, две в auth +# --------------------------------------------------------------------------- + + +def _probe_app() -> FastAPI: + """Мини-приложение с обеими зависимостями сразу — как у роутов «Команды».""" + app = FastAPI() + + @app.get("/probe") + async def probe( + db: Annotated[Any, Depends(get_db)], + identity_db: Annotated[Any, Depends(get_identity_db)], + ) -> dict[str, bool]: + return {"same_session": db is identity_db} + + return app + + +def test_default_mode_shares_one_session_with_get_db(monkeypatch: pytest.MonkeyPatch) -> None: + """`db is identity_db` в дефолте — не экономия коннекта, а требование прода. + + «Команда» пишет строку сотрудника (реестр) и его квоту (`account_quota_overrides`, + продуктовая таблица) В ОДНОЙ транзакции. Две сессии = две транзакции = + состояние «сотрудник создан, квота нет» на ровном месте. + """ + app = _probe_app() + app.dependency_overrides[get_db] = lambda: iter([_FakeSession()]) + + resp = TestClient(app).get("/probe") + + assert resp.status_code == 200, resp.text + assert resp.json() == {"same_session": True} + + +def test_auth_mode_yields_separate_registry_session(monkeypatch: pytest.MonkeyPatch) -> None: + """В режиме `auth` БД физически разные → и сессии обязаны быть разными объектами. + + `db is not identity_db` — рантайм-признак «БД разные», по которому `team.py` + решает, коммитить ли вторую транзакцию. + """ + use_identity_mode(monkeypatch, "auth") + registry_session = _FakeSession() + + from contextlib import contextmanager + + @contextmanager + def _fake_auth_session(): + yield registry_session + + monkeypatch.setattr(auth_db, "auth_session", _fake_auth_session) + + app = _probe_app() + app.dependency_overrides[get_db] = lambda: iter([_FakeSession()]) + + resp = TestClient(app).get("/probe") + + assert resp.status_code == 200, resp.text + assert resp.json() == {"same_session": False} + + +# --------------------------------------------------------------------------- +# Сломанная конфигурация не роняет запрос (rbac_guard) +# --------------------------------------------------------------------------- + + +def _guarded_app() -> FastAPI: + app = FastAPI() + app.middleware("http")(rbac_guard) + + @app.get("/api/v1/trade-in/dummy") + async def dummy() -> dict[str, bool]: + return {"ok": True} + + return app + + +def test_misconfigured_auth_store_degrades_to_401_not_500( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`IDENTITY_STORE=auth` без DSN + запрос С КУКОЙ → 401, а не 500. + + `AuthDatabaseNotConfiguredError` обрабатывается тем же путём, что и любой + сбой БД: резолв сессии не состоялся, дальше решает `auth_mode`. Сознательно + не отличается от «БД недоступна» — обе ситуации это сломанная конфигурация + реестра, и ни одна не имеет права отдавать 500 (или, тем более, пускать). + """ + use_identity_mode(monkeypatch, "auth") + client = TestClient(_guarded_app(), base_url="https://testserver") + client.cookies.set(config.settings.session_cookie_name, "some-token") + + resp = client.get("/api/v1/trade-in/dummy") + + assert resp.status_code == 401 + # Legacy trusted-header путь (dual-mode) при этом продолжает работать — + # сломанный реестр не отрезает существующих пользователей Caddy. + # + # ⚠️ Это поведение УЖЕ НЕДОСТИЖИМО в реальном процессе: до такого состояния + # приложение не доживает, потому что lifespan падает на старте (см. + # `test_lifespan_fails_fast_when_auth_store_has_no_dsn` ниже). Тест держит + # guard'а от 500-ки/анонимного доступа как второй рубеж — на случай, если + # DSN сломается уже ПОСЛЕ успешного старта. + fallback = client.get("/api/v1/trade-in/dummy", headers={"X-Authenticated-User": "kopylov"}) + assert fallback.status_code == 200, fallback.text + + +# --------------------------------------------------------------------------- +# Boot-time guard: сломанный реестр не должен ЖИТЬ на legacy-пути +# --------------------------------------------------------------------------- + + +def _run_lifespan(monkeypatch: pytest.MonkeyPatch) -> None: + """Прогоняет lifespan приложения до `yield` и обратно. + + FDW-bootstrap выключен: он ходит в продуктовую БД, которой в юнит-тестах + нет. К проверяемому здесь он отношения не имеет (и в самом lifespan обёрнут + в try/except), а без заглушки тест ждал бы таймаута коннекта. + """ + import asyncio + + from app import main as app_main + + monkeypatch.setattr(app_main, "ensure_fdw_user_mapping", lambda db: None) + + async def _cycle() -> None: + async with app_main.lifespan(app_main.app): + pass + + asyncio.run(_cycle()) + + +def test_lifespan_fails_fast_when_auth_store_has_no_dsn( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`IDENTITY_STORE=auth` + пустой DSN → контейнер НЕ поднимается. + + Почему не «работает как-нибудь»: `rbac_guard` ловит + `AuthDatabaseNotConfiguredError` вместе с любым другим сбоем резолва сессии + и уходит в legacy trusted-header ветку. Продуктовая БД при этом жива, и + такой деплой способен работать сутками, раздавая права из roles.yaml всем, + кого пропустил Caddy basic_auth, — включая аккаунты, у которых в реестре + `access_state='disabled'`/`'trial_expired'`. Ошибка КОНФИГУРАЦИИ обязана + убивать старт, а не деградировать в тихий обход реестра. + """ + use_identity_mode(monkeypatch, "auth") + monkeypatch.setattr(config.settings, "auth_database_url", "") + + with pytest.raises(auth_db.AuthDatabaseNotConfiguredError): + _run_lifespan(monkeypatch) + + +def test_lifespan_does_not_touch_auth_db_in_default_mode( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Дефолтный режим: старт НЕ обращается к БД `auth` и пустой DSN не мешает. + + Ровно ограничение PR — сегодняшний прод (`IDENTITY_STORE` не задан, + `AUTH_DATABASE_URL` нет вовсе) обязан подниматься как раньше. + """ + from app import main as app_main + + calls: list[str] = [] + monkeypatch.setattr(app_main, "get_auth_engine", lambda: calls.append("built")) + + _run_lifespan(monkeypatch) + + assert calls == [], "в дефолтном режиме engine БД `auth` не должен строиться на старте" diff --git a/tradein-mvp/backend/tests/test_team_api.py b/tradein-mvp/backend/tests/test_team_api.py index 3487e2c3..0a06cb60 100644 --- a/tradein-mvp/backend/tests/test_team_api.py +++ b/tradein-mvp/backend/tests/test_team_api.py @@ -2,19 +2,44 @@ 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`. +(`_Store`/`_FakeDB`) dispatching on SQL text standing in for реестра людей / +`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`. +Сессия РЕЕСТРА подменяется на самом низком уровне (`identity_store.SessionLocal` ++ `auth_db.auth_session`, см. `tests.support.identity_modes.patch_identity_sessions`), +а `app.core.db.get_db` — через `app.dependency_overrides`. Поэтому и +`identity_session()` (rbac_guard — middleware, FastAPI-DI там нет), и +`Depends(get_identity_db)` (`current_team_actor`, все team-роуты) выполняются +НАСТОЯЩИЕ, вместе со своим ветвлением по `settings.identity_store`. Все они +смотрят в ОДИН `_Store` на тест — сессия из POST /login сразу видна и +rbac_guard'у, и `current_team_actor`. + +ДВЕ СЕССИИ. В дефолтном режиме `get_identity_db` отдаёт ТОТ ЖЕ объект, что +`get_db` (одна БД, одна транзакция — сегодняшний прод). В режиме `auth` это +физически разные сессии, и `team.py` коммитит их отдельно (`if db is not +identity_db`). Здесь это воспроизводится честно: в режиме `auth` реестр и +продуктовые таблицы получают РАЗНЫЕ `_FakeDB` (общий `_Store` — как общий +«кластер», но разные соединения). + +⚠️ ЛОВУШКА FAKE-DB. `_FakeDB` диспатчит по ТЕКСТУ SQL, а эпик «единый вход» +переименовывает таблицы (`tradein_users`/`tradein_sessions` → `users`/`sessions`) +и меняет тип колонки состояния доступа (`is_active boolean` → `access_state +text`). Литерал «tradein_users» в диспатчере означал бы, что при +`IDENTITY_STORE=auth` ветка молча перестаёт матчиться, fake отдаёт пустоту, а +тест остаётся ЗЕЛЁНЫМ на сломанном коде. Поэтому имена берутся из `sql_names()` +(= `identity_schema()`, тот же словарь, что у продакшн-кода), а непонятый SQL +падает `AssertionError`, а не возвращает пустой результат. + +Значение состояния доступа fake хранит СЫРЫМ (то, что реально лежало бы в +колонке) и НЕ прогоняет через `identity_store.access_state_param()` — иначе +инверсия этой функции прошла бы round-trip через fake незамеченной. """ from __future__ import annotations import os +import re from datetime import UTC, datetime, timedelta from types import SimpleNamespace from typing import Any @@ -33,9 +58,19 @@ 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 +from app.services.identity_store import AccessState +from tests.support.identity_modes import ( + assert_insert_writes_access_state, + assert_reads_access_state, + assert_update_writes_access_state, + column_value, + patch_identity_sessions, + sql_names, + use_identity_mode, +) # --------------------------------------------------------------------------- -# Fake DB backing tradein_users / tradein_sessions / quota / user_events +# Fake DB backing реестр людей / sessions / quota / user_events # --------------------------------------------------------------------------- @@ -47,6 +82,8 @@ class _Store: 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.sql_log: list[str] = [] # весь SQL, доехавший до «БД» — см. тесты режимов + self.commits: list[int] = [] # id() сессий, на которых вызывали commit() self._next_id = 1 self.query_count = 0 # db.execute() calls — N+1 regression guard (review PR #2563) @@ -57,7 +94,7 @@ class _Store: *, role: str = "employee", manager_id: int | None = None, - is_active: bool = True, + access_state: AccessState = AccessState.ACTIVE, display_name: str | None = None, org_name: str | None = None, email: str | None = None, @@ -74,7 +111,8 @@ class _Store: "display_name": display_name, "org_name": org_name, "email": email, - "is_active": is_active, + # СЫРОЕ значение колонки текущего режима (boolean либо text). + "access_state": column_value(access_state), "created_at": created_at or datetime.now(UTC), } return uid @@ -162,7 +200,7 @@ class _FakeDB: pass def commit(self) -> None: - pass + self.store.commits.append(id(self)) def rollback(self) -> None: pass @@ -172,9 +210,13 @@ class _FakeDB: p = params or {} s = self.store s.query_count += 1 + s.sql_log.append(sql) + # Имена таблиц/колонки берутся ИЗ КОДА (identity_schema), а не из + # литералов — см. «ЛОВУШКА FAKE-DB» в модульном docstring. + names = sql_names() - # ---- tradein_sessions ---- - if "INSERT INTO tradein_sessions" in sql: + # ---- сессии реестра ---- + if f"INSERT INTO {names.sessions}" in sql: now = datetime.now(UTC) s.sessions[p["token"]] = { "user_id": p["user_id"], @@ -183,7 +225,7 @@ class _FakeDB: } return _Result([]) - if "UPDATE tradein_sessions" in sql and "SET last_seen_at" in sql: + if f"UPDATE {names.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) @@ -191,23 +233,25 @@ class _FakeDB: sess["expires_at"] = now + timedelta(hours=p["ttl_hours"]) return _Result([]) - if "DELETE FROM tradein_sessions WHERE token" in sql: + if f"DELETE FROM {names.sessions} WHERE token" in sql: s.sessions.pop(p["token"], None) return _Result([]) - if "DELETE FROM tradein_sessions WHERE user_id" in sql: + if f"DELETE FROM {names.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: + if f"FROM {names.sessions} s" in sql and f"JOIN {names.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([]) + # Колонка состояния приезжает под алиасом `access_state` в обоих + # режимах (`u.<колонка> AS access_state`), значение — сырое. return _Result( [ { @@ -219,18 +263,23 @@ class _FakeDB: "display_name": user["display_name"], "org_name": user["org_name"], "email": user["email"], - "is_active": user["is_active"], + "access_state": user["access_state"], } ] ) - # ---- tradein_users: login lookup (get_user_by_username) ---- - if "password_hash, role, is_active" in sql and "FROM tradein_users" in sql: + # ---- реестр: login lookup (get_user_by_username) ---- + # Дискриминатор — bind-параметр `:username` (у pre-check'а уникальности + # ниже он называется `:u`), поэтому ветки не пересекаются ни в одном режиме. + if f"FROM {names.users}" in sql and "WHERE username = :username" in sql: + assert_reads_access_state(sql, names) 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: + # ---- реестр: create ---- + if f"INSERT INTO {names.users}" in sql: + assert_insert_writes_access_state(sql, names) + assert_reads_access_state(sql, names) # RETURNING отдаёт её же uid = s._next_id s._next_id += 1 created_at = datetime.now(UTC) @@ -243,25 +292,31 @@ class _FakeDB: "display_name": p["display_name"], "org_name": p["org_name"], "email": p["email"], - "is_active": True, + # Ровно то, что код прислал параметром — БЕЗ нормализации. + # Инверсия `access_state_param()` обязана доехать до ответа API + # (`is_active`), а не раствориться в дублёре. + "access_state": p["access_state"], "created_at": created_at, } s.users[p["username"]] = row return _Result([dict(row)]) - # ---- tradein_users: manager_id validation ---- - if "role = 'manager'" in sql: + # ---- реестр: manager_id validation ---- + if f"FROM {names.users}" in sql and "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 managed rows (has explicit ORDER BY) ---- + # ---- реестр: list managed rows (has explicit ORDER BY) ---- # Две ветки реального кода: `role = 'employee'` (manager, либо admin с # ?manager_id=) и `role IN ('employee','manager')` (admin без фильтра — # ему нужны и менеджеры, иначе некому сбросить пароль, см. team.py). - if ("role = 'employee'" in sql or "role IN ('employee', 'manager')" in sql) and ( - "ORDER BY created_at DESC" in sql + if ( + f"FROM {names.users}" in sql + and ("role = 'employee'" in sql or "role IN ('employee', 'manager')" in sql) + and "ORDER BY created_at DESC" in sql ): + assert_reads_access_state(sql, names) managed = ( ("employee", "manager") if "role IN ('employee', 'manager')" in sql @@ -285,7 +340,7 @@ class _FakeDB: "display_name": u["display_name"], "org_name": u["org_name"], "email": u["email"], - "is_active": u["is_active"], + "access_state": u["access_state"], "manager_id": u["manager_id"], "created_at": u["created_at"], } @@ -293,8 +348,11 @@ class _FakeDB: ] ) - # ---- tradein_users: fetch single managed row by id ---- - if "role = 'employee'" in sql or "role IN ('employee', 'manager')" in sql: + # ---- реестр: fetch single managed row by id ---- + if f"FROM {names.users}" in sql and ( + "role = 'employee'" in sql or "role IN ('employee', 'manager')" in sql + ): + assert_reads_access_state(sql, names) managed = ( ("employee", "manager") if "role IN ('employee', 'manager')" in sql @@ -312,20 +370,21 @@ class _FakeDB: "display_name": user["display_name"], "org_name": user["org_name"], "email": user["email"], - "is_active": user["is_active"], + "access_state": user["access_state"], "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"): + # ---- реестр: uniqueness pre-check ---- + if sql.strip().startswith(f"SELECT id FROM {names.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: + # ---- реестр: update (PATCH) ---- + if f"UPDATE {names.users}" in sql and "SET display_name = COALESCE" in sql: + assert_update_writes_access_state(sql, names) user = s.user_by_id(p["id"]) assert user is not None if p.get("display_name") is not None: @@ -334,8 +393,11 @@ class _FakeDB: 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"] + # COALESCE(CAST(:access_state AS <тип>), <колонка>) — None означает + # «поле не пришло в PATCH», значение записывается КАК ЕСТЬ (см. + # комментарий про round-trip в INSERT выше). + if p.get("access_state") is not None: + user["access_state"] = p["access_state"] if p.get("password_hash") is not None: user["password_hash"] = p["password_hash"] return _Result([]) @@ -437,6 +499,8 @@ 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") + # Каждый тест стартует в ДЕФОЛТНОМ режиме реестра (сегодняшний прод). + use_identity_mode(monkeypatch, "tradein") # 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)) @@ -452,9 +516,23 @@ def store() -> _Store: return _Store() +@pytest.fixture +def auth_store(store: _Store, monkeypatch: pytest.MonkeyPatch) -> _Store: + """Тот же `store`, но реестр — БД `auth` (`users`/`sessions`, text-состояние). + + Запрашивать ПЕРЕД `client`: `store.add_user` фиксирует значение колонки по + режиму на момент вызова. + """ + use_identity_mode(monkeypatch, "auth") + return store + + @pytest.fixture def client(store: _Store, monkeypatch: pytest.MonkeyPatch) -> TestClient: - monkeypatch.setattr("app.core.rbac.SessionLocal", lambda: _FakeDB(store)) + # Подменяем сессию РЕЕСТРА на обоих её источниках сразу, а не ветвление по + # режиму: `identity_session()` / `get_identity_db()` остаются настоящими, + # включая инвариант «в дефолтном режиме это тот же объект, что у get_db». + patch_identity_sessions(monkeypatch, 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") @@ -818,7 +896,11 @@ def test_reset_password_revokes_old_sessions(client: TestClient, store: _Store) 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 + "emp_a", + hash_password("Secret123!"), + role="employee", + manager_id=mgr_id, + access_state=AccessState.DISABLED, ) _login(client, "mgr_a", "Secret123!") @@ -1157,3 +1239,178 @@ def test_employee_history_limit_max_200(client: TestClient, store: _Store) -> No resp = client.get(f"/api/v1/team/employees/{emp_id}/history", params={"limit": 500}) assert resp.status_code == 422 + + +# --------------------------------------------------------------------------- +# Эпик «единый вход»: режим IDENTITY_STORE=auth (общий реестр в БД `auth`). +# +# Всё выше идёт в ДЕФОЛТНОМ режиме — он же прод. Ниже — то, что появляется +# только после переезда: другая БД под реестром (две сессии вместо одной) и +# текстовое трёхзначное состояние доступа вместо булева `is_active`. +# --------------------------------------------------------------------------- + + +def test_default_mode_single_session_and_tradein_tables(client: TestClient, store: _Store) -> None: + """Дефолт: реестр и продуктовые таблицы — ОДНА сессия, один commit, старые имена. + + Это и есть «после мержа прод работает точно как сейчас» на уровне + транзакции: «сотрудник создан, квота нет» невозможно, потому что писать + обоих некуда, кроме одной транзакции. + """ + store.add_user("mgr_a", hash_password("Secret123!"), role="manager") + _login(client, "mgr_a", "Secret123!") + store.commits.clear() + + resp = client.post( + "/api/v1/team/employees", + json={"username": "emp_x", "password": "Secret123!", "monthly_limit": 7}, + ) + assert resp.status_code == 201, resp.text + + # Ровно один commit и ровно на одной сессии — `db is identity_db`. + assert len(set(store.commits)) == 1, store.commits + joined = "\n".join(store.sql_log) + assert "tradein_users" in joined + assert "tradein_sessions" in joined + assert not re.search(r"\b(FROM|INTO|UPDATE|JOIN)\s+users\b", joined) + assert not re.search(r"\b(FROM|INTO|UPDATE|JOIN)\s+sessions\b", joined) + # Новый сотрудник заводится открытым — булевым литералом, как и раньше. + assert store.users["emp_x"]["access_state"] is True + assert resp.json()["is_active"] is True + + +def test_auth_mode_commits_registry_and_product_db_separately( + auth_store: _Store, client: TestClient +) -> None: + """Режим `auth`: БД физически две → две сессии и два отдельных коммита. + + Порядок несущий (реестр первым): не доехавшая квота — это сотрудник с + глобальным лимитом (чинится повторным PATCH), а обратный порядок оставил бы + висящий override на несуществующего человека. + """ + auth_store.add_user("mgr_a", hash_password("Secret123!"), role="manager") + _login(client, "mgr_a", "Secret123!") + auth_store.commits.clear() + + resp = client.post( + "/api/v1/team/employees", + json={"username": "emp_x", "password": "Secret123!", "monthly_limit": 7}, + ) + assert resp.status_code == 201, resp.text + + assert len(set(auth_store.commits)) == 2, auth_store.commits + joined = "\n".join(auth_store.sql_log) + assert "tradein_users" not in joined + assert "tradein_sessions" not in joined + assert re.search(r"INSERT INTO\s+users\b", joined) + # Квота осталась в ПРОДУКТОВОЙ таблице — она в общий реестр не переезжает. + assert "INSERT INTO account_quota_overrides" in joined + assert auth_store.quota_overrides["emp_x"]["monthly_limit"] == 7 + + +def test_auth_mode_create_writes_text_active_literal( + auth_store: _Store, client: TestClient +) -> None: + """INSERT кладёт в колонку 'active' (text), а не булев true. + + Значение fake хранит как есть — если бы `access_state_param()` инвертировался + или отдавал не тот тип, это доехало бы прямо сюда и до `is_active` в ответе. + """ + auth_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_x", "password": "Secret123!"} + ) + + assert resp.status_code == 201, resp.text + assert auth_store.users["emp_x"]["access_state"] == "active" + assert resp.json()["is_active"] is True + + +def test_auth_mode_block_writes_disabled_and_revokes_sessions( + auth_store: _Store, client: TestClient +) -> None: + """PATCH is_active=false → колонка 'disabled' + все сессии сотрудника порваны. + + Сессии живут в БД РЕЕСТРА, поэтому рвать их надо через `identity_db`: с + продуктовой сессией DELETE ушёл бы не в ту БД, и блокировка не действовала бы + до истечения TTL (а sliding-refresh продлевал бы её бесконечно). + """ + mgr_id = auth_store.add_user("mgr_a", hash_password("Secret123!"), role="manager") + emp_id = auth_store.add_user( + "emp_a", hash_password("Secret123!"), role="employee", manager_id=mgr_id + ) + auth_store.sessions["emp-token"] = { + "user_id": emp_id, + "expires_at": datetime.now(UTC) + timedelta(hours=1), + "last_seen_at": datetime.now(UTC), + } + _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 + assert auth_store.users["emp_a"]["access_state"] == "disabled" + assert "emp-token" not in auth_store.sessions + + +def test_auth_mode_trial_expired_shows_as_blocked_and_unblock_activates( + auth_store: _Store, client: TestClient +) -> None: + """`trial_expired` в «Команде» выглядит заблокированным, а is_active=true снимает + пробное ограничение (переводит в `active`). + + Форма ответа API не меняется этим PR: `is_active` остаётся булевым и считается + как «пустят ли входить». Отдельное отображение пробного периода — вопрос UI-PR'а. + """ + mgr_id = auth_store.add_user("mgr_a", hash_password("Secret123!"), role="manager") + emp_id = auth_store.add_user( + "emp_a", + hash_password("Secret123!"), + role="employee", + manager_id=mgr_id, + access_state=AccessState.TRIAL_EXPIRED, + ) + _login(client, "mgr_a", "Secret123!") + + listed = client.get("/api/v1/team/employees") + assert listed.status_code == 200, listed.text + assert [e["is_active"] for e in listed.json()] == [False] + + resp = client.patch(f"/api/v1/team/employees/{emp_id}", json={"is_active": True}) + assert resp.status_code == 200, resp.text + assert resp.json()["is_active"] is True + assert auth_store.users["emp_a"]["access_state"] == "active" + + +def test_auth_mode_org_isolation_still_404s_foreign_employee( + auth_store: _Store, client: TestClient +) -> None: + """Главный инвариант «Команды» (чужой сотрудник → 404, не 403) переезд переживает.""" + auth_store.add_user("mgr_a", hash_password("Secret123!"), role="manager") + mgr_b_id = auth_store.add_user("mgr_b", hash_password("Secret123!"), role="manager") + foreign_id = auth_store.add_user( + "emp_b", hash_password("Secret123!"), role="employee", manager_id=mgr_b_id + ) + _login(client, "mgr_a", "Secret123!") + + assert client.get("/api/v1/team/employees").json() == [] + patched = client.patch(f"/api/v1/team/employees/{foreign_id}", json={"is_active": False}) + assert patched.status_code == 404 + assert client.get(f"/api/v1/team/employees/{foreign_id}/history").status_code == 404 + # Чужая строка не тронута. + assert auth_store.users["emp_b"]["access_state"] == "active" + + +def test_auth_mode_employee_role_still_403_on_team_routes( + auth_store: _Store, client: TestClient +) -> None: + """Роль резолвится из общего реестра — employee по-прежнему не админ «Команды».""" + auth_store.add_user("emp_only", hash_password("Secret123!"), role="employee") + _login(client, "emp_only", "Secret123!") + + resp = client.get("/api/v1/team/employees") + assert resp.status_code == 403 + assert "admin or manager" in resp.json()["detail"].lower() diff --git a/tradein-mvp/frontend/src/app/login/page.tsx b/tradein-mvp/frontend/src/app/login/page.tsx index 8bc61556..ad74d876 100644 --- a/tradein-mvp/frontend/src/app/login/page.tsx +++ b/tradein-mvp/frontend/src/app/login/page.tsx @@ -71,12 +71,32 @@ function sanitizeNext(next: string | null): string { return cleaned; } +/** + * Единственный 403 логина — «пробный доступ закончился» (пароль ВЕРНЫЙ, + * access_state='trial_expired' в реестре людей). Ветвимся по машиночитаемому + * `detail.code`, а не по тексту: текст сообщения бэк вправе менять, код — нет + * (app/api/v1/auth.py, _ACCESS_EXPIRED_CODE). + * + * Достижимо только при IDENTITY_STORE=auth: в дефолтном режиме состояние + * доступа булево (active/disabled), и trial_expired там не существует. + */ +function accessExpiredCode(body: unknown): string | undefined { + if (typeof body !== "object" || body === null) return undefined; + const detail = (body as { detail?: unknown }).detail; + if (typeof detail !== "object" || detail === null) return undefined; + const code = (detail as { code?: unknown }).code; + return typeof code === "string" ? code : undefined; +} + function loginErrorMessage(error: unknown): string { if (error instanceof HTTPError) { if (error.status === 401) return "Неверный логин или пароль"; if (error.status === 429) { return "Слишком много попыток. Попробуйте через несколько минут"; } + if (error.status === 403 && accessExpiredCode(error.body) === "access_expired") { + return "Пробный доступ закончился — обратитесь к менеджеру"; + } } return "Не удалось войти. Проверьте подключение и попробуйте ещё раз"; }