From b5976c0cc92ab1789ce348c187adb16d637970a4 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 1 Aug 2026 00:28:37 +0300 Subject: [PATCH] =?UTF-8?q?feat(auth):=20=D1=80=D0=BE=D0=BB=D0=B8=20=D0=B8?= =?UTF-8?q?=20=D1=82=D1=80=D1=91=D1=85=D0=B7=D0=BD=D0=B0=D1=87=D0=BD=D0=BE?= =?UTF-8?q?=D0=B5=20=D1=81=D0=BE=D1=81=D1=82=D0=BE=D1=8F=D0=BD=D0=B8=D0=B5?= =?UTF-8?q?=20=D0=B4=D0=BE=D1=81=D1=82=D1=83=D0=BF=D0=B0=20=D0=B2=20=D0=91?= =?UTF-8?q?=D0=94=20auth=20[PR-2a/6]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Схема под решения владельца от 2026-07-31 по эпику «единый вход». Python-кода нет, поведение прода не меняется — в БД auth пока никто не ходит. Развилка А закрыта в пользу ПОЛНОГО переезда: tradein_users (БД tradein) в итоге удаляется, auth.users становится единственным реестром людей. Значит role и manager_id переезжают сюда — это отменяет решение 001:15-19 («ролей здесь нет — сознательно»), что зафиксировано в шапке файла и переписанным COMMENT ON TABLE, а не оставлено расходиться молча. Развилка Б закрыта в пользу трёх состояний: is_active заменён на access_state (active / trial_expired / disabled). Булев флаг схлопывал «пускаем, но объясняем» и «не пускаем вовсе» в одно значение — trial-экран исчезал бы без падения тестов. Семантика зафиксирована в COMMENT: trial_expired при ВЕРНОМ пароле даёт 403 с отдельным кодом и НЕ выдаёт сессию, disabled — generic 401; неверный пароль в любом состоянии остаётся generic 401, то есть защита от перечисления логинов сохраняется. user2 («Брусника») → trial_expired. Колонки role/manager_id зеркалят м.192 побуквенно (CHECK ролей, иерархический CHECK, partial index, self-FK ON DELETE SET NULL), чтобы код «Меры» переехал на auth.users без правок. Добавлен users_manager_not_self_ck — на уровне БД самоназначение менеджером иначе проходит, а второй потребитель (Птица) валидации «Меры» не имеет. Гранты. INSERT выдан — без него переезд не состоится (создание сотрудника из «Команды»). DELETE НЕ выдан: потребителя нет (в team.py только POST и PATCH), а 002:22-33 отклоняла ровно такие гранты-на-будущее; появится хендлер — появится строка GRANT в той же миграции. Табличный UPDATE из 002:80 сужен до column-level: иначе auth_app молча получил бы право писать role и access_state, и ошибка в PATCH-эндпоинте превращалась бы в тихое повышение до админа или тихое снятие блокировки. role и manager_id в список не включены — их сегодня не пишет никто. Гранта на users_id_seq нет намеренно: для GENERATED ALWAYS AS IDENTITY PostgreSQL использует NextValueExpr → nextval_internal(check_permissions := false), ACL последовательности не проверяется. Утверждение 002:26-27 («идентичность требует nextval») фактически неверно; проверено обратным экспериментом — REVOKE, затем INSERT. Проверено исполнением на postgres:16, не по комментариям: - чистая сборка 001→002→003→004 — 13 строк, роли admin/manager×2/employee×10, user2 = trial_expired, is_active отсутствует, все 6 констрейнтов на месте; - повторный прогон 004 ×2 идемпотентен; - ручные прод-правки (user2 → active, user3 → manager) переживают повтор — backfill не затирает решения владельца; - периметр auth_app: INSERT users ✓, UPDATE access_state ✓, INSERT sessions ✓; UPDATE role ✗, UPDATE manager_id ✗, DELETE ✗, CREATE TABLE ✗; - CHECK'и ловят: admin с manager_id, self-manager, access_state вне списка, role вне списка, INSERT без role. Тест: 6 passed. Добавлена проверка запрета CREATE INDEX CONCURRENTLY — в связке с обязательной обёрткой BEGIN/COMMIT это комбинация, невыполнимая на проде (25001), а отдельной проверки на неё не было. --- backend/tests/sql/test_auth_sql_migrations.py | 22 + .../auth/004_users_roles_and_access_state.sql | 408 ++++++++++++++++++ 2 files changed, 430 insertions(+) create mode 100644 data/sql/auth/004_users_roles_and_access_state.sql diff --git a/backend/tests/sql/test_auth_sql_migrations.py b/backend/tests/sql/test_auth_sql_migrations.py index 492fcd84..3c02f7b9 100644 --- a/backend/tests/sql/test_auth_sql_migrations.py +++ b/backend/tests/sql/test_auth_sql_migrations.py @@ -120,6 +120,28 @@ def test_migrations_are_transactional() -> None: ), f"Миграции без обёртки BEGIN;/COMMIT;: {broken} (.claude/rules/sql.md → Structure)." +def test_no_concurrent_index_in_migrations() -> None: + """Ни одной CREATE/DROP INDEX CONCURRENTLY в data/sql/auth/*.sql. + + Red => миграция гарантированно падает на проде: CONCURRENTLY нельзя выполнять внутри + транзакционного блока (Postgres: 25001 «CREATE INDEX CONCURRENTLY cannot run inside a + transaction block»), а обёртка BEGIN;/COMMIT; здесь обязательна для всех файлов + (test_migrations_are_transactional). Две проверки по отдельности зелёные, а вместе + невыполнимые — поэтому запрет нужен явный: комбинация ловится только здесь. + Нужен CONCURRENTLY на большой таблице — это отдельный ручной прогон вне auto-apply, + а не файл в этом каталоге. + """ + hits: list[str] = [] + for path in _auth_sql_files(): + text = path.read_text(encoding="utf-8") + for line_no, line in enumerate(text.splitlines(), start=1): + if line.lstrip().startswith("--"): + continue # комментарий может объяснять запрет, не нарушая его + if re.search(r"\bCONCURRENTLY\b", line, re.IGNORECASE): + hits.append(f"{path.name}:{line_no}: {line.strip()}") + assert not hits, "CONCURRENTLY внутри BEGIN/COMMIT — упадёт на деплое: " + "; ".join(hits) + + def test_no_password_material_in_auth_sql() -> None: """Ни в data/sql/auth, ни в ops/db-bootstrap нет plaintext-паролей и bcrypt-хешей. diff --git a/data/sql/auth/004_users_roles_and_access_state.sql b/data/sql/auth/004_users_roles_and_access_state.sql new file mode 100644 index 00000000..5a83e356 --- /dev/null +++ b/data/sql/auth/004_users_roles_and_access_state.sql @@ -0,0 +1,408 @@ +-- auth/004: продуктовые роли + org-иерархия + трёхзначный access_state вместо булева is_active. +-- +-- ⚠️ ЭТА МИГРАЦИЯ СОЗНАТЕЛЬНО ОТМЕНЯЕТ РЕШЕНИЯ, ЗАПИСАННЫЕ В 001 И 002. +-- Это не рассинхрон и не ошибка автора: решение владельца продукта от 2026-07-31 принято +-- ПОСЛЕ того, как 001-003 были написаны и применены на проде. Применённую миграцию править +-- нельзя (повторно она не выполнится — трекинг в _schema_migrations), поэтому актуальная +-- правда живёт здесь, а в 001/002 остаются исторические формулировки: +-- * 001:15-19 «Здесь НЕТ колонки role — сознательно» → ОТМЕНЕНО, см. WHY-1; +-- * 002:22-33 «users — INSERT/DELETE НЕ выдаются, сознательно» → ОТМЕНЕНО ЧАСТИЧНО: INSERT +-- выдаётся (без него переезд не состоится), DELETE — по-прежнему нет, см. Часть 4; +-- * 002:26-27 «идентичность требует nextval» (грант USAGE на sequence) → ФАКТИЧЕСКИ +-- НЕВЕРНО, гранта не требуется; проверено, разбор в Части 4; +-- * 003:78-90 «открытая развилка про trial-экран, решается в PR-2/3» → ЗАКРЫТА, см. WHY-2. +-- Ориентир для читателя: актуальное состояние колонок описано COMMENT'ами в БД, они +-- переписаны здесь. Заголовок 001 — археология, а не спецификация. +-- +-- WHY-1 — продуктовые роли переезжают в `auth` (отмена решения 001): +-- 001 строилась на схеме «идентичность общая, полномочия у продукта»: auth.users знает, КТО +-- человек, tradein_users знает, ЧТО ему можно. Владелец выбрал другой сценарий — ПОЛНЫЙ +-- переезд: tradein_users (БД tradein) в итоге удаляется, auth.users остаётся единственным +-- реестром людей. Как только реестр один, роль перестаёт быть «знанием продукта»: без неё в +-- auth.users нельзя ни завести сотрудника, ни собрать раздел «Команда», ни ответить на вопрос +-- «чьи заявки видит этот менеджер» — а спросить больше не у кого, второй таблицы не будет. +-- Промежуточный вариант (человек в auth.users, его роль в tradein_users) — это два реестра, +-- которые кто-то обязан держать синхронными руками; их расхождение выглядит как «пользователь +-- есть, но он никто» и чинится только вручную по факту жалобы. +-- Цена решения ровно та, которую 001 и называла: новая роль в любом из продуктов = миграция +-- этой БД. Принято сознательно — это дешевле, чем двойной реестр людей. +-- +-- WHY-2 — три состояния доступа вместо булева is_active (закрытие развилки из 003): +-- Булев флаг схлопывает два РАЗНЫХ события в одно значение: «пробный период закончился» и +-- «доступ закрыт владельцем». Для пользователя разница видимая и она уже реализована в +-- сегодняшнем стеке: expired-аккаунт доходит до фронта и видит осмысленный экран «пробный +-- доступ закончился» (auth/roles.yaml → expired: paths: [] + deny "/**"; frontend +-- NoAccessScreen variant="trial"), а закрытый — просто не входит. Переключившись на единую +-- форму входа с булевым is_active, мы бы потеряли trial-экран МОЛЧА: состояние перестало бы +-- существовать, и ни один тест бы не упал. Ровно это и было записано как открытая развилка в +-- 003:78-90. Решение: состояний три. +-- active — доступ есть, обычный вход. +-- trial_expired — пароль ВЕРНЫЙ, но пробный период истёк: логин отвечает 403 с отдельным +-- кодом и текстом «пробный доступ закончился», сессия НЕ выдаётся. +-- disabled — жёсткая блокировка: generic 401, для пользователя неотличимо от «неверный +-- пароль». +-- Неверный пароль в ЛЮБОМ состоянии → generic 401. Иначе отдельный 403 превращается в оракул +-- существования логина: перебором можно перечислить аккаунты, не зная ни одного пароля. +-- Осмысленный ответ полагается только тому, кто пароль уже доказал. +-- text + CHECK, а не enum-тип: добавить четвёртое состояние — это ALTER одного констрейнта в +-- обычной миграции, тогда как ALTER TYPE ... ADD VALUE нельзя использовать в той же +-- транзакции, где значение добавлено (PG16), и enum тянет за собой отдельный тип в дампах. +-- Enum-типов в репозитории нет вовсе — не заводим первый ради трёх значений. +-- +-- WHAT: +-- 1. role — text NOT NULL + CHECK ('admin','manager','employee'). Тип, набор значений +-- и отсутствие DEFAULT — зеркало tradein_users.role (м.192:42). +-- 2. manager_id — self-FK ON DELETE SET NULL + иерархический CHECK + запрет self-manager + +-- partial index. Зеркало м.192:43/50-52/84-86, чтобы код «Меры» переехал на +-- auth.users без правок. +-- 3. access_state — text NOT NULL DEFAULT 'active' + CHECK на три значения; backfill из +-- is_active, точечный перевод user2 («Брусника») в trial_expired, затем +-- DROP COLUMN is_active. +-- 4. Гранты auth_app — INSERT на users (DELETE НЕ выдаётся) + сужение табличного UPDATE (002:80) до +-- column-level: новые колонки role/access_state не должны попасть под него +-- молча. +-- +-- IDEMPOTENCY: +-- ADD COLUMN IF NOT EXISTS / DROP COLUMN IF EXISTS / CREATE INDEX IF NOT EXISTS; констрейнты — +-- через DO-блок с проверкой pg_constraint (в PostgreSQL нет ADD CONSTRAINT IF NOT EXISTS для +-- CHECK/FK, паттерн из м.193:80-90); GRANT идемпотентен по определению; UPDATE-backfill'ы +-- отфильтрованы так, что второй прогон не находит строк (детали у каждого блока). +-- Проверка pg_constraint здесь фильтрует ДОПОЛНИТЕЛЬНО по conrelid (в отличие от м.193, где +-- только conname): имена констрейнтов уникальны в пределах таблицы, а не БД — одноимённый +-- констрейнт на соседней таблице заставил бы миграцию молча пропустить создание своего. +-- +-- ⚠️ ПОСЛЕ 004 ФАЙЛЫ 001 И 003 БОЛЬШЕ НЕ ПЕРЕИГРЫВАЮТСЯ ПООТДЕЛЬНОСТИ. +-- Обе ссылаются на колонку is_active, которой после этой миграции нет, и обе падают на уже +-- мигрированной БД с «column is_active does not exist»: +-- * 001 — на `COMMENT ON COLUMN users.is_active` (001:75). CREATE TABLE IF NOT EXISTS +-- пропускается, а COMMENT выполняется всегда — то есть ручной `psql -f 001` падает +-- РАНЬШЕ 003, вопреки интуиции «ломается только сид». +-- * 003 — на INSERT со списком колонок, включающим is_active (а если бы и не упал — +-- role NOT NULL без DEFAULT не даст вставить строку). +-- Это следствие требования «применённые миграции не правим», а не регресс. Поддерживаемый +-- сценарий восстановления — прогон каталога ЦЕЛИКОМ по возрастанию номеров (001→002→003→004) +-- на пустой БД; он рабочий, порядок гарантирован сортировкой имён в deploy.yml. Нужно добить +-- сид на живой БД — пиши новый файл 00N, не переигрывай 003. +-- +-- Dependencies: 001_identity_schema.sql (users), 002_auth_app_role.sql (роль auth_app — гранты +-- Части 4 её предполагают), 003_users_seed.sql (13 строк, которым backfill проставляет role). +-- Deploy order: применяется на прод авто-циклом deploy.yml по data/sql/auth/*.sql. Python-кода в +-- этом PR нет и поведение прода не меняется — в БД `auth` пока никто не ходит; код логина, +-- чтение role/access_state и удаление tradein_users — отдельные PR'ы ПОСЛЕ (см. +-- .claude/rules/sql.md «Migration order»: схема первой). + +BEGIN; + +-- --------------------------------------------------------------------------------------------- +-- Часть 1: role +-- --------------------------------------------------------------------------------------------- +-- DEFAULT сознательно НЕТ (как в м.192): роль — осознанное решение того, кто заводит человека. +-- С дефолтом INSERT, забывший указать роль, тихо создал бы работающий аккаунт с полномочиями +-- «по умолчанию»; без дефолта он падает на NOT NULL — это и есть нужное поведение. +-- Колонка добавляется NULLable, заполняется backfill'ом ниже и только потом получает NOT NULL: +-- прямой ADD COLUMN ... NOT NULL без DEFAULT упал бы на 13 уже существующих строках сида. +ALTER TABLE users ADD COLUMN IF NOT EXISTS role text; + +-- Backfill. Источник истины — м.193:101-113 (org-карта владельца продукта от 2026-07-30), +-- сверено построчно по файлу, не по памяти. Роли не являются секретом: они уже лежат в git +-- (м.193 и auth/roles.yaml) — запрет на git касается паролей и хешей, не полномочий. +-- `role IS NULL` в каждом WHERE даёт сразу две вещи: идемпотентность (второй прогон не находит +-- строк) и защиту от отката ручных решений — повышение сотрудника до manager, сделанное после +-- первого прогона, повторным применением файла не вернётся к seed-значению. +UPDATE users SET role = 'admin' WHERE role IS NULL AND username = 'admin'; +UPDATE users SET role = 'manager' WHERE role IS NULL AND username IN ('kopylov', 'praktika'); +-- Catch-all — ПОСЛЕДНИМ и именно employee: любая строка, попавшая в auth.users мимо сида +-- (ручная вставка, восстановление из дампа, будущий аккаунт), получает НАИМЕНЕЕ +-- привилегированную роль. Fail-safe: ошибка в этом месте не должна раздавать admin. +UPDATE users SET role = 'employee' WHERE role IS NULL; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'users_role_ck' AND conrelid = 'users'::regclass + ) THEN + ALTER TABLE users + ADD CONSTRAINT users_role_ck CHECK (role IN ('admin', 'manager', 'employee')); + END IF; +END $$; + +-- SET NOT NULL идемпотентен (на уже NOT NULL колонке — no-op) и стоит ПОСЛЕ backfill: на строке +-- с NULL он упал бы, а catch-all выше гарантирует, что таких строк не осталось. +ALTER TABLE users ALTER COLUMN role SET NOT NULL; + +-- --------------------------------------------------------------------------------------------- +-- Часть 2: manager_id (org-иерархия) +-- --------------------------------------------------------------------------------------------- +-- FK и CHECK объявлены ОТДЕЛЬНЫМИ шагами, а не inline в ADD COLUMN (как в м.192, где это было +-- частью CREATE TABLE IF NOT EXISTS — «всё или ничего»). Причина: `ADD COLUMN IF NOT EXISTS ... +-- REFERENCES ...` пропускает ВЕСЬ оператор, если колонка уже есть, — на БД, где manager_id +-- когда-то завели руками без FK, миграция отчиталась бы об успехе и оставила связь без +-- ссылочной целостности. Раздельные идемпотентные шаги такого состояния не допускают. +-- Имя FK задано явно тем же, которое сгенерировал бы PostgreSQL для inline-формы, — чтобы схема +-- на проде и схема из чистой сборки не различались именами констрейнтов. +ALTER TABLE users ADD COLUMN IF NOT EXISTS manager_id bigint; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'users_manager_id_fkey' AND conrelid = 'users'::regclass + ) THEN + -- ON DELETE SET NULL (зеркало м.192:43): удаление менеджера не должно каскадом сносить + -- его сотрудников — они остаются в реестре без привязки, и это чинится назначением + -- нового менеджера, а не восстановлением строк из бэкапа. + ALTER TABLE users + ADD CONSTRAINT users_manager_id_fkey + FOREIGN KEY (manager_id) REFERENCES users(id) ON DELETE SET NULL; + END IF; +END $$; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'users_role_manager_hierarchy_ck' AND conrelid = 'users'::regclass + ) THEN + ALTER TABLE users + ADD CONSTRAINT users_role_manager_hierarchy_ck CHECK ( + role NOT IN ('admin', 'manager') OR manager_id IS NULL + ); + END IF; +END $$; + +-- Запрет self-manager. users_role_manager_hierarchy_ck выше держит только admin/manager; для +-- employee self-FK допускает ссылку строки на саму себя, и `UPDATE users SET manager_id = id` +-- прошёл бы. Через сегодняшний API это недостижимо (team.py:398-406 требует role='manager' у +-- цели, PATCH manager_id вообще не меняет), но 004 делает auth.users ЕДИНСТВЕННЫМ реестром — в +-- него начнёт писать и «Птица», у которой этой валидации нет, а любой будущий WITH RECURSIVE по +-- manager_id на такой строке зациклится. Строчный CHECK ловит самый вероятный случай (опечатка +-- или копипаста собственного id) и стоит ноль. +-- Чего этот констрейнт НЕ ловит: взаимную пару employee↔employee (A.manager_id=B, +-- B.manager_id=A) и ссылку на строку с role<>'manager' — оба требуют чтения ДРУГОЙ строки, +-- строчным CHECK'ом это не выражается (нужен триггер или FK на несуществующий уникальный ключ +-- (id, role)). Инвариант зафиксирован COMMENT'ом к колонке — он живёт в приложении. +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'users_manager_not_self_ck' AND conrelid = 'users'::regclass + ) THEN + ALTER TABLE users + ADD CONSTRAINT users_manager_not_self_ck CHECK ( + manager_id IS NULL OR manager_id <> id + ); + END IF; +END $$; + +-- Partial index (зеркало м.192:84-86): у admin/manager и у свободных слотов manager_id = NULL, +-- и эти строки никогда не участвуют в выборке «сотрудники этого менеджера». Индексировать NULL'ы +-- значит платить за большую часть таблицы, которая по этому пути не читается. +CREATE INDEX IF NOT EXISTS users_manager_id_idx + ON users (manager_id) + WHERE manager_id IS NOT NULL; + +-- --------------------------------------------------------------------------------------------- +-- Часть 3: access_state вместо is_active +-- --------------------------------------------------------------------------------------------- +-- DEFAULT 'active' здесь, в отличие от role, уместен: «доступ есть» — это состояние, в котором +-- заводят любого нового сотрудника, и молчаливый дефолт не расширяет ничьих полномочий. +ALTER TABLE users ADD COLUMN IF NOT EXISTS access_state text NOT NULL DEFAULT 'active'; + +-- CHECK ставится СРАЗУ после колонки, до backfill'а: тогда он проверяет и сам backfill — +-- опечатка в значении ниже уронит миграцию, а не просочится в данные. +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'users_access_state_ck' AND conrelid = 'users'::regclass + ) THEN + ALTER TABLE users + ADD CONSTRAINT users_access_state_ck CHECK ( + access_state IN ('active', 'trial_expired', 'disabled') + ); + END IF; +END $$; + +-- Backfill из is_active — под проверкой существования колонки, потому что в конце этого же +-- блока она удаляется: повторный прогон файла обязан пройти без ошибок, а прямое обращение к +-- несуществующей колонке — ошибка парсинга, не «0 строк». +-- EXECUTE (динамический SQL), а не обычные UPDATE внутри IF: обычные операторы уцелели бы лишь +-- благодаря ленивой подготовке операторов в PL/pgSQL (невыполненная ветка не разбирается). Это +-- рабочая, но недокументированная в самом файле деталь реализации; EXECUTE делает независимость +-- от отсутствующей колонки явной для читателя. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM pg_attribute + WHERE attrelid = 'users'::regclass + AND attname = 'is_active' + AND NOT attisdropped + ) THEN + -- Механическое отображение старой семантики: булев «доступ закрыт» = жёсткая блокировка. + -- `access_state = 'active'` в WHERE — не мёртвое условие: оно фиксирует, что переписывается + -- только значение, доставшееся из DEFAULT, и никогда — уже осмысленно проставленное. + EXECUTE $q$ + UPDATE users + SET access_state = 'disabled' + WHERE is_active = false + AND access_state = 'active' + $q$; + + -- Точечно: user2 («Брусника», доступ закрыт владельцем 2026-07-30) — не disabled, а + -- trial_expired. Основание: в auth/roles.yaml у него role=expired, то есть исторически он + -- видит trial-экран, а не отказ входа; решение владельца от 2026-07-31 эту семантику + -- сохраняет. + -- Условие `access_state = 'disabled'` — это защита от затирания ручного решения: + -- переводится РОВНО то значение, которое механическая ветка выше только что и вывела. + -- Если к моменту повторного прогона владелец уже открыл «Бруснике» доступ (active) или + -- перевёл её в другое состояние, WHERE не сматчится и решение человека переживёт миграцию. + -- Безусловный UPDATE по username возвращал бы аккаунт в trial_expired после каждого + -- прогона, и разбор «почему у клиента снова экран пробного периода» стоил бы часов при + -- нулевой пользе. Хардкод одного username оправдан: это разовая фиксация конкретного + -- исторического факта, а не правило — общего признака «пробный доступ» в схеме до сих пор + -- не было, выводить его задним числом не из чего. + EXECUTE $q$ + UPDATE users + SET access_state = 'trial_expired' + WHERE username = 'user2' + AND access_state = 'disabled' + $q$; + END IF; +END $$; + +-- Снятие is_active. Деструктивный шаг — но именно он и есть смысл решения: оставить обе колонки +-- значило бы два источника правды о доступе, расходящихся при первой же правке через UI. +-- Безопасно: на момент этого PR БД `auth` не читается ни одним работающим кодом (Caddy basic_auth +-- + tradein_users по-прежнему обслуживают прод), а данные колонки полностью перенесены выше. +-- DROP обязан жить именно здесь, а не в 003: 003 применён на проде и правке не подлежит. +ALTER TABLE users DROP COLUMN IF EXISTS is_active; + +-- --------------------------------------------------------------------------------------------- +-- Часть 4: гранты auth_app под режим единственного реестра (отмена решения 002:22-33) +-- + сужение унаследованного табличного UPDATE до column-level +-- --------------------------------------------------------------------------------------------- +-- 002 намеренно не выдавала INSERT/DELETE на users, и её аргумент был верным для своего момента: +-- в PR-1 не существовало ни кода, ни UI создания аккаунтов, а грант «на будущее» — это открытая +-- операция, которой никто не пользуется и которую никто не тестирует. Аргумент перестаёт +-- применяться ровно сейчас: после полного переезда auth.users — единственный реестр людей, а +-- раздел «Команда» «Меры» (tradein-mvp/backend/app/api/v1/team.py: POST /employees заводит +-- сотрудника, PATCH правит) — единственный интерфейс, которым сотрудника заводят и убирают. +-- Без INSERT переезд физически не состоится: сегодняшний INSERT идёт в tradein_users, а её не +-- станет. +-- DELETE здесь НЕ выдаётся, хотя первая редакция этой миграции его содержала. Причина отказа: +-- DELETE-эндпоинта в team.py нет (только POST /employees и PATCH — проверено), то есть потребителя +-- у права нет ни одного, а 002:22-33 отклоняла ровно такие гранты-на-будущее. Симметричный +-- контраргумент («снять неиспользуемое право дешевле, чем добавлять его в момент релиза») здесь не +-- перевешивает: DELETE по users каскадит на sessions (001:94), то есть цена ошибки в коде выше +-- обычной, а добавить строку GRANT в миграцию того PR, где появится DELETE-хендлер, стоит ровно +-- столько же. Право выдаётся вместе с кодом, который им пользуется, — не раньше. +-- DELETE ≠ закрытие доступа. Закрытие — это access_state ('disabled' / 'trial_expired'): +-- обратимо, сохраняет строку и историю. Именно оно, а не удаление строки, закрывает сегодняшний +-- сценарий «Команды»; удаление понадобилось бы только чтобы убрать ошибочно заведённый слот. +GRANT INSERT ON users TO auth_app; + +-- Гранта на последовательность users_id_seq здесь НЕТ — и это не забывчивость. +-- 002:26-27 записала как факт, что «идентичность требует nextval», то есть INSERT из auth_app +-- якобы упадёт с «permission denied for sequence» без USAGE на последовательности. Для +-- `GENERATED ALWAYS AS IDENTITY` (001:53) это неверно: PostgreSQL подставляет не вызов +-- nextval('...'), а узел NextValueExpr, который дёргает nextval_internal(seqid, +-- check_permissions := false) — ACL последовательности не проверяется вовсе. Это документированное +-- отличие identity от serial, и оно проверено живьём на postgres:16, а не выведено из +-- документации: после `REVOKE ALL ON SEQUENCE users_id_seq FROM app` INSERT в identity-таблицу +-- прошёл и вернул id, тогда как в контрольной таблице с bigserial тот же INSERT в тех же +-- условиях упал ровно с «permission denied for sequence». +-- Отсюда два следствия. Первое: грант не нужен — он выдал бы auth_app право звать +-- nextval('users_id_seq') напрямую (жечь идентификаторы) и читать last_value (число заведённых +-- аккаунтов), при том что ни один путь кода этого не делает; это прямо противоречило бы +-- REVOKE ALL ON ALL SEQUENCES из 002:73. Второе: «живая проверка» вида «auth_app сделал INSERT, +-- значит грант рабочий» ничего не доказывает — тот же INSERT проходит и после REVOKE, поэтому +-- проверять надо обратное (REVOKE, затем INSERT). +-- Если users.id когда-нибудь переведут на обычный DEFAULT nextval(...) — грант станет +-- обязательным, и его придётся добавить той же миграцией, что меняет колонку. + +-- Сужение UPDATE до column-level. 002:80 выдала ТАБЛИЧНЫЙ `GRANT SELECT, UPDATE ON users`, +-- обосновав его узко («смена пароля самим пользователем и проставление хеша админом»), — но +-- табличный UPDATE автоматически распространяется на любые колонки, добавленные позже. Не сузь +-- мы его здесь, auth_app молча получил бы право писать role и access_state, и периметр 002 +-- расширился бы ровно тем, что 004 добавила, без единой строки GRANT. +-- Почему это важно именно для этих двух колонок: любая SQL-инъекция или логическая ошибка в +-- UPDATE-эндпоинте (сегодня такой ровно один — team.py PATCH /employees, COALESCE-список полей +-- по WHERE id = :id) из «испортил профиль» превращалась бы в `SET role='admin' WHERE id=<свой>` +-- или `SET access_state='active' WHERE username='user2'` — тихое повышение до админа и тихое +-- снятие блокировки, без смены пароля, то есть без внешнего признака компрометации. Это ровно +-- тот класс, ради которого 002 и заводила отдельную роль (002:5-6). +-- role в список НЕ включена сознательно: сегодня её не пишет никто (team.py POST вставляет +-- литерал 'employee', PATCH в SET-списке role/manager_id не имеет вовсе). Появится админский +-- путь смены роли — добавится одной строкой новой миграции; это дешевле, чем держать открытым +-- право на эскалацию привилегий «на всякий случай». +-- manager_id по той же причине не включён: назначение сотрудника менеджеру сегодня делается +-- только при создании (INSERT), а не UPDATE'ом. +-- access_state включён — блокировка/разблокировка через «Команду» (сегодняшний +-- `is_active = COALESCE(...)` в PATCH) переезжает именно в эту колонку. +-- REVOKE перед GRANT обязателен и идемпотентен: REVOKE табличной привилегии снимает и +-- колоночные, поэтому повторный прогон файла даёт то же состояние (внутри одной транзакции, +-- то есть без окна «прав нет» для работающего приложения). +REVOKE UPDATE ON users FROM auth_app; +GRANT UPDATE (password_hash, display_name, org_name, email, access_state, updated_at) + ON users TO auth_app; + +-- --------------------------------------------------------------------------------------------- +-- COMMENT'ы: переписываем то, что 004 сделала неверным в 001 +-- --------------------------------------------------------------------------------------------- +COMMENT ON TABLE users IS + 'Единый реестр людей для «Меры» (trade-in) и «Птицы» (Site Finder): идентичность И ' + 'полномочия. Решение владельца продукта 2026-07-31 — ПОЛНЫЙ переезд: tradein_users ' + 'удаляется, второго реестра не будет. Прежняя формулировка («роли остаются в продуктовых ' + 'БД», 001) отменена миграцией 004 — см. её заголовок.'; + +COMMENT ON COLUMN users.role IS + 'Полномочия: admin | manager | employee. Зеркало tradein_users.role (tradein м.192) — код ' + '«Меры» должен переехать на эту таблицу без правок в проверках роли. DEFAULT намеренно нет: ' + 'роль выбирает тот, кто заводит человека; INSERT без роли обязан падать, а не создавать ' + 'аккаунт с полномочиями «по умолчанию».'; + +COMMENT ON COLUMN users.manager_id IS + 'Self-FK на users(id), ON DELETE SET NULL: удаление менеджера оставляет его сотрудников в ' + 'реестре без привязки, а не сносит их каскадом. NULL для admin/manager (top-level роли, ' + 'констрейнт users_role_manager_hierarchy_ck) и для employee без организации. ' + 'ИНВАРИАНТЫ, КОТОРЫЕ БД НЕ ПРОВЕРЯЕТ (обязан держать КАЖДЫЙ пишущий сюда код — реестр общий ' + 'для «Меры» и «Птицы»): цель ссылки обязана иметь role = ''manager''; циклы (A→B, B→A) ' + 'запрещены — рекурсивный обход иерархии на них зациклится. Схемой ловится только ссылка ' + 'строки на саму себя (users_manager_not_self_ck): остальное требует чтения другой строки и ' + 'строчным CHECK не выражается. Отсутствие проверки в БД — не разрешение.'; + +COMMENT ON COLUMN users.access_state IS + 'Состояние доступа, три значения — заменило булев is_active (миграция 004). ' + 'active: вход разрешён. ' + 'trial_expired: пробный период истёк — при ВЕРНОМ пароле логин отвечает 403 с отдельным ' + 'кодом и текстом «пробный доступ закончился», сессия не выдаётся (аккаунт видит осмысленный ' + 'экран, а не «неверный пароль»). ' + 'disabled: доступ закрыт — generic 401, неотличимо от неверного пароля. ' + 'Неверный пароль в любом состоянии → generic 401: иначе отдельный ответ для trial_expired ' + 'стал бы оракулом существования логина. Булев флаг схлопывал бы trial_expired и disabled в ' + 'одно значение, и trial-экран исчез бы молча. ' + 'ИНВАРИАНТ ДЛЯ API (в БД не выразим): перевод ПОСЛЕДНЕГО active-админа в любое другое ' + 'состояние обязан отклоняться на уровне приложения. Констрейнт с role не связан, ' + 'UPDATE ... SET access_state = ''disabled'' WHERE username = ''admin'' в БД проходит, а после ' + 'перехода на единую форму входа это self-lockout: не остаётся аккаунта, способного открыть ' + 'доступ обратно через UI, восстановление — только psql на прод-БД. Сегодня путь закрыт тем, ' + 'что «Команда» не отдаёт строки с role = ''admin'' никому (team.py); любой новый админский ' + 'экран, пишущий access_state, обязан проверку восстановить.'; + +COMMENT ON CONSTRAINT users_role_manager_hierarchy_ck ON users IS + 'admin/manager обязаны иметь manager_id IS NULL — это top-level роли, «начальника» у них в ' + 'этой модели нет (зеркало tradein м.192). Для employee manager_id любой, включая NULL ' + '(свободный слот без организации допустим).'; + +COMMENT ON CONSTRAINT users_manager_not_self_ck ON users IS + 'Строка не может быть собственным менеджером (manager_id <> id). Ловит опечатку/копипасту ' + 'id при ручной правке и у второго потребителя реестра («Птица»), где валидации «Команды» ' + 'нет. Взаимные пары и ссылку на не-менеджера строчный CHECK не ловит — см. COMMENT к ' + 'users.manager_id.'; + +COMMENT ON CONSTRAINT users_access_state_ck ON users IS + 'Фиксирует ровно три состояния доступа. Расширение — новой миграцией с ALTER этого ' + 'констрейнта; тип text + CHECK выбран вместо enum именно ради дешёвого расширения.'; + +COMMIT; -- 2.45.3