diff --git a/README.md b/README.md index 4f98aaa..a3f56f6 100644 --- a/README.md +++ b/README.md @@ -11,14 +11,14 @@ python3 -m venv venv && venv/bin/pip install -r requirements-dev.txt venv/bin/python scripts/seed_demo.py # по желанию: демо-данные из макетов ``` - UI: `http://<хост>:8088/`, Swagger: `http://<хост>:8088/docs`. -- Вход: `ADMIN_USERNAME` / `ADMIN_PASSWORD` из `.env`. Администратор создаётся при первом старте на пустой БД. +- Вход: `ADMIN_USERNAME` / `ADMIN_PASSWORD` из `.env`. Суперадминистратор создаётся при первом старте на пустой БД. ## Конфигурация (`.env`) | Переменная | Назначение | |---|---| | `POSTGRES_DB`, `POSTGRES_USER`, `POSTGRES_PASSWORD` | База данных | | `JWT_SECRET` | Ключ подписи токенов. Обязателен, не короче 32 символов, заглушки отклоняются: без корректного значения приложение не стартует | -| `ADMIN_USERNAME`, `ADMIN_PASSWORD` | Первый администратор; пароль не короче 8 символов | +| `ADMIN_USERNAME`, `ADMIN_PASSWORD` | Первый суперадминистратор; пароль не короче 8 символов | | `APP_PORT` | Порт UI и API на хосте (8088) | | `APP_BIND` | Адрес публикации порта: по умолчанию `0.0.0.0`; `127.0.0.1` — только за reverse-proxy | | `DB_HOST_PORT` | Порт PostgreSQL на хосте, публикуется только на `127.0.0.1` | @@ -27,8 +27,8 @@ venv/bin/python scripts/seed_demo.py # по желанию: демо-да ## Архитектура | Слой | Технологии | |---|---| -| API | FastAPI, pydantic v2, JWT (срок 8 ч), пароли в argon2, роли `admin` (запись) и `viewer` (чтение) | -| БД | PostgreSQL 16, SQLAlchemy 2, Alembic (миграции `0001`–`0009`), типы `CIDR`/`INET` | +| API | FastAPI, pydantic v2, JWT (срок 8 ч), пароли в argon2, роли `superadmin`, `admin`, `viewer` с привязкой к организации | +| БД | PostgreSQL 16, SQLAlchemy 2, Alembic (миграции `0001`–`0013`), типы `CIDR`/`INET` | | UI | Статический SPA (vanilla JS, ES-модуль) раздаётся приложением; шрифты IBM Plex хранятся локально, внешних зависимостей нет | ``` @@ -44,6 +44,11 @@ docs/changes/ планы и итоги доработок docs/reviews/ ## Модель данных и правила `organizations` → `vrfs` → `prefixes` (дерево) → `addresses`; `devices` + `device_types`; `isps` + `isp_networks`; `users`; `audit_log`. +**Пользователи и организации** +- `admin`/`viewer` привязаны к одной организации (`organization_id`), `superadmin` — ни к одной. БД допускает `admin`/`viewer` без организации только у отключённой записи (состояние после миграции 0011). +- Смена роли на `superadmin` снимает организацию автоматически; понижение до `admin`/`viewer` требует указать `organization_id`. +- Изоляция: `admin`/`viewer` видят и меняют только данные своей организации, включая «Обзор», журнал и счётчики типов устройств. Чужой объект неотличим от несуществующего — 404. + **VRF и префиксы** - У каждой организации автоматически создаётся VRF `default`. Имя VRF уникально в пределах организации без учёта регистра. - Префикс принадлежит VRF своей организации; это гарантирует составной FK в БД. @@ -62,6 +67,13 @@ docs/changes/ планы и итоги доработок docs/reviews/ **Удаление** - Объекты с зависимыми данными не удаляются (409). Отказ фиксируется в журнале с перечнем мешающих объектов. +- Организация не удаляется, если у неё есть привязанные пользователи. + +**Журнал аудита** +- Событие привязано к организации своей сущности (`organization_id`); `admin`/`viewer` видят только события своей организации, включая отклонённые удаления. +- Системные события (пользователи, вход, настройки и очистка журнала, типы устройств) не привязаны к организации и видны только `superadmin`. +- События организации показывают IP и User-Agent исполнителя, в том числе суперадминистратора. +- После удаления организации её события сохраняются с `organization_id = NULL`. ## API (`/api/v1`) | Область | Эндпоинты | @@ -82,9 +94,14 @@ docs/changes/ планы и итоги доработок docs/reviews/ ## Безопасность **Роли и учётные записи** -- `viewer` только читает. Раздел «Пользователи» доступен только `admin`. Роль по умолчанию — `viewer`. +| Роль | Права | +|---|---| +| `superadmin` | Все организации; пользователи, создание и удаление организаций, типы устройств, настройки и очистка журнала | +| `admin` | Запись в своей организации: VRF, префиксы, адреса, устройства, операторы, карточка организации | +| `viewer` | Чтение своей организации. Роль по умолчанию | + - Логин уникален без учёта регистра. -- Свою учётную запись нельзя понизить, отключить или удалить; то же относится к последнему активному администратору. +- Свою учётную запись нельзя понизить, отключить или удалить; последний активный `superadmin` защищён от понижения, отключения и удаления. **Пароли и токены** - Смена пароля отзывает ранее выданные токены. Свой пароль меняется только с подтверждением текущего. @@ -114,11 +131,13 @@ docs/changes/ планы и итоги доработок docs/reviews/ reverse_proxy 127.0.0.1:8088 } ``` +- Переход на ролевую модель (миграция 0011): прежние `admin` становятся `superadmin`, прежние `viewer` отключаются до назначения организации суперадминистратором. - Перед обновлением рабочей БД проверьте данные скриптами только для чтения: `scripts/find_duplicate_addresses.py` и `scripts/find_unusable_addresses.py`. - Имя compose-проекта задаётся флагом `-p`. Текущий стенд поднят как `ipam_control_006` (`docker compose -p ipam_control_006 …`); без флага команды работают с проектом `ipam_control`. ## Интерфейс -- Экраны: «Обзор», «Префиксы» (дерево по VRF), «Адреса» подсети, «Организации», «Операторы», «Устройства», «Журнал», «Пользователи» (только admin). +- Экраны: «Обзор», «Префиксы» (дерево по VRF), «Адреса» подсети, «Организации», «Операторы», «Устройства», «Журнал», «Пользователи» (только `superadmin`). +- Переключатель организации — только у `superadmin`; `admin`/`viewer` работают в своей организации. - Строка реестра кликабельна целиком. Действия над строкой — в меню «⋯». - Групповые операции через чекбоксы (кроме «Журнала»): удаление, смена типа устройств, статус префиксов и адресов, доступ пользователей. - Экран «Префиксы» загружает до 20 000 префиксов организации и сообщает, если загружены не все. @@ -164,6 +183,8 @@ docker compose -p ipam_control_006 up -d --build && venv/bin/python -m pytest -q | 028 | Экран «Префиксы» без усечения | [план](docs/changes/028-prefixes-ui-pagination/PLAN.md) · [итог](docs/changes/028-prefixes-ui-pagination/SUMMARY.md) | | 029 | Целостность дерева префиксов | [план](docs/changes/029-prefix-tree-lock/PLAN.md) · [итог](docs/changes/029-prefix-tree-lock/SUMMARY.md) | | 030 | Исправление замечаний ревью 025–029 | [план](docs/changes/030-review-fixes-025-029/PLAN.md) · [итог](docs/changes/030-review-fixes-025-029/SUMMARY.md) | +| 032 | Ролевая модель с привязкой к организации и суперадминистратором | [план](docs/changes/032-role-model-org-scope/PLAN.md) · [итог](docs/changes/032-role-model-org-scope/SUMMARY.md) | +| 033 | Исправление находок ревью 032 | [план](docs/changes/033-review-fixes-032/PLAN.md) · [итог](docs/changes/033-review-fixes-032/SUMMARY.md) | ## Отчёты ревью - [Ревью кодовой базы](docs/reviews/2026-09-26-codebase-review.md) (находки → изменения 011–023) @@ -171,3 +192,6 @@ docker compose -p ipam_control_006 up -d --build && venv/bin/python -m pytest -q - [Повторный анализ кодовой базы](docs/reviews/2026-09-26-codebase-review-2.md) (→ 025–029) - [Ревью и тестирование изменений 025–029](docs/reviews/2026-09-26-changes-025-029-review.md) (→ 030) - [Ревью и тестирование изменения 030](docs/reviews/2026-09-27-changes-030-review.md) +- [Пентест (чёрный ящик)](docs/reviews/2026-09-27-pentest.md) (→ план 031, не реализован) +- [Ревью и тестирование изменения 032](docs/reviews/2026-09-27-changes-032-review.md) (→ 033) +- [Ревью кодовой базы после 032](docs/reviews/2026-09-27-codebase-review.md) (→ 033) diff --git a/alembic/versions/0010_user_role_superadmin.py b/alembic/versions/0010_user_role_superadmin.py new file mode 100644 index 0000000..dc692cc --- /dev/null +++ b/alembic/versions/0010_user_role_superadmin.py @@ -0,0 +1,24 @@ +"""Добавление роли superadmin для пользователей без привязки к организации (изменение 032) + +Revision ID: 0010 +Revises: 0009 +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0010" +down_revision = "0009" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ALTER TYPE ... ADD VALUE должен быть вне транзакции; используем autocommit_block + with op.get_context().autocommit_block(): + op.execute(sa.text("ALTER TYPE user_role ADD VALUE IF NOT EXISTS 'superadmin'")) + + +def downgrade() -> None: + # PostgreSQL не позволяет удалять значения enum; downgrade не осуществляется + pass diff --git a/alembic/versions/0011_users_organization_scope.py b/alembic/versions/0011_users_organization_scope.py new file mode 100644 index 0000000..f032bac --- /dev/null +++ b/alembic/versions/0011_users_organization_scope.py @@ -0,0 +1,46 @@ +"""Добавление привязки пользователей к организациям: admin/viewer привязаны, superadmin нет (изменение 032) + +Revision ID: 0011 +Revises: 0010 +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +revision = "0011" +down_revision = "0010" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Добавляем колонку organization_id + op.add_column("users", sa.Column("organization_id", sa.Integer(), nullable=True)) + op.create_index(op.f("ix_users_organization_id"), "users", ["organization_id"], unique=False) + op.create_foreign_key(op.f("fk_users_organization_id_organizations"), "users", "organizations", ["organization_id"], ["id"]) + + # Backfill: согласно плану (изменение 032, решение 4) + # 1. Пользователи с ролью 'admin' (текущие администраторы) → superadmin, organization_id=NULL + # 2. Пользователи с ролью 'viewer' → organization_id=NULL, is_active=false (заблокированы) + op.execute(sa.text("UPDATE users SET role = 'superadmin', organization_id = NULL WHERE role = 'admin'")) + op.execute(sa.text("UPDATE users SET organization_id = NULL, is_active = false WHERE role = 'viewer'")) + + # Добавляем CHECK constraint ПОСЛЕ backfill (иначе может не пройти) + # Инвариант: is_active=false OR (role='superadmin')=(organization_id IS NULL) + # Ослабленный для неактивных — допускает временное состояние viewer без организации + op.create_check_constraint( + "ck_users_role_org_scope", + "users", + "is_active = false OR (role = 'superadmin') = (organization_id IS NULL)", + ) + + +def downgrade() -> None: + op.drop_constraint("ck_users_role_org_scope", "users", type_="check") + op.drop_constraint(op.f("fk_users_organization_id_organizations"), "users", type_="foreignkey") + op.drop_index(op.f("ix_users_organization_id"), table_name="users") + op.drop_column("users", "organization_id") + + # Backfill downgrade: вернуть superadmin → admin + op.execute(sa.text("UPDATE users SET role = 'admin' WHERE role = 'superadmin'")) diff --git a/alembic/versions/0012_audit_log_organization.py b/alembic/versions/0012_audit_log_organization.py new file mode 100644 index 0000000..f1e654e --- /dev/null +++ b/alembic/versions/0012_audit_log_organization.py @@ -0,0 +1,64 @@ +"""Добавление привязки записей журнала к организациям для изоляции по организациям (изменение 032) + +Revision ID: 0012 +Revises: 0011 +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0012" +down_revision = "0011" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Добавляем колонку organization_id в audit_log + op.add_column("audit_log", sa.Column("organization_id", sa.Integer(), nullable=True)) + op.create_index(op.f("ix_audit_log_organization_id"), "audit_log", ["organization_id"], unique=False) + op.create_foreign_key(op.f("fk_audit_log_organization_id_organizations"), "audit_log", "organizations", ["organization_id"], ["id"]) + + # Backfill: привязываем записи к организациям через entity_id + # Для сущностей, у которых есть organization_id (organization, vrf, device, isp, prefix), + # подтягиваем значение из таблицы сущности; для остальных остаётся NULL (видно только superadmin) + + # Для сущностей, которые уже удалены, organisation_id остаётся NULL + # (осознанный компромисс — не полная историческая реконструкция) + + # organization events + op.execute(sa.text( + "UPDATE audit_log SET organization_id = o.id " + "FROM organizations o WHERE audit_log.entity_type = 'organization' AND audit_log.entity_id = o.id" + )) + + # vrf events + op.execute(sa.text( + "UPDATE audit_log SET organization_id = v.organization_id " + "FROM vrfs v WHERE audit_log.entity_type = 'vrf' AND audit_log.entity_id = v.id" + )) + + # device events + op.execute(sa.text( + "UPDATE audit_log SET organization_id = d.organization_id " + "FROM devices d WHERE audit_log.entity_type = 'device' AND audit_log.entity_id = d.id" + )) + + # isp events + op.execute(sa.text( + "UPDATE audit_log SET organization_id = i.organization_id " + "FROM isps i WHERE audit_log.entity_type = 'isp' AND audit_log.entity_id = i.id" + )) + + # prefix events (через vrf) + op.execute(sa.text( + "UPDATE audit_log SET organization_id = v.organization_id " + "FROM prefixes p JOIN vrfs v ON p.vrf_id = v.id " + "WHERE audit_log.entity_type = 'prefix' AND audit_log.entity_id = p.id" + )) + + +def downgrade() -> None: + op.drop_constraint(op.f("fk_audit_log_organization_id_organizations"), "audit_log", type_="foreignkey") + op.drop_index(op.f("ix_audit_log_organization_id"), table_name="audit_log") + op.drop_column("audit_log", "organization_id") diff --git a/alembic/versions/0013_audit_log_org_fk_set_null.py b/alembic/versions/0013_audit_log_org_fk_set_null.py new file mode 100644 index 0000000..c375be1 --- /dev/null +++ b/alembic/versions/0013_audit_log_org_fk_set_null.py @@ -0,0 +1,29 @@ +"""FK audit_log.organization_id → ON DELETE SET NULL (изменение 033, находка №1) + +Revision ID: 0013 +Revises: 0012 +""" +from alembic import op + + +revision = "0013" +down_revision = "0012" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Без ondelete="SET NULL" удаление организации падает: строки её журнала (organization_id) блокируют DELETE + op.drop_constraint(op.f("fk_audit_log_organization_id_organizations"), "audit_log", type_="foreignkey") + op.create_foreign_key( + op.f("fk_audit_log_organization_id_organizations"), "audit_log", "organizations", + ["organization_id"], ["id"], ondelete="SET NULL", + ) + + +def downgrade() -> None: + op.drop_constraint(op.f("fk_audit_log_organization_id_organizations"), "audit_log", type_="foreignkey") + op.create_foreign_key( + op.f("fk_audit_log_organization_id_organizations"), "audit_log", "organizations", + ["organization_id"], ["id"], + ) diff --git a/app/api/v1/journal.py b/app/api/v1/journal.py index e35c76c..53eb129 100644 --- a/app/api/v1/journal.py +++ b/app/api/v1/journal.py @@ -12,8 +12,8 @@ from app import schemas as s from app.db import get_db from app.models import AuditLog, ClearAttempt, User from app.rotation import get_settings, rotate, save_settings -from app.security import admin_user, current_user, verify_password -from app.services import MAX_OFFSET, audit, commit, like_escape +from app.security import admin_user, current_user, superadmin_user, verify_password +from app.services import MAX_OFFSET, audit, commit, like_escape, require_org, scope_org # изменение 032 router = APIRouter(dependencies=[Depends(current_user)], tags=["journal"]) MAX_ATTEMPTS = 5 @@ -56,9 +56,10 @@ def _filtered(stmt, event_type: str, entity_type: str, actor: str, date_from: da @router.get("/audit", response_model=s.Page[s.AuditOut]) def list_audit( event_type: str = "", entity_type: str = "", actor: str = "", date_from: date | None = None, date_to: date | None = None, - q: str = "", client_ip: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), + q: str = "", client_ip: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), user: User = Depends(current_user), # изменение 032 ): stmt = _filtered(select(AuditLog), event_type, entity_type, actor, date_from, date_to, q, client_ip) + stmt = scope_org(stmt, AuditLog.organization_id, user) # изменение 032: фильтр по организации total = db.scalar(select(func.count()).select_from(stmt.subquery())) or 0 rows = db.scalars(stmt.order_by(AuditLog.id.desc()).limit(limit).offset(offset)).all() return s.Page(items=[s.AuditOut.from_row(r) for r in rows], total=total) @@ -72,9 +73,11 @@ class Summary(BaseModel): @router.get("/audit/summary", response_model=Summary) -def summary(db: Session = Depends(get_db)): +def summary(db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 cfg = get_settings(db) - total, oldest = db.execute(select(func.count(), func.min(AuditLog.ts))).one() + stmt = select(func.count(), func.min(AuditLog.ts)).select_from(AuditLog) + stmt = scope_org(stmt, AuditLog.organization_id, user) # изменение 032: фильтр по организации + total, oldest = db.execute(stmt).one() return Summary(total=total, oldest_ts=oldest, **cfg) @@ -85,9 +88,13 @@ class Facets(BaseModel): @router.get("/audit/facets", response_model=Facets) -def facets(db: Session = Depends(get_db)): - pairs = db.execute(select(AuditLog.entity_type, AuditLog.action).distinct().order_by(AuditLog.entity_type, AuditLog.action)).all() - users = db.scalars(select(AuditLog.username).distinct().order_by(AuditLog.username)).all() +def facets(db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 + stmt = select(AuditLog.entity_type, AuditLog.action).distinct() + stmt = scope_org(stmt, AuditLog.organization_id, user) # изменение 032: фильтр по организации + pairs = db.execute(stmt.order_by(AuditLog.entity_type, AuditLog.action)).all() + stmt2 = select(AuditLog.username).distinct() + stmt2 = scope_org(stmt2, AuditLog.organization_id, user) # изменение 032: фильтр по организации + users = db.scalars(stmt2.order_by(AuditLog.username)).all() return Facets( event_types=[f"{e}.{a}" for e, a in pairs], entity_types=sorted({e for e, _ in pairs}), actors=[u if u in ("system", "anonymous") else f"ui:{u}" for u in users], @@ -95,10 +102,11 @@ def facets(db: Session = Depends(get_db)): @router.get("/audit/{uid}", response_model=s.AuditOut) -def get_entry(uid: uuid.UUID, db: Session = Depends(get_db)): +def get_entry(uid: uuid.UUID, db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 row = db.scalar(select(AuditLog).where(AuditLog.uid == uid)) if row is None: raise HTTPException(404, "Запись не найдена") + require_org(user, row.organization_id, "Запись журнала") # изменение 032 return s.AuditOut.from_row(row) @@ -118,7 +126,7 @@ class SettingsSaved(JournalSettings): @router.put("/journal/settings", response_model=SettingsSaved) -def update_settings(body: JournalSettings, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_settings(body: JournalSettings, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin old = get_settings(db) new = body.model_dump() save_settings(db, new) @@ -149,7 +157,7 @@ def _lock_state(db: Session, user_id: int) -> tuple[int, int]: @router.post("/journal/clear") -def clear_journal(body: ClearIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def clear_journal(body: ClearIn, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin used, retry = _lock_state(db, user.id) if retry: audit(db, user, "journal", None, "clear_locked", "clear", {"retry_after_seconds": retry}, message="Очистка журнала: попытка во время блокировки") diff --git a/app/api/v1/overview.py b/app/api/v1/overview.py index e03c090..8dc76ab 100644 --- a/app/api/v1/overview.py +++ b/app/api/v1/overview.py @@ -6,9 +6,9 @@ from sqlalchemy.orm import Session from app import schemas as s from app.api.v1.prefixes import _prefix_outs from app.db import get_db -from app.models import Address, AddressStatus, AuditLog, Prefix, PrefixStatus, Vrf +from app.models import Address, AddressStatus, AuditLog, Prefix, PrefixStatus, User, Vrf from app.security import current_user -from app.services import capacity, utilization +from app.services import capacity, scope_org, utilization # изменение 032 router = APIRouter(dependencies=[Depends(current_user)]) @@ -39,14 +39,17 @@ def _ipv4_roots(db: Session): @router.get("/overview", response_model=Overview, tags=["overview"]) -def overview(db: Session = Depends(get_db)): - active = db.scalars(select(Prefix).where(Prefix.status == PrefixStatus.active)).all() +def overview(db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 + stmt_prefix = select(Prefix).where(Prefix.status == PrefixStatus.active) + stmt_prefix = scope_org(stmt_prefix, Prefix.organization_id, user) # изменение 032 + active = db.scalars(stmt_prefix).all() outs = _prefix_outs(db, list(active)) parents = {p.parent_id for p in outs if p.parent_id} leaves4 = [p for p in outs if p.id not in parents and p.family == 4] # top_prefixes — как раньше, по листьям top = sorted((p for p in leaves4 if p.capacity > 1), key=lambda p: p.utilization, reverse=True)[:4] roots_stmt = _ipv4_roots(db) + roots_stmt = scope_org(roots_stmt, Prefix.organization_id, user) # изменение 032 roots = db.execute(roots_stmt).all() cap = sum(capacity(str(r.prefix)) for r in roots) roots_cte = roots_stmt.cte("overview_roots") @@ -58,10 +61,16 @@ def overview(db: Session = Depends(get_db)): ).all()) assigned = counts.get(AddressStatus.assigned, 0) reserved = counts.get(AddressStatus.reserved, 0) - recent = db.scalars(select(AuditLog).order_by(AuditLog.id.desc()).limit(4)).all() + stmt_audit = select(AuditLog) + stmt_audit = scope_org(stmt_audit, AuditLog.organization_id, user) # изменение 032: фильтр по организации + recent = db.scalars(stmt_audit.order_by(AuditLog.id.desc()).limit(4)).all() + stmt_prefix_count = select(func.count()).select_from(Prefix) + stmt_prefix_count = scope_org(stmt_prefix_count, Prefix.organization_id, user) # изменение 032 + stmt_vrf_count = select(func.count()).select_from(Vrf) + stmt_vrf_count = scope_org(stmt_vrf_count, Vrf.organization_id, user) # изменение 032 return Overview( - prefixes=db.scalar(select(func.count()).select_from(Prefix)) or 0, - vrfs=db.scalar(select(func.count()).select_from(Vrf)) or 0, + prefixes=db.scalar(stmt_prefix_count) or 0, + vrfs=db.scalar(stmt_vrf_count) or 0, assigned=assigned, capacity=cap, utilization=utilization(assigned, cap), reserved=reserved, top_prefixes=top, recent_changes=[s.AuditOut.from_row(r) for r in recent], ) diff --git a/app/api/v1/prefixes.py b/app/api/v1/prefixes.py index 1393799..c7e9160 100644 --- a/app/api/v1/prefixes.py +++ b/app/api/v1/prefixes.py @@ -10,7 +10,7 @@ from app.models import Address, AddressStatus, Device, Organization, Prefix, Pre from app.security import admin_user, current_user from app.services import ( MAX_OFFSET, apply_update, audit, blockers, capacity, commit, count, flush, contains, free_page, get_or_404, network_role, next_free_address, next_free_subnet, refuse_delete, - utilization, + require_org, scope_org, utilization, # изменение 032 ) router = APIRouter(dependencies=[Depends(current_user)], tags=["prefixes"]) @@ -170,9 +170,10 @@ def _subtree(db: Session, root: Prefix) -> list[Prefix]: return result -def _move_to_vrf(db: Session, p: Prefix, vrf_id: int) -> tuple[dict, int]: +def _move_to_vrf(db: Session, p: Prefix, vrf_id: int, user: User) -> tuple[dict, int]: """Переносит префикс с поддеревом в другой VRF той же организации; возвращает (diff, число префиксов).""" vrf = get_or_404(db, Vrf, vrf_id, "VRF") + require_org(user, vrf.organization_id, "VRF") # изменение 033, находка №11 if vrf.organization_id != p.organization_id: raise HTTPException(422, "Целевой VRF принадлежит другой организации") subtree = _subtree(db, p) @@ -205,11 +206,14 @@ def _move_to_vrf(db: Session, p: Prefix, vrf_id: int) -> tuple[dict, int]: def list_prefixes( organization_id: int | None = None, vrf_id: int | None = None, status: PrefixStatus | None = None, family: int | None = Query(None, ge=4, le=6), q: str = "", - limit: int = Query(100, ge=1, le=1000), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), + limit: int = Query(100, ge=1, le=1000), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), user: User = Depends(current_user), # изменение 032 ): stmt = select(Prefix) if organization_id: + require_org(user, organization_id, "Организация") # изменение 032 stmt = stmt.where(Prefix.organization_id == organization_id) + else: + stmt = scope_org(stmt, Prefix.organization_id, user) # изменение 032 if vrf_id: stmt = stmt.where(Prefix.vrf_id == vrf_id) if status: @@ -224,14 +228,18 @@ def list_prefixes( @router.get("/prefixes/{id}", response_model=s.PrefixOut) -def get_prefix(id: int, db: Session = Depends(get_db)): - return _prefix_outs(db, [get_or_404(db, Prefix, id, "Префикс")])[0] +def get_prefix(id: int, db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 + p = get_or_404(db, Prefix, id, "Префикс") + require_org(user, p.organization_id, "Префикс") # изменение 032 + return _prefix_outs(db, [p])[0] @router.post("/prefixes", response_model=s.PrefixOut, status_code=201) def create_prefix(body: s.PrefixIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): + require_org(user, body.organization_id, "Организация") # изменение 033, находка №11: до _lock_vrf и чтения чужого VRF _lock_vrf(db, body.vrf_id) # до любых чтений дерева VRF (изменение 029) vrf = get_or_404(db, Vrf, body.vrf_id, "VRF") + require_org(user, vrf.organization_id, "VRF") # изменение 033, находка №11 if vrf.organization_id != body.organization_id: raise HTTPException(422, "VRF принадлежит другой организации") get_or_404(db, Organization, body.organization_id, "Организация") @@ -241,6 +249,7 @@ def create_prefix(body: s.PrefixIn, db: Session = Depends(get_db), user: User = parent_id = body.parent_id if parent_id is not None: parent = get_or_404(db, Prefix, parent_id, "Родительский префикс") + require_org(user, parent.organization_id, "Родительский префикс") # изменение 033, находка №11 if parent.vrf_id != vrf.id or not ipaddress.ip_network(body.prefix).subnet_of(ipaddress.ip_network(str(parent.prefix))): raise HTTPException(422, "Родительский префикс должен содержать новый и быть в том же VRF") p = Prefix(**{**body.model_dump(), "parent_id": parent_id}) @@ -253,7 +262,7 @@ def create_prefix(body: s.PrefixIn, db: Session = Depends(get_db), user: User = db.rollback() raise HTTPException(422, msg) moved = rehome_addresses(db, p) - audit(db, user, "prefix", p, "created", str(p.prefix), {"vrf": vrf.name, **({"moved_addresses": moved} if moved else {})}) + audit(db, user, "prefix", p, "created", str(p.prefix), {"vrf": vrf.name, **({"moved_addresses": moved} if moved else {})}, organization_id=body.organization_id) # изменение 032: organization_id commit(db, "Такой префикс уже есть в этом VRF") return _prefix_outs(db, [p])[0] @@ -278,9 +287,11 @@ def _find_subnet(db: Session, parent: Prefix, length: int) -> tuple[str | None, @router.get("/prefixes/{id}/subnets/next", response_model=s.SubnetPreview) -def preview_subnet(id: int, length: int = Query(ge=1, le=128), db: Session = Depends(get_db)): +def preview_subnet(id: int, length: int = Query(ge=1, le=128), db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 033 """Предпросмотр: какой блок будет выделен, без создания.""" - found, lo, hi = _find_subnet(db, get_or_404(db, Prefix, id, "Префикс"), length) + p = get_or_404(db, Prefix, id, "Префикс") + require_org(user, p.organization_id, "Префикс") # изменение 033, находка №9: не хватало проверки организации + found, lo, hi = _find_subnet(db, p, length) return s.SubnetPreview(prefix=found, length_min=lo, length_max=hi) @@ -288,6 +299,7 @@ def preview_subnet(id: int, length: int = Query(ge=1, le=128), db: Session = Dep def allocate_subnet(id: int, body: s.SubnetNextIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): """Создаёт вложенный префикс заданного размера в первом свободном выровненном блоке родителя.""" parent = _lock_prefix(db, id) # до любых чтений дерева (изменение 029/030) — сериализует параллельные выделения из одного родителя + require_org(user, parent.organization_id, "Префикс") # изменение 032 found, _, _ = _find_subnet(db, parent, body.length) if found is None: raise HTTPException(409, f"В префиксе {parent.prefix} нет свободного блока /{body.length}") @@ -297,7 +309,7 @@ def allocate_subnet(id: int, body: s.SubnetNextIn, db: Session = Depends(get_db) flush(db, "Такой префикс уже есть в этом VRF, повторите запрос") attach_to_tree(db, p, keep_parent=True) moved = rehome_addresses(db, p) # адреса родителя из выделенного блока (учтены и при выборе блока, но вдруг появились параллельно) - audit(db, user, "prefix", p, "created", found, {"vrf": parent.vrf.name, "allocated_from": str(parent.prefix), **({"moved_addresses": moved} if moved else {})}) + audit(db, user, "prefix", p, "created", found, {"vrf": parent.vrf.name, "allocated_from": str(parent.prefix), **({"moved_addresses": moved} if moved else {})}, organization_id=parent.organization_id) # изменение 032: organization_id commit(db, "Такой префикс уже есть в этом VRF, повторите запрос") return _prefix_outs(db, [p])[0] @@ -310,10 +322,11 @@ def update_prefix(id: int, body: s.PrefixUpdate, db: Session = Depends(get_db), # раньше чем прочитан текущий p.vrf_id (изменение 030, ревью 025-029 находка №1); если new_vrf совпадёт # с текущим VRF, лишняя блокировка того же VRF безвредна p = _lock_prefix(db, id, new_vrf) if new_vrf is not None else get_or_404(db, Prefix, id, "Префикс") + require_org(user, p.organization_id, "Префикс") # изменение 032 changed = {k: str(v) for k, v in apply_update(p, data).items()} if new_vrf is not None and new_vrf != p.vrf_id: - changed.update(_move_to_vrf(db, p, new_vrf)[0]) - audit(db, user, "prefix", p, "updated", str(p.prefix), changed) + changed.update(_move_to_vrf(db, p, new_vrf, user)[0]) + audit(db, user, "prefix", p, "updated", str(p.prefix), changed, organization_id=p.organization_id) # изменение 032: organization_id commit(db) return _prefix_outs(db, [p])[0] @@ -321,12 +334,13 @@ def update_prefix(id: int, body: s.PrefixUpdate, db: Session = Depends(get_db), @router.delete("/prefixes/{id}", status_code=204) def delete_prefix(id: int, force: bool = False, db: Session = Depends(get_db), user: User = Depends(admin_user)): p = _lock_prefix(db, id) # до переподвешивания детей: устаревший p.parent_id иначе достанется всем детям (изменение 029/030) + require_org(user, p.organization_id, "Префикс") # изменение 032 used = None if force else blockers(db, select(func.host(Address.address)).where(Address.prefix_id == id).order_by(Address.address)) if used: - refuse_delete(db, user, "prefix", p, str(p.prefix), "В префиксе есть адреса; удалите их или используйте force=true", {"addresses": used}) + refuse_delete(db, user, "prefix", p, str(p.prefix), "В префиксе есть адреса; удалите их или используйте force=true", {"addresses": used}, organization_id=p.organization_id) # изменение 033, находка №4 db.execute(update(Prefix).where(Prefix.parent_id == id).values(parent_id=p.parent_id)) gone = count(db, select(Address.id).where(Address.prefix_id == id)) if force else 0 - audit(db, user, "prefix", p, "deleted", str(p.prefix), {"force": True, "addresses_deleted": gone} if gone else None) + audit(db, user, "prefix", p, "deleted", str(p.prefix), {"force": True, "addresses_deleted": gone} if gone else None, organization_id=p.organization_id) # изменение 032: organization_id db.delete(p) commit(db) @@ -340,9 +354,10 @@ def _addr_out(a: Address, device_name: str | None = None) -> s.AddressOut: ) -def _check_device(db: Session, prefix: Prefix, device_id: int | None): +def _check_device(db: Session, user: User, prefix: Prefix, device_id: int | None): if device_id is not None: d = get_or_404(db, Device, device_id, "Устройство") + require_org(user, d.organization_id, "Устройство") # изменение 033, находка №11 if d.organization_id != prefix.organization_id: raise HTTPException(422, "Устройство принадлежит другой организации") @@ -350,11 +365,12 @@ def _check_device(db: Session, prefix: Prefix, device_id: int | None): @router.get("/prefixes/{id}/addresses", response_model=s.AddressPage) def list_addresses( id: int, status: str = "", q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), - db: Session = Depends(get_db), + db: Session = Depends(get_db), user: User = Depends(current_user), # изменение 032 ): """status: assigned | reserved | deprecated | free | пусто (все; свободные подмешиваются для малых подсетей). Пагинация — в SQL; страница «свободных» считается арифметически (без перебора адресов подсети).""" p = get_or_404(db, Prefix, id, "Префикс") + require_org(user, p.organization_id, "Префикс") # изменение 032 cap = capacity(str(p.prefix)) counts = dict(db.execute(select(Address.status, func.count()).where(Address.prefix_id == id).group_by(Address.status)).all()) stored = sum(counts.values()) @@ -394,6 +410,7 @@ def list_addresses( @router.post("/prefixes/{id}/addresses", response_model=s.AddressOut, status_code=201) def create_address(id: int, body: s.AddressIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): p = _lock_prefix(db, id) # выбор самого узкого префикса должен видеть согласованное дерево VRF (изменение 029/030) + require_org(user, p.organization_id, "Префикс") # изменение 032 net, ip = ipaddress.ip_network(str(p.prefix)), ipaddress.ip_address(body.address) if ip not in net: raise HTTPException(422, f"Адрес {body.address} не принадлежит префиксу {p.prefix}") @@ -403,11 +420,11 @@ def create_address(id: int, body: s.AddressIn, db: Session = Depends(get_db), us narrowest = _narrowest_prefix(db, p.vrf_id, body.address) if narrowest is not None and narrowest.id != p.id: # адрес хранится в самом узком префиксе VRF raise HTTPException(422, f"Адрес {body.address} принадлежит вложенному префиксу {narrowest.prefix}, назначьте его там") - _check_device(db, p, body.device_id) + _check_device(db, user, p, body.device_id) a = Address(prefix_id=id, vrf_id=p.vrf_id, **body.model_dump()) db.add(a) flush(db, "Адрес уже есть в этом префиксе") - audit(db, user, "address", a, "assigned" if a.status == AddressStatus.assigned else "created", body.address) + audit(db, user, "address", a, "assigned" if a.status == AddressStatus.assigned else "created", body.address, organization_id=p.organization_id) # изменение 032: organization_id commit(db, "Адрес уже есть в этом префиксе") db.refresh(a) return _addr_out(a) @@ -419,17 +436,18 @@ def allocate_next( ): """Автоназначение первого свободного адреса; только для префиксов с флагом is_pool.""" p = _lock_prefix(db, id) # занятые диапазоны (вложенные префиксы) должны читаться по согласованному дереву (изменение 029/030) + require_org(user, p.organization_id, "Префикс") # изменение 032 if not p.is_pool: raise HTTPException(422, "Префикс не является пулом для автоназначения") ip = next_free_address(ipaddress.ip_network(str(p.prefix)), _busy_ranges(db, p)) # вложенные префиксы и адрес сети/broadcast пропускаются if ip is None: raise HTTPException(409, "В префиксе нет свободных адресов") data = body.model_dump(exclude_unset=True, exclude_none=True) if body else {} - _check_device(db, p, data.get("device_id")) + _check_device(db, user, p, data.get("device_id")) a = Address(prefix_id=id, vrf_id=p.vrf_id, address=ip, **data) db.add(a) flush(db, "Адрес уже занят, повторите запрос") - audit(db, user, "address", a, "assigned", ip) + audit(db, user, "address", a, "assigned", ip, organization_id=p.organization_id) # изменение 032: organization_id commit(db, "Адрес уже занят, повторите запрос") db.refresh(a) return _addr_out(a) @@ -438,12 +456,14 @@ def allocate_next( @router.patch("/addresses/{id}", response_model=s.AddressOut) def update_address(id: int, body: s.AddressUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)): a = get_or_404(db, Address, id, "Адрес") + p = db.get(Prefix, a.prefix_id) + require_org(user, p.organization_id, "Адрес") # изменение 032 data = body.model_dump(exclude_unset=True) if data.get("dns_name"): s.AddressIn(address=s.ip_text(a.address), dns_name=data["dns_name"]) # валидация FQDN - _check_device(db, db.get(Prefix, a.prefix_id), data.get("device_id")) + _check_device(db, user, p, data.get("device_id")) changed = apply_update(a, data) - audit(db, user, "address", a, "updated", s.ip_text(a.address), {k: str(v) for k, v in changed.items()}) + audit(db, user, "address", a, "updated", s.ip_text(a.address), {k: str(v) for k, v in changed.items()}, organization_id=p.organization_id) # изменение 032: organization_id commit(db) db.refresh(a) return _addr_out(a) @@ -452,6 +472,8 @@ def update_address(id: int, body: s.AddressUpdate, db: Session = Depends(get_db) @router.delete("/addresses/{id}", status_code=204) def delete_address(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): a = get_or_404(db, Address, id, "Адрес") - audit(db, user, "address", a, "deleted", s.ip_text(a.address)) + p = db.get(Prefix, a.prefix_id) + require_org(user, p.organization_id, "Адрес") # изменение 032 + audit(db, user, "address", a, "deleted", s.ip_text(a.address), organization_id=p.organization_id) # изменение 032: organization_id db.delete(a) commit(db) diff --git a/app/api/v1/refs.py b/app/api/v1/refs.py index cb92e39..4d9861e 100644 --- a/app/api/v1/refs.py +++ b/app/api/v1/refs.py @@ -8,8 +8,8 @@ from app.db import get_db from app.models import ( Address, AddressStatus, Device, DeviceType, Isp, IspNetwork, Organization, Prefix, User, Vrf, ) -from app.security import admin_user, current_user -from app.services import MAX_OFFSET, apply_update, audit, blockers, commit, contains, count, flush, get_or_404, refuse_delete +from app.security import admin_user, current_user, superadmin_user +from app.services import MAX_OFFSET, apply_update, audit, blockers, commit, contains, count, flush, get_or_404, refuse_delete, require_org, scope_org from fastapi import HTTPException router = APIRouter(dependencies=[Depends(current_user)]) @@ -37,8 +37,9 @@ def _org_out(db: Session, o: Organization) -> s.OrgOut: @router.get("/organizations", response_model=s.Page[s.OrgOut], tags=["organizations"]) -def list_orgs(q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db)): +def list_orgs(q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), user: User = Depends(current_user)): stmt = select(Organization) + stmt = scope_org(stmt, Organization.id, user) # изменение 032 if q: stmt = stmt.where(or_(*(contains(c, q) for c in (Organization.name, Organization.short_name, Organization.inn, Organization.address)))) total = count(db, stmt) @@ -47,43 +48,46 @@ def list_orgs(q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = @router.get("/organizations/{id}", response_model=s.OrgOut, tags=["organizations"]) -def get_org(id: int, db: Session = Depends(get_db)): +def get_org(id: int, db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 + require_org(user, id, "Организация") return _org_out(db, get_or_404(db, Organization, id, "Организация")) @router.post("/organizations", response_model=s.OrgOut, status_code=201, tags=["organizations"]) -def create_org(body: s.OrgIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def create_org(body: s.OrgIn, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin o = Organization(**body.model_dump()) db.add(o) flush(db, "Организация с таким названием или ИНН уже существует") db.add(Vrf(organization_id=o.id, name="default")) - audit(db, user, "organization", o, "created", o.name) + audit(db, user, "organization", o, "created", o.name, organization_id=o.id) # изменение 032: organization_id commit(db, "Организация с таким названием или ИНН уже существует") return _org_out(db, o) @router.patch("/organizations/{id}", response_model=s.OrgOut, tags=["organizations"]) -def update_org(id: int, body: s.OrgIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_org(id: int, body: s.OrgIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032: admin_user + require_org(user, id, "Организация") # изменение 032 o = get_or_404(db, Organization, id, "Организация") changed = apply_update(o, body.model_dump()) - audit(db, user, "organization", o, "updated", o.name, changed) + audit(db, user, "organization", o, "updated", o.name, changed, organization_id=id) # изменение 032: organization_id commit(db, "Организация с таким названием или ИНН уже существует") return _org_out(db, o) @router.delete("/organizations/{id}", status_code=204, tags=["organizations"]) -def delete_org(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def delete_org(id: int, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin o = get_or_404(db, Organization, id, "Организация") found = { "prefixes": blockers(db, select(func.concat(cast(Prefix.prefix, String), " (", Vrf.name, ")")).select_from(Prefix).join(Vrf, Vrf.id == Prefix.vrf_id) .where(Prefix.organization_id == id).order_by(Prefix.id)), "devices": blockers(db, select(Device.name).where(Device.organization_id == id).order_by(Device.id)), "isps": blockers(db, select(Isp.name).where(Isp.organization_id == id).order_by(Isp.id)), + "users": blockers(db, select(User.username).where(User.organization_id == id).order_by(User.id)), # изменение 032: пользователи } if any(found.values()): - refuse_delete(db, user, "organization", o, o.name, "Нельзя удалить: у организации есть префиксы, устройства или операторы", found) + refuse_delete(db, user, "organization", o, o.name, "Нельзя удалить: у организации есть привязанные объекты", found, organization_id=id) # изменение 033, находка №4 db.execute(delete(Vrf).where(Vrf.organization_id == id)) # немедленно: между Vrf и Organization нет relationship(), порядок DELETE в UoW не гарантирован - audit(db, user, "organization", o, "deleted", o.name) + audit(db, user, "organization", o, "deleted", o.name, organization_id=id) # изменение 032: organization_id db.delete(o) commit(db) @@ -105,49 +109,59 @@ def _vrf_out(db: Session, v: Vrf) -> s.VrfOut: @router.get("/vrfs", response_model=s.Page[s.VrfOut], tags=["vrf"]) -def list_vrfs(organization_id: int | None = None, db: Session = Depends(get_db)): +def list_vrfs(organization_id: int | None = None, db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 stmt = select(Vrf) if organization_id: + require_org(user, organization_id, "Организация") # изменение 032 stmt = stmt.where(Vrf.organization_id == organization_id) + else: + stmt = scope_org(stmt, Vrf.organization_id, user) # изменение 032 rows = db.scalars(stmt.order_by(Vrf.id)).all() return s.Page(items=_vrf_outs(db, list(rows)), total=len(rows)) @router.post("/vrfs", response_model=s.VrfOut, status_code=201, tags=["vrf"]) -def create_vrf(body: s.VrfIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def create_vrf(body: s.VrfIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032: admin_user вместо admin + require_org(user, body.organization_id, "Организация") # изменение 032 get_or_404(db, Organization, body.organization_id, "Организация") v = Vrf(**body.model_dump()) db.add(v) flush(db, "VRF с таким названием уже есть в организации") - audit(db, user, "vrf", v, "created", v.name) + audit(db, user, "vrf", v, "created", v.name, organization_id=body.organization_id) # изменение 032: organization_id commit(db, "VRF с таким названием уже есть в организации") return _vrf_out(db, v) @router.patch("/vrfs/{id}", response_model=s.VrfOut, tags=["vrf"]) -def update_vrf(id: int, body: s.VrfUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_vrf(id: int, body: s.VrfUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 v = get_or_404(db, Vrf, id, "VRF") + require_org(user, v.organization_id, "VRF") # изменение 032 changed = apply_update(v, body.model_dump(exclude_unset=True, exclude_none=True)) - audit(db, user, "vrf", v, "updated", v.name, changed) + audit(db, user, "vrf", v, "updated", v.name, changed, organization_id=v.organization_id) # изменение 032: organization_id commit(db, "VRF с таким названием уже есть в организации") return _vrf_out(db, v) @router.delete("/vrfs/{id}", status_code=204, tags=["vrf"]) -def delete_vrf(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def delete_vrf(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 v = get_or_404(db, Vrf, id, "VRF") + require_org(user, v.organization_id, "VRF") # изменение 032 used = blockers(db, select(cast(Prefix.prefix, String)).where(Prefix.vrf_id == id).order_by(Prefix.id)) if used: - refuse_delete(db, user, "vrf", v, v.name, "Нельзя удалить: VRF используется префиксами", {"prefixes": used}) - audit(db, user, "vrf", v, "deleted", v.name) + refuse_delete(db, user, "vrf", v, v.name, "Нельзя удалить: VRF используется префиксами", {"prefixes": used}, organization_id=v.organization_id) # изменение 033, находка №4 + audit(db, user, "vrf", v, "deleted", v.name, organization_id=v.organization_id) # изменение 032: organization_id db.delete(v) commit(db) # ---------------------------------------------------------------- device types -def _type_outs(db: Session, rows: list[DeviceType]) -> list[s.DeviceTypeOut]: +def _type_outs(db: Session, rows: list[DeviceType], user: User | None = None) -> list[s.DeviceTypeOut]: + """изменение 033, находка №12: счётчик — в границах организации пользователя, а не по всем организациям.""" ids = [t.id for t in rows] - counts = dict(db.execute(select(Device.device_type_id, func.count()).where(Device.device_type_id.in_(ids)).group_by(Device.device_type_id)).all()) if ids else {} + stmt = select(Device.device_type_id, func.count()).where(Device.device_type_id.in_(ids)) + if user is not None: + stmt = scope_org(stmt, Device.organization_id, user) + counts = dict(db.execute(stmt.group_by(Device.device_type_id)).all()) if ids else {} out = [] for t in rows: item = s.DeviceTypeOut.model_validate(t) @@ -156,18 +170,18 @@ def _type_outs(db: Session, rows: list[DeviceType]) -> list[s.DeviceTypeOut]: return out -def _type_out(db: Session, t: DeviceType) -> s.DeviceTypeOut: - return _type_outs(db, [t])[0] +def _type_out(db: Session, t: DeviceType, user: User | None = None) -> s.DeviceTypeOut: + return _type_outs(db, [t], user)[0] @router.get("/device-types", response_model=s.Page[s.DeviceTypeOut], tags=["devices"]) -def list_types(db: Session = Depends(get_db)): +def list_types(db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 033, находка №12 rows = db.scalars(select(DeviceType).order_by(DeviceType.id)).all() - return s.Page(items=_type_outs(db, list(rows)), total=len(rows)) + return s.Page(items=_type_outs(db, list(rows), user), total=len(rows)) @router.post("/device-types", response_model=s.DeviceTypeOut, status_code=201, tags=["devices"]) -def create_type(body: s.DeviceTypeIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def create_type(body: s.DeviceTypeIn, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin t = DeviceType(name=body.name) db.add(t) flush(db, "Тип с таким названием уже существует") @@ -177,7 +191,7 @@ def create_type(body: s.DeviceTypeIn, db: Session = Depends(get_db), user: User @router.patch("/device-types/{id}", response_model=s.DeviceTypeOut, tags=["devices"]) -def update_type(id: int, body: s.DeviceTypeIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_type(id: int, body: s.DeviceTypeIn, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin t = get_or_404(db, DeviceType, id, "Тип") changed = apply_update(t, {"name": body.name}) audit(db, user, "device_type", t, "updated", t.name, changed) @@ -186,7 +200,7 @@ def update_type(id: int, body: s.DeviceTypeIn, db: Session = Depends(get_db), us @router.delete("/device-types/{id}", status_code=204, tags=["devices"]) -def delete_type(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def delete_type(id: int, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032: только superadmin t = get_or_404(db, DeviceType, id, "Тип") if t.is_default: refuse_delete(db, user, "device_type", t, t.name, "Нельзя удалить тип по умолчанию") @@ -228,11 +242,14 @@ def _device_out(db: Session, d: Device) -> s.DeviceOut: @router.get("/devices", response_model=s.Page[s.DeviceOut], tags=["devices"]) def list_devices( organization_id: int | None = None, device_type_id: int | None = None, q: str = "", - limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), + limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), db: Session = Depends(get_db), user: User = Depends(current_user), # изменение 032 ): stmt = select(Device) if organization_id: + require_org(user, organization_id, "Организация") # изменение 032 stmt = stmt.where(Device.organization_id == organization_id) + else: + stmt = scope_org(stmt, Device.organization_id, user) # изменение 032 if device_type_id: stmt = stmt.where(Device.device_type_id == device_type_id) if q: @@ -244,38 +261,43 @@ def list_devices( @router.post("/devices", response_model=s.DeviceOut, status_code=201, tags=["devices"]) -def create_device(body: s.DeviceIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def create_device(body: s.DeviceIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032: admin_user вместо admin + require_org(user, body.organization_id, "Организация") # изменение 032 get_or_404(db, Organization, body.organization_id, "Организация") get_or_404(db, DeviceType, body.device_type_id, "Тип") d = Device(**body.model_dump()) db.add(d) flush(db, "Устройство с таким именем уже есть в организации") - audit(db, user, "device", d, "created", d.name) + audit(db, user, "device", d, "created", d.name, organization_id=body.organization_id) # изменение 032: organization_id commit(db, "Устройство с таким именем уже есть в организации") return _device_out(db, d) @router.get("/devices/{id}", response_model=s.DeviceOut, tags=["devices"]) -def get_device(id: int, db: Session = Depends(get_db)): - return _device_out(db, get_or_404(db, Device, id, "Устройство")) +def get_device(id: int, db: Session = Depends(get_db), user: User = Depends(current_user)): # изменение 032 + d = get_or_404(db, Device, id, "Устройство") + require_org(user, d.organization_id, "Устройство") # изменение 032 + return _device_out(db, d) @router.patch("/devices/{id}", response_model=s.DeviceOut, tags=["devices"]) -def update_device(id: int, body: s.DeviceUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_device(id: int, body: s.DeviceUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 d = get_or_404(db, Device, id, "Устройство") + require_org(user, d.organization_id, "Устройство") # изменение 032 data = body.model_dump(exclude_unset=True, exclude_none=True) if "device_type_id" in data: get_or_404(db, DeviceType, data["device_type_id"], "Тип") changed = apply_update(d, data) - audit(db, user, "device", d, "updated", d.name, changed) + audit(db, user, "device", d, "updated", d.name, changed, organization_id=d.organization_id) # изменение 032: organization_id commit(db, "Устройство с таким именем уже есть в организации") return _device_out(db, d) @router.delete("/devices/{id}", status_code=204, tags=["devices"]) -def delete_device(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def delete_device(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 d = get_or_404(db, Device, id, "Устройство") - audit(db, user, "device", d, "deleted", d.name) + require_org(user, d.organization_id, "Устройство") # изменение 032 + audit(db, user, "device", d, "deleted", d.name, organization_id=d.organization_id) # изменение 032: organization_id db.delete(d) commit(db) @@ -298,11 +320,14 @@ def _isp_out(db: Session, i: Isp) -> s.IspOut: @router.get("/isps", response_model=s.Page[s.IspOut], tags=["isps"]) def list_isps( organization_id: int | None = None, q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), - db: Session = Depends(get_db), + db: Session = Depends(get_db), user: User = Depends(current_user), # изменение 032 ): stmt = select(Isp) if organization_id: + require_org(user, organization_id, "Организация") # изменение 032 stmt = stmt.where(Isp.organization_id == organization_id) + else: + stmt = scope_org(stmt, Isp.organization_id, user) # изменение 032 if q: nets = select(IspNetwork.isp_id).where(contains(cast(IspNetwork.cidr, String), q)) orgs = select(Organization.id).where(contains(Organization.name, q)) @@ -313,34 +338,38 @@ def list_isps( @router.post("/isps", response_model=s.IspOut, status_code=201, tags=["isps"]) -def create_isp(body: s.IspIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def create_isp(body: s.IspIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032: admin_user вместо admin + require_org(user, body.organization_id, "Организация") # изменение 032 get_or_404(db, Organization, body.organization_id, "Организация") data = body.model_dump() nets = data.pop("networks") i = Isp(**data, networks=[IspNetwork(cidr=n) for n in nets]) db.add(i) db.flush() - audit(db, user, "isp", i, "created", i.name) + audit(db, user, "isp", i, "created", i.name, organization_id=body.organization_id) # изменение 032: organization_id commit(db) return _isp_out(db, i) @router.put("/isps/{id}", response_model=s.IspOut, tags=["isps"]) -def update_isp(id: int, body: s.IspIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def update_isp(id: int, body: s.IspIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 i = get_or_404(db, Isp, id, "Оператор") + require_org(user, i.organization_id, "Оператор") # изменение 032 + require_org(user, body.organization_id, "Организация") # изменение 033, находка №11: до get_or_404 чужой организации get_or_404(db, Organization, body.organization_id, "Организация") data = body.model_dump() nets = data.pop("networks") changed = apply_update(i, data) i.networks = [IspNetwork(cidr=n) for n in nets] - audit(db, user, "isp", i, "updated", i.name, changed) + audit(db, user, "isp", i, "updated", i.name, changed, organization_id=i.organization_id) # изменение 032: organization_id commit(db) return _isp_out(db, i) @router.delete("/isps/{id}", status_code=204, tags=["isps"]) -def delete_isp(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): +def delete_isp(id: int, db: Session = Depends(get_db), user: User = Depends(admin_user)): # изменение 032 i = get_or_404(db, Isp, id, "Оператор") - audit(db, user, "isp", i, "deleted", i.name) + require_org(user, i.organization_id, "Оператор") # изменение 032 + audit(db, user, "isp", i, "deleted", i.name, organization_id=i.organization_id) # изменение 032: organization_id db.delete(i) commit(db) diff --git a/app/api/v1/users.py b/app/api/v1/users.py index 60c9991..d22535a 100644 --- a/app/api/v1/users.py +++ b/app/api/v1/users.py @@ -1,8 +1,9 @@ -"""Пользователи: учётные записи UI (роли admin/viewer), смена своего пароля. +"""Пользователи: учётные записи UI (роли superadmin/admin/viewer), смена своего пароля. Правила: логин после создания не меняется (он же `sub` в JWT), нельзя отключить/понизить/удалить -свою учётную запись и последнего активного администратора; зарезервированные логины `system` +свою учётную запись и последнего активного суперадминистратора; зарезервированные логины `system` и `anonymous` запрещены — журнал различает по ним служебные события (`actor_of` в app/services.py). +Управление пользователями — только superadmin (изменение 032). """ from datetime import datetime, timezone @@ -12,29 +13,29 @@ from sqlalchemy.orm import Session from app import schemas as s from app.db import get_db -from app.models import Role, User -from app.security import admin_user, create_token, current_user, hash_password, verify_password +from app.models import Organization, Role, User +from app.security import admin_user, create_token, current_user, hash_password, superadmin_user, verify_password from app.services import MAX_OFFSET, apply_update, audit, commit, contains, count, flush, get_or_404, refuse_delete router = APIRouter(dependencies=[Depends(current_user)], tags=["users"]) -USERS_ADMIN_LOCK = 703002 # advisory lock: изменения прав администраторов идут по одному (иначе двое отключат друг друга одновременно) +USERS_LOCK = 703002 # advisory lock: изменения прав суперадминистраторов идут по одному (иначе двое отключат друг друга одновременно) -def _lock_admins(db: Session) -> None: - db.execute(select(func.pg_advisory_xact_lock(USERS_ADMIN_LOCK))) +def _lock_users(db: Session) -> None: + db.execute(select(func.pg_advisory_xact_lock(USERS_LOCK))) -def _other_active_admins(db: Session, user_id: int) -> int: - """Активные администраторы, кроме указанного: 0 — система осталась бы без прав записи.""" - return count(db, select(User.id).where(User.role == Role.admin, User.is_active, User.id != user_id)) +def _other_active_superadmins(db: Session, user_id: int) -> int: + """Активные суперадминистраторы, кроме указанного: 0 — система осталась бы без прав записи (изменение 032).""" + return count(db, select(User.id).where(User.role == Role.superadmin, User.is_active, User.id != user_id)) @router.get("/users", response_model=s.Page[s.UserOut]) def list_users( q: str = "", limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=MAX_OFFSET), - db: Session = Depends(get_db), admin: User = Depends(admin_user), + db: Session = Depends(get_db), user: User = Depends(superadmin_user), # изменение 032 ): stmt = select(User) if q.strip(): @@ -45,54 +46,68 @@ def list_users( @router.post("/users", response_model=s.UserOut, status_code=201) -def create_user(body: s.UserIn, db: Session = Depends(get_db), admin: User = Depends(admin_user)): +def create_user(body: s.UserIn, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032 # логин уникален без учёта регистра: в токене и в журнале он должен опознаваться однозначно if db.scalar(select(User.id).where(func.lower(User.username) == body.username.lower())): raise HTTPException(409, "Пользователь с таким логином уже существует") - u = User(username=body.username, password_hash=hash_password(body.password), role=body.role, is_active=body.is_active) + if body.organization_id is not None: + get_or_404(db, Organization, body.organization_id, "Организация") # изменение 033, находка №10 + u = User(username=body.username, password_hash=hash_password(body.password), role=body.role, organization_id=body.organization_id, is_active=body.is_active) # изменение 032 db.add(u) - flush(db, "Пользователь с таким логином уже существует") - audit(db, admin, "user", u, "created", u.username) - commit(db, "Пользователь с таким логином уже существует") + flush(db) # изменение 033, находка №10: логин уже проверен выше, гонку покрывает CONFLICT_MSG + audit(db, user, "user", u, "created", u.username) + commit(db) return u @router.patch("/users/{id}", response_model=s.UserOut) -def update_user(id: int, body: s.UserUpdate, db: Session = Depends(get_db), admin: User = Depends(admin_user)): +def update_user(id: int, body: s.UserUpdate, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032 data = body.model_dump(exclude_unset=True, exclude_none=True) if "role" in data or "is_active" in data: - _lock_admins(db) # до чтения пользователя и подсчёта администраторов + _lock_users(db) # до чтения пользователя и подсчёта суперадминистраторов (изменение 032) u = get_or_404(db, User, id, "Пользователь") - if u.id == admin.id and "password" in data: + if u.id == user.id and "password" in data: raise HTTPException(422, "Свой пароль меняется через /users/me/password (с подтверждением текущего)") pwd = data.pop("password", None) role, is_active = data.get("role", u.role), data.get("is_active", u.is_active) - loses_admin = u.role == Role.admin and u.is_active and (role != Role.admin or not is_active) - if u.id == admin.id and (role != Role.admin or not is_active): + loses_superadmin = u.role == Role.superadmin and u.is_active and (role != Role.superadmin or not is_active) # изменение 032 + if u.id == user.id and (role != Role.superadmin or not is_active): # изменение 032 raise HTTPException(409, "Нельзя отключить или понизить свою учётную запись") - if loses_admin and _other_active_admins(db, u.id) == 0: - raise HTTPException(409, "Нельзя отключить или понизить единственного активного администратора") + if loses_superadmin and _other_active_superadmins(db, u.id) == 0: # изменение 032 + raise HTTPException(409, "Нельзя отключить или понизить единственного активного суперадминистратора") + # реконсиляция «роль — организация» по итоговому состоянию, до apply_update (изменение 033, находка №3) + org_id_passed = "organization_id" in data # exclude_none=True выше уже отбросил явный null + if role == Role.superadmin: + if org_id_passed: + raise HTTPException(422, "Суперадминистратор не привязан к организации") + data["organization_id"] = None # повышение снимает организацию само + else: + org_f = data.get("organization_id", u.organization_id) + if org_f is None and (is_active or "role" in data or org_id_passed): + raise HTTPException(422, "Администратор и просмотрщик должны быть привязаны к организации") + if org_id_passed: + get_or_404(db, Organization, data["organization_id"], "Организация") changed = apply_update(u, data) if pwd: u.password_hash = hash_password(pwd) u.password_changed_at = datetime.now(timezone.utc) if changed: - audit(db, admin, "user", u, "updated", u.username, changed) + audit(db, user, "user", u, "updated", u.username, changed) if pwd: - audit(db, admin, "user", u, "password_reset", u.username, message=f"Пользователь {u.username}: пароль изменён администратором") + audit(db, user, "user", u, "password_reset", u.username, message=f"Пользователь {u.username}: пароль изменён администратором") commit(db) return u @router.delete("/users/{id}", status_code=204) -def delete_user(id: int, db: Session = Depends(get_db), admin: User = Depends(admin_user)): - _lock_admins(db) +def delete_user(id: int, db: Session = Depends(get_db), user: User = Depends(superadmin_user)): # изменение 032 + _lock_users(db) u = get_or_404(db, User, id, "Пользователь") - if u.id == admin.id: - refuse_delete(db, admin, "user", u, u.username, "Нельзя удалить свою учётную запись") - if u.role == Role.admin and u.is_active and _other_active_admins(db, u.id) == 0: - refuse_delete(db, admin, "user", u, u.username, "Нельзя удалить единственного активного администратора") - audit(db, admin, "user", u, "deleted", u.username) + if u.id == user.id: + refuse_delete(db, user, "user", u, u.username, "Нельзя удалить свою учётную запись") + if u.role == Role.superadmin and u.is_active and _other_active_superadmins(db, u.id) == 0: # изменение 032 + refuse_delete(db, user, "user", u, u.username, "Нельзя удалить единственного активного суперадминистратора") + audit(db, user, "user", u, "deleted", u.username) db.delete(u) commit(db) diff --git a/app/main.py b/app/main.py index 5fab2b4..c4a1e7c 100644 --- a/app/main.py +++ b/app/main.py @@ -28,7 +28,7 @@ def seed(): with SessionLocal() as db: if not db.scalar(select(User.id).limit(1)): if settings.admin_password: - db.add(User(username=settings.admin_username, password_hash=hash_password(settings.admin_password), role=Role.admin)) + db.add(User(username=settings.admin_username, password_hash=hash_password(settings.admin_password), role=Role.superadmin, organization_id=None)) # изменение 032: role=superadmin, organization_id=None else: log.warning("В БД нет пользователей и ADMIN_PASSWORD не задан: администратор не создан, войти в UI нельзя") if not db.scalar(select(DeviceType.id).limit(1)): diff --git a/app/models.py b/app/models.py index f94fdf7..52d7eb1 100644 --- a/app/models.py +++ b/app/models.py @@ -3,7 +3,7 @@ import uuid from datetime import datetime from sqlalchemy import ( - BigInteger, Boolean, DateTime, Enum, ForeignKey, ForeignKeyConstraint, Index, Integer, String, Text, + BigInteger, Boolean, CheckConstraint, DateTime, Enum, ForeignKey, ForeignKeyConstraint, Index, Integer, String, Text, UniqueConstraint, func, text, ) from sqlalchemy.dialects.postgresql import CIDR, INET, JSONB, UUID @@ -27,6 +27,7 @@ class AddressStatus(str, enum.Enum): class Role(str, enum.Enum): + superadmin = "superadmin" admin = "admin" viewer = "viewer" @@ -137,10 +138,15 @@ class Address(Base): class User(Base): __tablename__ = "users" + __table_args__ = ( + # изменение 032: superadmin без организации, admin/viewer с организацией или неактивны + CheckConstraint("is_active = false OR (role = 'superadmin') = (organization_id IS NULL)", name="ck_users_role_org_scope"), # изменение 033, находка №13 + ) id: Mapped[int] = mapped_column(primary_key=True) username: Mapped[str] = mapped_column(String(100), unique=True) password_hash: Mapped[str] = mapped_column(String(255)) role: Mapped[Role] = mapped_column(Enum(Role, name="user_role"), default=Role.viewer) + organization_id: Mapped[int | None] = mapped_column(ForeignKey("organizations.id"), index=True) # изменение 032: NK для admin/viewer, NULL для superadmin is_active: Mapped[bool] = mapped_column(Boolean, default=True) password_changed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) # токены с iat раньше — недействительны @@ -162,6 +168,7 @@ class AuditLog(Base): message: Mapped[str] = mapped_column(Text, default="", server_default="") client_ip: Mapped[str | None] = mapped_column(INET, index=True) # IP клиента запроса; NULL для системных событий meta: Mapped[dict | None] = mapped_column(JSONB) # user_agent, method, path, request_id + organization_id: Mapped[int | None] = mapped_column(ForeignKey("organizations.id", ondelete="SET NULL"), index=True) # изменение 032: для событий по сущностям организации, NULL для системных; ondelete — изменение 033, находка №1 __table_args__ = (Index("ix_audit_log_entity_type_action", "entity_type", "action"),) diff --git a/app/schemas.py b/app/schemas.py index dd96306..d3df566 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -3,7 +3,7 @@ import re from datetime import datetime from typing import Annotated, Generic, TypeVar -from pydantic import AfterValidator, BaseModel, ConfigDict, EmailStr, Field, field_validator +from pydantic import AfterValidator, BaseModel, ConfigDict, EmailStr, Field, field_validator, model_validator from app.models import AddressStatus, PrefixStatus, Role @@ -86,6 +86,7 @@ class UserOut(ORM): id: int username: str role: Role + organization_id: int | None = None # изменение 032 is_active: bool = True @@ -93,13 +94,26 @@ class UserIn(BaseModel): username: Login password: str = Field(min_length=8, max_length=128) role: Role = Role.viewer # наименьшие права по умолчанию (изменение 016) + organization_id: int | None = None # изменение 032: обязателен для admin/viewer, запрещён для superadmin is_active: bool = True + @model_validator(mode="after") + def _org_by_role(self) -> "UserIn": + # model_validator (изменение 033, находка №2): срабатывает и когда organization_id не передан явно + if self.role == Role.superadmin and self.organization_id is not None: + raise ValueError("Суперадминистратор не привязан к организации") + if self.role != Role.superadmin and self.organization_id is None: + raise ValueError("Администратор и просмотрщик должны быть привязаны к организации") + return self + class UserUpdate(BaseModel): role: Role | None = None + organization_id: int | None = None # изменение 032 is_active: bool | None = None password: str | None = Field(None, min_length=8, max_length=128) + # без валидатора роль/организация (изменение 033, находка №2): без текущего состояния записи + # инвариант не проверить — реконсиляция по итоговому состоянию в update_user (находка №3) class PasswordChange(BaseModel): diff --git a/app/security.py b/app/security.py index e357192..5bb96e8 100644 --- a/app/security.py +++ b/app/security.py @@ -63,6 +63,14 @@ def current_user( def admin_user(user: User = Depends(current_user)) -> User: - if user.role != Role.admin: + # изменение 032: admin_user проверяет роль != viewer (пропускает admin И superadmin) + if user.role == Role.viewer: + raise HTTPException(403, "Недостаточно прав") + return user + + +def superadmin_user(user: User = Depends(current_user)) -> User: + # изменение 032: строгая проверка на superadmin + if user.role != Role.superadmin: raise HTTPException(403, "Недостаточно прав") return user diff --git a/app/services.py b/app/services.py index 12a581e..c1bdcbe 100644 --- a/app/services.py +++ b/app/services.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from app.request_context import request_meta -from app.models import Address, AuditLog, Base +from app.models import Address, AuditLog, Base, Role MAX_CAPACITY = 2**53 - 1 MAX_OFFSET = 10_000_000 # верхняя граница offset во всех списках @@ -49,18 +49,20 @@ def actor_of(username: str) -> str: return username if username in ("system", "anonymous") else f"ui:{username}" -def audit(db: Session, user, entity_type: str, entity, action: str, label: str, diff: dict | None = None, message: str | None = None): +def audit(db: Session, user, entity_type: str, entity, action: str, label: str, diff: dict | None = None, message: str | None = None, organization_id: int | None = None): + # изменение 032: organization_id передаётся явно; None для системных событий и событий пользователя ctx = None if user is SYSTEM else request_meta.get() # системные события (ротация) — без IP, даже если запущены из запроса db.add(AuditLog( username=user.username, entity_type=entity_type, entity_id=getattr(entity, "id", None), entity_label=label, action=action, diff=diff, message=message or make_message(entity_type, action, label, diff), client_ip=ctx["client_ip"] if ctx else None, meta=ctx["meta"] if ctx else None, + organization_id=organization_id, )) BLOCKERS_LIMIT = 20 -_GROUPS = {"prefixes": "префиксы", "devices": "устройства", "isps": "операторы", "addresses": "адреса"} +_GROUPS = {"prefixes": "префиксы", "devices": "устройства", "isps": "операторы", "addresses": "адреса", "users": "пользователи"} # изменение 032 def blockers(db: Session, labels) -> dict | None: @@ -69,13 +71,14 @@ def blockers(db: Session, labels) -> dict | None: return {"total": total, "items": list(db.scalars(labels.limit(BLOCKERS_LIMIT)))} if total else None -def refuse_delete(db: Session, user, entity_type: str, entity, label: str, reason: str, blocked_by: dict | None = None): - """Отказ в удалении (409): фиксируем предупреждение в журнале (`.delete_blocked`) и отвечаем прежним текстом `reason`.""" +def refuse_delete(db: Session, user, entity_type: str, entity, label: str, reason: str, blocked_by: dict | None = None, organization_id: int | None = None): + """Отказ в удалении (409): фиксируем предупреждение в журнале (`.delete_blocked`) и отвечаем прежним текстом `reason`. + `organization_id` (изменение 033, находка №4) — чтобы запись видел админ организации, а не только суперадминистратор.""" blocked_by = {k: v for k, v in (blocked_by or {}).items() if v} diff = {"reason": reason, **({"blocked_by": blocked_by} if blocked_by else {})} noun = _NOUNS.get(entity_type, (entity_type, "m"))[0] tail = ("связанные объекты (" + ", ".join(f"{_GROUPS[k]}: {v['total']}" for k, v in blocked_by.items()) + ")") if blocked_by else reason[:1].lower() + reason[1:] - audit(db, user, entity_type, entity, "delete_blocked", label, diff, message=f"{noun} {label}: удаление отклонено — {tail}") + audit(db, user, entity_type, entity, "delete_blocked", label, diff, message=f"{noun} {label}: удаление отклонено — {tail}", organization_id=organization_id) commit(db) # к этому моменту в транзакции только запись журнала raise HTTPException(409, reason) @@ -100,10 +103,21 @@ def flush(db: Session, conflict_msg: str = CONFLICT_MSG): raise HTTPException(409, conflict_msg) +# изменение 033 (попутно): согласование рода в тексте 404 — по первому слову `what` +_FEMININE_FIRST_WORDS = {"Организация", "Запись"} +_NEUTER_FIRST_WORDS = {"Устройство"} + + +def not_found(what: str) -> str: + first = what.split(" ", 1)[0] + suffix = "а" if first in _FEMININE_FIRST_WORDS else "о" if first in _NEUTER_FIRST_WORDS else "" + return f"{what} не найден{suffix}" + + def get_or_404(db: Session, model: type[Base], id_: int, what: str = "Объект"): obj = db.get(model, id_) if obj is None: - raise HTTPException(404, f"{what} не найден") + raise HTTPException(404, not_found(what)) return obj @@ -208,3 +222,18 @@ def next_free_address(net, occupied: list[tuple[int, int]]) -> str | None: break cand = end + 1 return str(type(net.network_address)(cand)) if cand <= last else None + + +def require_org(user, organization_id: int | None, what: str): + """изменение 032: проверка доступа к организации. superadmin пропускает, иначе 404 при несовпадении.""" + if user.role == Role.superadmin: + return + if user.organization_id != organization_id: + raise HTTPException(404, not_found(what)) + + +def scope_org(stmt, column, user): + """изменение 032: фильтр списков по организации. superadmin без фильтра, иначе фильтр по user.organization_id.""" + if user.role == Role.superadmin: + return stmt + return stmt.where(column == user.organization_id) diff --git a/docs/changes/031-expose-hardening/PLAN.md b/docs/changes/031-expose-hardening/PLAN.md new file mode 100644 index 0000000..9c1157a --- /dev/null +++ b/docs/changes/031-expose-hardening/PLAN.md @@ -0,0 +1,43 @@ +# Сужение раскрытия API и HTTP-экспозиции (изменение 031) + +Источник: `docs/reviews/2026-09-27-pentest.md`, находки № 1 (открытый Swagger) и № 2 (HTTP на `0.0.0.0` без TLS). +Обе — про периметр: приложение по чёрному ящику устойчиво, но раскрывает лишнее. Ниже — только код и конфигурация. + +## Часть 1 — Swagger и OpenAPI по умолчанию выключены (находка № 1) +**Проблема:** `/docs`, `/redoc`, `/openapi.json` открыты анонимно (проверено на стенде: карта из 28 эндпоинтов без авторизации). `SecurityHeadersMiddleware` уже знает о `DOCS_PATHS`, но сами маршруты создаёт FastAPI по умолчанию. + +**Решение:** +1. `app/config.py`: `docs_enabled: bool = False` в `Settings` (управляется `DOCS_ENABLED` в `.env`). +2. `app/main.py`: при создании `FastAPI(...)` — `docs_url`, `redoc_url`, `openapi_url` = `None`, если `not settings.docs_enabled`. Тогда маршрутов просто нет (404), а не «спрятаны». `DOCS_PATHS` в middleware оставить: при `docs_enabled=true` заголовки на них по-прежнему не навешиваются. +3. `.env.example`: строка `# DOCS_ENABLED=true — открыть Swagger/OpenAPI (по умолчанию выключено)`. +4. `README.md`: в «Быстром старте» отметить, что Swagger включается `DOCS_ENABLED=true`; для разработки его можно держать включённым. + +**Проверка:** при пустом `.env` `/docs` и `/openapi.json` → 404; при `DOCS_ENABLED=true` → 200. UI и API не затронуты. + +## Часть 2 — предупреждение о небезопасной экспозиции + готовый TLS-профиль (находка № 2) +**Проблема:** приложение слушает `0.0.0.0:8088` по HTTP, токены и пароли идут по LAN открытым текстом. Рекомендация из изменения 022 (reverse-proxy + `APP_BIND=127.0.0.1` + `TRUSTED_PROXIES`) документирована, но на стенде не задействована. Смена `APP_BIND` по умолчанию отключит текущий доступ к стенду `192.168.5.9:8088` — **делается только по решению пользователя** (см. ниже). + +**Решение (без смены умолчаний):** +1. `app/main.py` рядом с `validate_secrets()`: функция-предупреждение при старте (`logging.warning`, не отказ), если одновременно `APP_BIND` не `127.0.0.1` и `TRUSTED_PROXIES` пуст — «приложение принимает прямые HTTP-соединения из сети без TLS; поставьте reverse-proxy и задайте APP_BIND=127.0.0.1 (README «Публикация»)». `APP_BIND` читать через `Settings` (добавить поле `app_bind: str = "0.0.0.0"`, только для этой проверки; фактическую привязку по-прежнему делает compose). +2. Готовый reverse-proxy как отдельный опциональный compose-профиль `tls` (`docker-compose.yml`): сервис `caddy` (`caddy:2-alpine`) с `Caddyfile`, проксирующий на `app:8000`, том для сертификатов. Профиль не поднимается по умолчанию (`profiles: ["tls"]`), поэтому текущий стенд не меняется. +3. `Caddyfile` в корне (шаблон): домен из переменной, `reverse_proxy app:8000`, автоматический TLS. Комментарий, что при внутреннем домене нужен `tls internal` или свой сертификат. +4. `.env.example`: `# APP_BIND=127.0.0.1` и `# TRUSTED_PROXIES=<сеть Docker прокси>` — с пояснением связки. +5. `README.md`, «Публикация»: заменить текстовый пример на запуск профиля `docker compose --profile tls up -d` и настройку `Caddyfile`. + +**Требует решения пользователя (в план внесено, но не выполняется без ответа):** +- Менять ли `APP_BIND` по умолчанию на `127.0.0.1`. Плюс — приложение перестаёт напрямую торчать в LAN; минус — текущий доступ к стенду по `192.168.5.9:8088` пропадёт до подъёма прокси. По умолчанию план оставляет `0.0.0.0` и только предупреждает в логе. + +## Файлы +`app/config.py`, `app/main.py`, `docker-compose.yml`, `Caddyfile` (новый), `.env.example`, `README.md`, `docs/changes/031-expose-hardening/SUMMARY.md`. + +## Вне объёма +- Аутентификация на Swagger вместо выключения (сложнее: `/openapi.json` нужен UI-докам; при `docs_enabled=false` проблема снята полностью). +- Реальные сертификаты и DNS — эксплуатационная настройка, не код. +- Находка № 3 пентеста (дефолтный логин `admin`) — организационная, отдельной доработкой не оформляется. + +## Проверка (выполняет ревьюер) +1. `venv/bin/python -c 'import app.main'`; пересборка стенда, `app` healthy. +2. `/docs`, `/openapi.json` → 404 при выключенном флаге, 200 при `DOCS_ENABLED=true`. +3. В логе старта — предупреждение о прямой HTTP-экспозиции при текущих `APP_BIND`/`TRUSTED_PROXIES`. +4. `docker compose config --profile tls` валиден; профиль `tls` не поднимается без флага. +5. `pytest -q` — регрессия зелёная (эндпоинты не менялись). diff --git a/docs/changes/032-role-model-org-scope/PLAN.md b/docs/changes/032-role-model-org-scope/PLAN.md new file mode 100644 index 0000000..37cd689 --- /dev/null +++ b/docs/changes/032-role-model-org-scope/PLAN.md @@ -0,0 +1,104 @@ +# Ролевая модель с привязкой к организации и суперадминистратором (изменение 032) + +## Context +Сейчас роль пользователя (`admin`/`viewer`) действует на все организации сразу — любой администратор видит и меняет данные любой организации, +включая журнал аудита. Нужна привязка учётной записи к одной организации: `admin`/`viewer` работают только в её пределах, включая чтение +(реестры, «Обзор», журнал). Добавляется роль `superadmin` — без привязки к организации, видит и меняет всё, и единственный, кто создаёт/меняет +пользователей (включая назначение админов организациям). Уровней прав внутри организации по-прежнему два: чтение или запись. + +## Решения (согласованы с пользователем) +1. **Управление пользователями — только `superadmin`.** Администратор организации не создаёт и не редактирует других пользователей (даже `viewer` своей организации). +2. **Типы устройств (`device_types`) остаются общим справочником.** Читают все роли из любой организации; создание/переименование/удаление — только `superadmin`. +3. **Журнал аудита изолирован по организации.** Админ/viewer организации видит в журнале только события своей организации. Системные события без + привязки к организации (вход/выход, настройки/очистка журнала, ротация, управление пользователями) видит только `superadmin`. +4. **Миграция существующих пользователей:** все нынешние пользователи с ролью `admin` становятся `superadmin` (`organization_id = NULL`). + Пользователи с ролью `viewer` остаются `viewer`, `organization_id = NULL`, `is_active = false` (учётная запись заблокирована), пока + `superadmin` не назначит им организацию явным `PATCH`. + +## Модель данных +- `Role`: добавить `superadmin`. Enum в PostgreSQL расширяется отдельной миграцией (`ALTER TYPE ... ADD VALUE` вне транзакции — `autocommit_block()`), + использовать новое значение можно только в следующей миграции. +- `users.organization_id` (nullable, `FK organizations.id`, индекс). Инвариант: `superadmin` ⇒ `organization_id IS NULL`; `admin`/`viewer` ⇒ `organization_id` задан. + На уровне БД — `CheckConstraint`, ослабленный для неактивных записей (`is_active = false OR (role = 'superadmin') = (organization_id IS NULL)`), + чтобы допустить временное состояние «viewer без организации, вход заблокирован» после миграции. На уровне API (`UserIn`/`UserUpdate`) инвариант строгий + и без исключений — заблокированное состояние возникает только из миграции, не создаётся через API. +- `audit_log.organization_id` (nullable, индекс). Заполняется у новых записей, если у сущности события есть организация (`organization`, `vrf`, `prefix`, + `address`, `device`, `isp`); у `user`/`journal`/`session`/`device_type` — `NULL` (видно только `superadmin`). Для существующих записей — точечный backfill + join'ом по `entity_id` там, где сущность ещё существует (`organization`, `vrf`, `device`, `isp`, `prefix`); что не сматчилось (сущность уже удалена) — остаётся + `NULL`, то есть уходит в «видно только superadmin». Это осознанный компромисс, не полная реконструкция истории. +- Удаление организации (`delete_org`) получает новую группу блокираторов — `users`: пока у организации есть привязанные пользователи (активные или нет), + удалить её нельзя (иначе `organization_id` осиротеет или нарушится FK). + +Миграции: `0010_user_role_superadmin.py` (значение enum), `0011_users_organization_scope.py` (колонка, ограничение, backfill ролей по решению 4), +`0012_audit_log_organization.py` (колонка, индекс, backfill). + +## Права доступа (сводка) +| Действие | Кто | +|---|---| +| Пользователи: список/создание/правка/удаление | только `superadmin` | +| Организации: создание/удаление | только `superadmin` | +| Организации: правка карточки | `superadmin` или `admin` **своей** организации | +| VRF, префиксы, адреса, устройства, операторы: чтение/запись | `admin`/`viewer` — только своя организация; `superadmin` — любая | +| Типы устройств: чтение | любая роль, любая организация | +| Типы устройств: создание/правка/удаление | только `superadmin` | +| Настройки/очистка журнала (`/journal/settings`, `/journal/clear`) | только `superadmin` (затрагивает весь журнал) | +| Журнал: чтение (`/audit*`) | `admin`/`viewer` — только события своей организации; `superadmin` — все, включая системные | +| «Обзор» | `admin`/`viewer` — сводка по своей организации; `superadmin` — по всем | + +## Реализация +**`app/models.py`** — `Role.superadmin`; `User.organization_id` + `CheckConstraint`; `AuditLog.organization_id`. + +**`app/security.py`** — `admin_user` меняет смысл: пропускает `admin` и `superadmin` (роль ≠ `viewer`), название и место в коде не меняются. +Новая зависимость `superadmin_user` (строго `Role.superadmin`) — по образцу `admin_user`. + +**`app/services.py`** — переиспользуемые хелперы рядом с `get_or_404`/`refuse_delete`: +- `require_org(user, organization_id, what)` — для операций с известным целевым id: `superadmin` пропускает, иначе при несовпадении `HTTPException(404, f"{what} не найден")` + (404, а не 403 — не подтверждать существование чужих данных, тот же стиль сообщений, что у `get_or_404`). +- `scope_org(stmt, column, user)` — для списков: у `superadmin` не трогает `stmt`, иначе `stmt.where(column == user.organization_id)` (безусловно, любой переданный + клиентом `organization_id` в query-параметрах просто пересекается с этим условием — отдельной ошибки не нужно). +- `audit()` получает необязательный `organization_id` (по умолчанию `None`) — простановка на стороне вызывающего кода, не выводится автоматически. +- `_GROUPS`/`blockers` — добавить `"users": "пользователи"` для группы блокираторов удаления организации. + +**`app/api/v1/users.py`** — все маршруты на `superadmin_user`. `UserIn`/`UserUpdate` (`app/schemas.py`) получают `organization_id: int | None` +с валидацией «обязателен для admin/viewer, запрещён для superadmin» (`model_validator`), в `UserOut` — тоже поле. Инвариант «нельзя обезвредить последнего +активного администратора» переводится на `superadmin`: защищается последний активный `superadmin`, не «последний admin организации» (в организации +администраторов назначает и переназначает `superadmin` по своему усмотрению — не защищается отдельно, это осознанно, см. «Вне объёма»). + +**`app/api/v1/refs.py`** — организации: `create_org`/`delete_org` → `superadmin_user`; `update_org` → `admin_user` + `require_org`; `list_orgs`/`get_org` → +`scope_org`/`require_org`. VRF, устройства, операторы: та же пара `admin_user` + `require_org`/`scope_org` по образцу друг друга (один раз описать паттерн, +применить в `create_vrf`/`update_vrf`/`delete_vrf`/`list_vrfs`, `create_device`/`update_device`/`list_devices`/`get_device`, аналогично `isps`). +Типы устройств: чтение без изменений (уже открыто всем), запись → `superadmin_user`. + +**`app/api/v1/prefixes.py`** — префиксы и адреса: `require_org`/`scope_org` там, где сейчас читается/проверяется `organization_id` (создание — из `body`, +остальные операции — через уже загруженный префикс/родителя). `list_prefixes`/`list_addresses`(через принадлежащий префикс) — `scope_org`. + +**`app/api/v1/journal.py`** — `list_audit`, `/audit/summary`, `/audit/facets`, `GET /audit/{uid}` — `scope_org` (для `{uid}` — `require_org` с 404 при чужой +или системной записи). `/journal/settings`, `/journal/clear` → `superadmin_user`. + +**`app/api/v1/overview.py`** — `_ipv4_roots` и подсчёт назначенных/зарезервированных адресов, `recent_changes` — фильтр по `organization_id` для не-`superadmin` +(системные события без организации в `recent_changes` не подмешиваются). + +**`app/main.py`** — `seed()`: пользователь-бутстрап из `.env` создаётся с `role=Role.superadmin`, `organization_id=None`. + +**`web/app.js`** — `ROLE_RU` + `"Суперадминистратор"`; `isAdmin()` = роль ≠ `viewer` (как и на сервере), новая `isSuperadmin()`. Пункт навигации +«Пользователи» — виден только `isSuperadmin()`. `orgSwitcher()` — интерактивный (с выпадающим списком) только для `isSuperadmin()`; для `admin`/`viewer` +показывает название их единственной организации без переключателя (`GET /organizations` для них и так вернёт один элемент). Экран «Организации»: +кнопка «Добавить организацию» и пункт «Удалить» в меню строки — только `isSuperadmin()`; редактирование доступно как обычная запись. Экран «Пользователи»: +диалог создания/правки получает выбор организации (скрывается при роли «Суперадминистратор», обязателен для «Администратор»/«Просмотр»). Диалог типов +устройств: кнопки добавления/переименования/удаления — только `isSuperadmin()`, список остаётся видимым всем. + +**`scripts/seed_demo.py`** — создание демо-пользователей проставляет `organization_id` для ролей `admin`/`viewer` (иначе скрипт сломает новая валидация схемы). + +**`README.md`** — разделы «Безопасность» (роли, права по организациям, `superadmin`) и «Модель данных» (новые поля/ограничения). + +## Вне объёма (сознательно) +- Защита «последнего администратора организации» — не вводится; единственный защищаемый инвариант — последний активный `superadmin`. +- Полная историческая реконструкция `organization_id` в старых записях журнала для уже удалённых сущностей — остаются `NULL` (только `superadmin`). +- Типы устройств не разбиваются по организациям (решение 2). +- Автотесты и правка существующих (`tests/test_users.py` и др., которые создают пользователей без `organization_id`) — на этапе тестирования отдельно, не сейчас. + +## Проверка (на этапе тестирования, не при реализации) +- Импорт приложения, синтаксис UI, миграции 0010–0012 применяются, `alembic check` без расхождений. +- Сценарии: `superadmin` создаёт организацию и в ней `admin`; этот `admin` видит/меняет только свою организацию (префиксы, устройства, журнал, «Обзор»), + запросы к чужой организации получают 404; `viewer` организации только читает; `superadmin` управляет типами устройств, `admin` организации их только видит; + удаление организации с привязанными пользователями отклоняется с перечнем; после миграции унаследованный `admin` из `.env` работает как `superadmin`. diff --git a/docs/changes/032-role-model-org-scope/SUMMARY.md b/docs/changes/032-role-model-org-scope/SUMMARY.md new file mode 100644 index 0000000..74dacb1 --- /dev/null +++ b/docs/changes/032-role-model-org-scope/SUMMARY.md @@ -0,0 +1,60 @@ +# Итог: ролевая модель с привязкой к организации и суперадминистратором (изменение 032) + +План: `docs/changes/032-role-model-org-scope/PLAN.md`. Ревью реализации — `docs/reviews/2026-09-27-changes-032-review.md` +(находки № 1–8) и `docs/reviews/2026-09-27-codebase-review.md` (находки № 9–15); все они исправлены в изменении 033 +(`docs/changes/033-review-fixes-032/SUMMARY.md`) — этот файл описывает исходную реализацию 032 в части модели данных +и объёма прав, без деталей самих находок ревью. + +## Модель данных +- `app/models.py`: `Role.superadmin`; `User.organization_id` (nullable, `FK organizations.id`, индекс) с + `CheckConstraint("is_active = false OR (role = 'superadmin') = (organization_id IS NULL)")` (имя `ck_users_role_org_scope` в модели — с 033); + `AuditLog.organization_id` (nullable, индекс, `FK organizations.id`). +- Миграции: `0010_user_role_superadmin.py` (значение enum `superadmin` через `autocommit_block()`), + `0011_users_organization_scope.py` (колонка `users.organization_id`, backfill ролей, CHECK после backfill), + `0012_audit_log_organization.py` (колонка `audit_log.organization_id`, backfill по `entity_id` для `organization`/`vrf`/`device`/`isp`/`prefix`; + для удалённых сущностей и системных типов (`user`/`journal`/`session`/`device_type`) остаётся `NULL` — видно только `superadmin`). +- Backfill ролей (решение 4 плана): бывшие `admin` → `superadmin`, `organization_id=NULL`; бывшие `viewer` → + `organization_id=NULL`, `is_active=false` (заблокированы до явного назначения организации `superadmin`). +- Удаление организации (`delete_org`) получило блокиратор `"users"` — организация с привязанными пользователями не удаляется. + +## Права доступа +Реализовано по сводной таблице плана: `superadmin` управляет пользователями, организациями (создание/удаление) и +типами устройств (запись); `admin`/`viewer` работают только в своей организации (VRF, префиксы, адреса, устройства, +операторы, карточка организации, журнал, «Обзор»); типы устройств читают все роли. + +- `app/security.py`: `admin_user` теперь пропускает роль ≠ `viewer` (то есть `admin` и `superadmin`); новая зависимость + `superadmin_user` — строго `Role.superadmin`. +- `app/services.py`: хелперы `require_org(user, organization_id, what)` (404 при чужой организации, `superadmin` — без + ограничений) и `scope_org(stmt, column, user)` (фильтр списков); `audit()` получил параметр + `organization_id` для простановки на журнальных записях (`refuse_delete()` — с 033). +- `app/api/v1/users.py` — все маршруты на `superadmin_user`; инвариант «нельзя обезвредить последнего активного + администратора» переведён на `superadmin`. +- `app/api/v1/refs.py`, `app/api/v1/prefixes.py`, `app/api/v1/journal.py`, `app/api/v1/overview.py` — `require_org`/`scope_org` + во всех точках чтения и записи, разделение прав на организации/типы устройств/журнал по сводной таблице. +- `app/main.py`: `seed()` создаёт пользователя-бутстрап с `role=Role.superadmin`, `organization_id=None`. + +## UI (`web/app.js`, `web/styles.css`) +- `ROLE_RU` дополнен «Суперадминистратор»; `isAdmin()` = роль ≠ `viewer`, новая `isSuperadmin()`. +- Пункт навигации «Пользователи» — только `isSuperadmin()`. `orgSwitcher()` интерактивен только для `superadmin`; для + `admin`/`viewer` — статичное название их организации. +- Экран «Организации»: «Добавить организацию» и «Удалить» — только `superadmin`. +- Экран «Пользователи»: диалог создания/правки получил выбор организации (скрывается для роли «Суперадминистратор»). +- Диалог типов устройств: управление — только `superadmin`, список виден всем. + +## Отклонения от плана +Без отклонений в объёме прав и модели данных. Первый проход реализации содержал несколько дефектов (сломанное +удаление организации, невалидируемый инвариант «роль — организация» при отсутствии поля в теле запроса, нерабочая +смена роли на/с `superadmin`, невидимость `*.delete_blocked` в журнале организации, оракулы существования чужих +объектов, глобальные счётчики типов устройств и мелкие UI-огрехи) — все исправлены отдельным циклом, изменение 033. + +## Проверено +- `import app.main`, `node --check web/app.js` — чисто. +- Миграции 0010–0012 применяются на стенде; `alembic check` без расхождений (после 033 — включая 0013). +- Изоляция чтения/записи по организациям, роли `admin`/`viewer`/`superadmin`, управление типами устройств и + пользователями — проверено на стенде (см. протокол сценариев в `docs/reviews/2026-09-27-changes-032-review.md` + и итоговые проверки в `docs/changes/033-review-fixes-032/SUMMARY.md`). + +## Не проверено +- Работоспособность 032 отдельно от 033 — не актуально (033 доводит 032 до готовности, коммитятся вместе). +- Полная историческая реконструкция `organization_id` в записях журнала для уже удалённых на момент миграции сущностей — + сознательно не выполнялась (план, раздел «Вне объёма»). diff --git a/docs/changes/033-review-fixes-032/PLAN.md b/docs/changes/033-review-fixes-032/PLAN.md new file mode 100644 index 0000000..9db7437 --- /dev/null +++ b/docs/changes/033-review-fixes-032/PLAN.md @@ -0,0 +1,134 @@ +# Исправление находок ревью изменения 032 (изменение 033) + +Источники: `docs/reviews/2026-09-27-changes-032-review.md` (находки № 1–8) и `docs/reviews/2026-09-27-codebase-review.md` (№ 9–15). +Изменение 032 в репозиторий не закоммичено. 033 доводит его до готовности, и коммитятся они вместе. + +## Порядок +| Этап | Находки | Суть | +|---|---|---| +| 1 | 1, 9 | Удаление организаций, утечка данных через предпросмотр подсети | +| 2 | 2, 3, 10, 8 | Проверка связки «роль — организация» по итоговому состоянию (API + диалог UI) | +| 3 | 4, 11, 12, 13 | Изоляция журнала и ответов, счётчики типов, имя CHECK | +| 4 | 5, 6, 7, 14, 15 | UI, стиль, документация | +| 5 | — | Тесты, очистка стенда, SUMMARY | + +--- + +## Этап 1 + +### № 1. FK `audit_log.organization_id` → `ON DELETE SET NULL` (критическая) +- `app/models.py`: `AuditLog.organization_id` → `ForeignKey("organizations.id", ondelete="SET NULL")`. +- Новая миграция `alembic/versions/0013_audit_log_org_fk_set_null.py`. 0012 уже применена на стенде, поэтому её не правим. + Миграция удаляет `fk_audit_log_organization_id_organizations` и создаёт её заново с `ondelete="SET NULL"`. `downgrade` — обратная операция. +- `users.organization_id` не трогаем: FK с `RESTRICT` там уместен, удаление закрывает блокиратор `users` в `delete_org`. +- Проверка: создать организацию → сразу `DELETE` → 204; записи журнала этой организации остаются с `organization_id = NULL`. + +### № 9. `GET /prefixes/{id}/subnets/next` без проверки организации (высокая) +- `app/api/v1/prefixes.py::preview_subnet`: добавить `user: User = Depends(current_user)`, прочитать префикс и вызвать + `require_org(user, p.organization_id, "Префикс")` до `_find_subnet`. +- Проверка: viewer организации B на префикс организации A → 404. + +## Этап 2 — связка «роль — организация» пользователя + +Общий принцип: строгий инвариант API (`superadmin` ⇔ `organization_id IS NULL`) проверяется **по итоговому состоянию записи**, +а не по наличию полей в теле запроса. Нарушение → 422 с понятным текстом, а не 409 от БД. + +### № 2. Валидация `UserIn` +- `app/schemas.py`: `@field_validator` в `UserIn` заменить на `@model_validator(mode="after")` с той же логикой и теми же текстами. +- Из `UserUpdate` валидатор убрать: без текущего состояния записи он не может проверить инвариант (см. № 3). + +### № 3. Смена роли в `update_user` +Файл `app/api/v1/users.py`. Итоговое состояние вычисляется до `apply_update`: +- `role_f = data.get("role", u.role)`, `active_f = data.get("is_active", u.is_active)`; +- если `role_f == superadmin`: + - `organization_id` явно передан → 422 «Суперадминистратор не привязан к организации»; + - иначе принудительно `data["organization_id"] = None` (повышение без ручной очистки организации); +- иначе `org_f = data.get("organization_id", u.organization_id)`. Если `org_f is None` и (`active_f` или в запросе есть `role`/`organization_id`) → + 422 «Администратор и просмотрщик должны быть привязаны к организации». Это касается и понижения суперадминистратора без `organization_id`. + Не затронутые запросом записи в унаследованном состоянии (например, неактивный viewer после миграции 0011) при смене пароля не ломаются. +- Если `organization_id` передан — `get_or_404(db, Organization, ...)` (см. № 10). +- Изменение `organization_id` попадает в `changed` и в журнал через существующий `apply_update`. + +### № 10. Ошибки целостности подписаны как «логин уже существует» +- `create_user`: до `db.add` — `get_or_404(db, Organization, body.organization_id, "Организация")`, если `organization_id` задан. + В `flush`/`commit` убрать текст «Пользователь с таким логином уже существует» и оставить `CONFLICT_MSG`: занятость логина уже проверяется + явным запросом выше, а редкую гонку двух одинаковых логинов покрывает общий текст. +- `update_user`: проверка организации — в № 3. +- Групповое «Разрешить доступ» для неактивного viewer без организации получит 422 с текстом из № 3. Отдельной правки не требует: `bulk()` уже показывает ошибки по строкам. + +### № 8 (+ UI-часть № 3). Диалог пользователя (`web/app.js::userDialog`) +- Поле «Организация» выводится всегда (кроме своей записи) и скрывается, если в селекторе роли **текущее** значение — `superadmin`. + Переключение роли в открытом диалоге сразу показывает или прячет поле: в обработчике `dd-pick` для `name="role"` переключать `hidden` + у обёртки поля, по образцу существующих зависимых полей, если они есть. + Условие `S.dialog && u?.role === "superadmin"` убрать. +- Отправка, одинаковая для создания и правки: `organization_id` добавляется в тело, только если `v.role !== "superadmin"` и значение выбрано. + При смене роли на `superadmin` в PATCH поле не передаётся — сервер очистит его сам (№ 3). + +## Этап 3 + +### № 4. `*.delete_blocked` в журнале организации +- `app/services.py::refuse_delete`: параметр `organization_id: int | None = None`, пробросить в `audit(...)`. +- Вызовы: `delete_org` → `id`; `delete_vrf` → `v.organization_id`; `delete_prefix` → `p.organization_id`; + `delete_type`, `delete_user` → без организации (системные события). + +### № 11. Различимые ответы для объектов чужой организации +Для не-`superadmin` чужой объект должен давать тот же 404, что и несуществующий. `superadmin` сохраняет прежние 422. +- `create_prefix`: `require_org(user, body.organization_id, ...)` → **затем** `_lock_vrf`, чтобы не блокировать чужой VRF. + После чтения VRF — `require_org(user, vrf.organization_id, "VRF")`, и только потом проверка 422 «VRF принадлежит другой организации». +- `_check_device(db, user, prefix, device_id)`: после `get_or_404` — `require_org(user, d.organization_id, "Устройство")`, затем прежняя проверка 422. + Обновить три вызова. +- `update_isp`: `require_org(user, body.organization_id, "Организация")` **до** `get_or_404(Organization)`, с тем же текстом. +- `update_prefix` с переносом VRF (`_move_to_vrf`): для не-`superadmin` чужой целевой VRF → 404 тем же приёмом. Сейчас это 422. + +### № 12. Счётчики типов устройств +- `app/api/v1/refs.py::_type_outs(db, rows, user)`: подсчёт устройств через `scope_org(..., Device.organization_id, user)`. + `list_types` получает `user: User = Depends(current_user)`. Для `create/update_type` (только `superadmin`) поведение не меняется. + +### № 13. Имя CHECK-ограничения +- `app/models.py`: `CheckConstraint(..., name="ck_users_role_org_scope")`, как в миграции 0011. Миграция не нужна. + +## Этап 4 + +### № 5. Кнопка «Добавить устройство» +- `screens.devices`: вернуть безусловный `btn("Добавить устройство", ...)` — устройства создаёт любой `admin` своей организации. + +### № 6. Бейдж `superadmin` +- `web/styles.css`: `.badge.purple{background:#efe8fb;color:#5b2a9e}` рядом с остальными модификаторами. + +### № 7. Локальный импорт `Role` +- `app/services.py`: `Role` — в верхнеуровневый `from app.models import ...`, локальные импорты в `require_org`/`scope_org` убрать. + +### № 14. IP и User-Agent суперадминистратора в журнале организации — решено: вариант A +- Код не меняется. В README («Журнал») описать, что события организации показывают IP и клиент исполнителя, включая суперадминистратора. +- Отклонённый вариант B — скрывать `client_ip`/`meta` чужих исполнителей. Потребовал бы ограничить и фильтры `client_ip`/`q`, + иначе IP восстанавливается подбором через фильтр. + +### Попутно (выявлено на проверке этапа 2) +- `get_or_404`/`require_org` формируют «{what} не найден» — для женского рода получается «Организация не найден». + Нужно согласовать род, например через `_NOUNS` из `app/services.py` или явный текст в вызове. + +### № 15. README +- Раздел «UI»: «Пользователи» (только `superadmin`). +- Роли: «последний активный `superadmin` защищён от понижения, отключения и удаления» — вместо текущей обратной формулировки. +- Раздел про связку «роль — организация»: смена роли на `superadmin` сама снимает организацию, понижение требует `organization_id`. +- Строка об изменении 033 в истории изменений, в формате README. + +## Этап 5 — тесты и стенд +Исполнитель правит код этапов 1–4 и не запускает тесты. Тесты обновляются после повторного ревью, минимально: +- `tests/test_users.py`: при создании `viewer` передать `organization_id` (через фикстуру `org`); добавить проверки в тот же сквозной сценарий: + `role="admin"` без организации → 422; `PATCH {"role": "superadmin"}` → 200 и `organization_id = null`; обратно без организации → 422. +- `tests/test_journal.py:43`: прямая SQL-вставка — роль `'superadmin'` (очистка журнала доступна только ему). +- Один новый тест изоляции (`tests/test_api.py`): viewer организации B → 404 на `GET /prefixes/{id_A}` и `GET /prefixes/{id_A}/subnets/next`. +- `test_delete_organization` проходит без изменений после № 1. +- Стенд: `alembic upgrade head`, затем удалить накопленные организации `test-*` (18 шт.) через API — после № 1 удаление работает. + +## Файлы +`app/models.py`, `app/schemas.py`, `app/services.py`, `app/api/v1/users.py`, `app/api/v1/prefixes.py`, `app/api/v1/refs.py`, +`alembic/versions/0013_audit_log_org_fk_set_null.py` (новый), `web/app.js`, `web/styles.css`, `README.md`, `tests/*` (этап 5), +`docs/changes/033-review-fixes-032/SUMMARY.md` (по завершении). + +## Проверка готовности +- `alembic upgrade head` и `alembic check` — чисто; `import app.main`, `node --check web/app.js` — без ошибок. +- `pytest -q` — все тесты зелёные. +- Сценарии на стенде: удаление новой организации; предпросмотр чужой подсети → 404; повышение `admin` → `superadmin` + и обратно через UI; `vrf.delete_blocked` виден админу организации в журнале. diff --git a/docs/changes/033-review-fixes-032/SUMMARY.md b/docs/changes/033-review-fixes-032/SUMMARY.md new file mode 100644 index 0000000..6c98e0f --- /dev/null +++ b/docs/changes/033-review-fixes-032/SUMMARY.md @@ -0,0 +1,77 @@ +# Итог: исправление находок ревью изменения 032 (изменение 033) + +Источники: `docs/reviews/2026-09-27-changes-032-review.md` (находки № 1–8) и `docs/reviews/2026-09-27-codebase-review.md` +(№ 9–15). Изменение 032 в репозиторий не закоммичено — 033 доводит его до готовности, коммитятся вместе. + +## Находки → что сделано → файлы + +| # | Серьёзность | Что сделано | Файлы | +|---|---|---|---| +| 1 | Критическая | FK `audit_log.organization_id → organizations.id` получил `ondelete="SET NULL"` (модель + новая миграция) | `app/models.py`, `alembic/versions/0013_audit_log_org_fk_set_null.py` | +| 2 | Высокая | `UserIn`: кросс-полевая проверка «роль — организация» переведена с `@field_validator` на `@model_validator(mode="after")` (срабатывает и когда поле не передано); из `UserUpdate` валидатор убран — без текущего состояния записи инвариант не проверить | `app/schemas.py` | +| 3 | Высокая | `update_user`: реконсиляция «роль — организация» по итоговому состоянию до `apply_update` — повышение до `superadmin` само очищает `organization_id`, понижение без `organization_id` → 422; `userDialog`: отправка `organization_id` только при роли ≠ `superadmin` (одинаково для create/edit) | `app/api/v1/users.py`, `web/app.js` | +| 4 | Средняя | `refuse_delete` получил параметр `organization_id`, проброшен в `audit()`; проставлен в `delete_org`, `delete_vrf`, `delete_prefix` (там, где организация объекта известна) | `app/services.py`, `app/api/v1/refs.py`, `app/api/v1/prefixes.py` | +| 5 | Низкая | `screens.devices`: безусловный `btn("Добавить устройство", …)` вместо мёртвого ветвления с неподдерживаемым `disabled` | `web/app.js` | +| 6 | Низкая | Добавлен модификатор `.badge.purple` для бейджа роли `superadmin` | `web/styles.css` | +| 7 | Инфо | Локальные `from app.models import Role` в `require_org`/`scope_org` убраны — используется верхнеуровневый импорт | `app/services.py` | +| 8 | Низкая | Поле «Организация» в `userDialog` реагирует на живой выбор роли в открытом диалоге (`hidden` переключается в обработчике `dd-pick` для `name="role"`), а не на исходную роль записи | `web/app.js` | +| 9 | Высокая | `preview_subnet` получил `user: User = Depends(current_user)` и `require_org(user, p.organization_id, "Префикс")` до `_find_subnet` | `app/api/v1/prefixes.py` | +| 10 | Средняя | `create_user`: проверка `organization_id` через `get_or_404(Organization)` до `db.add`; текст «Пользователь с таким логином уже существует» на `flush`/`commit` убран (логин уже проверен явным запросом), гонку покрывает общий `CONFLICT_MSG`; `update_user` — проверка организации по № 3 | `app/api/v1/users.py` | +| 11 | Низкая | Устранены оракулы «422 для чужого объекта / 404 для несуществующего»: `create_prefix` — `require_org` по организации тела **до** `_lock_vrf`, затем по организации VRF **до** проверки 422, затем по организации родительского префикса; `_check_device` — `require_org` до 422; `update_isp` — `require_org` по телу **до** `get_or_404(Organization)`; `update_prefix`/`_move_to_vrf` — `require_org` по целевому VRF до 422. Для `superadmin` прежние 422 сохранены | `app/api/v1/prefixes.py`, `app/api/v1/refs.py` | +| 12 | Низкая | `_type_outs`: подсчёт устройств через `scope_org(..., Device.organization_id, user)`; `list_types` получил `user: User = Depends(current_user)` | `app/api/v1/refs.py` | +| 13 | Низкая | `CheckConstraint` в модели получил `name="ck_users_role_org_scope"` — совпадает с именем в миграции 0011; отдельная миграция не нужна | `app/models.py` | +| 14 | Инфо | Решено: вариант A (см. ниже) | `README.md` | +| 15 | Инфо | README актуализирован: «Пользователи» только `superadmin`, формулировка «последний активный superadmin защищён…», раздел о связке «роль — организация», строка изменения 033 в истории | `README.md` | + +### Попутно (выявлено на проверке этапа 2) +Согласование рода в текстах 404: `get_or_404`/`require_org` формировали «{what} не найден» для любого `what`, из-за +чего получалось «Организация не найден» (женский род). Добавлена функция `not_found(what)` в `app/services.py` со +словарями исключений по первому слову (`_FEMININE_FIRST_WORDS = {"Организация", "Запись"}`, `_NEUTER_FIRST_WORDS = {"Устройство"}`), +используется в `get_or_404` и `require_org`. + +### Дополнительно (выявлено на ревью этапа 3) +При проверке № 11 обнаружен ещё один не закрытый оракул: `create_prefix` не проверял организацию **родительского** +префикса (`parent_id`) — чужой `parent_id` давал 422 вместо 404. Закрыто той же схемой: `require_org(user, parent.organization_id, "Родительский префикс")` +сразу после `get_or_404(db, Prefix, parent_id, ...)`, до проверки принадлежности VRF/подсети. Файл: `app/api/v1/prefixes.py`. + +## № 14. Метаданные суперадминистратора в журнале организации — решено: вариант A +Код не меняется. В README (раздел «Журнал аудита») явно описано: события организации показывают IP-адрес и клиент +(User-Agent) исполнителя, включая случаи, когда действие выполнил `superadmin`. Отклонённый вариант B (скрывать +`client_ip`/`meta` чужих исполнителей от админа/просмотрщика организации) потребовал бы также ограничивать фильтры +`client_ip`/`q` в `/audit` — иначе IP восстанавливается подбором через фильтр — и признан избыточным для этого цикла. + +## Миграция +`alembic/versions/0013_audit_log_org_fk_set_null.py` — пересоздаёт `fk_audit_log_organization_id_organizations` с +`ondelete="SET NULL"` (0012 уже применена на стенде, её саму не правим). `downgrade` возвращает FK без `ondelete`. + +## Тесты +- `tests/test_api.py::test_in_use_objects_cannot_be_deleted` — дополнен проверкой, что отказ в удалении VRF + (`vrf.delete_blocked`) виден в `/audit` от имени `admin` организации, у которой VRF отклонён (закрывает № 4). +- `tests/test_api.py::test_prefix_isolation_between_organizations` — новый тест: `admin` организации B получает 404 + на `GET /prefixes/{id_A}`, `GET /prefixes/{id_A}/subnets/next` (№ 9), `POST /prefixes` с чужим VRF и с чужим `parent_id` (№ 11). +- `tests/test_users.py::test_users_management` — создание `viewer` теперь передаёт `organization_id` (фикстура `org`); + добавлены проверки: `role="admin"` без `organization_id` → 422 (№ 2), `PATCH {"role": "superadmin"}` → 200 и + `organization_id = null`, обратное понижение без `organization_id` → 422, с `organization_id` → 200 (№ 3). +- `tests/test_journal.py::test_clear_requires_password_and_locks_out` — прямая SQL-вставка пользователя теперь с ролью + `'superadmin'` (очистка журнала доступна только ему после 032/033). + +## Проверки +- `pytest` — 15 passed. +- `alembic check` — чисто (расхождений нет), включая миграцию 0013. +- `downgrade` 0013 → 0012 и обратный `upgrade` — выполнены без ошибок. +- Сценарии на стенде: + - удаление организации — 204, записи журнала этой организации сохраняются с `organization_id = NULL` (№ 1); + - предпросмотр чужой подсети (`GET /prefixes/{id}/subnets/next`) от имени `admin`/`viewer` чужой организации — 404 (№ 9); + - повышение `admin` → `superadmin` и понижение обратно — 200/422 по сценарию № 3; + - `PATCH`/`POST` с несуществующей организацией — 404 (№ 10); + - чужие VRF, устройство, `parent_id`, организация оператора — 404 для `admin` своей организации, прежние 422 сохранены + для `superadmin` (№ 11); + - счётчики типов устройств (`devices_count`) — в пределах организации вызывающего (№ 12); + - `*.delete_blocked` виден `admin` организации в его собственном журнале (№ 4); + - тексты 404 согласованы по роду («Организация не найдена», «Устройство не найдено» и т. д.) — попутная правка; + - накопленные на стенде 18 организаций `test-*` удалены через API (стало возможно после № 1). + +## Не проверено +- Живое переключение поля «Организация» в диалоге пользователя при смене роли в открытом select (№ 8) — проверено + чтением кода, не проверено в браузере. +- Цвет бейджа `superadmin` (класс `.badge.purple`, № 6) — проверено чтением CSS, не проверено визуально в браузере. diff --git a/docs/reviews/2026-09-27-changes-032-review.md b/docs/reviews/2026-09-27-changes-032-review.md new file mode 100644 index 0000000..b2ea083 --- /dev/null +++ b/docs/reviews/2026-09-27-changes-032-review.md @@ -0,0 +1,150 @@ +# Ревью и тестирование изменения 032 — 2026-09-27 + +**Объём:** реализация `docs/changes/032-role-model-org-scope/PLAN.md` исполнителем на Haiku 4.5 (агент `ae8bd63b5568cb6ee`), незакоммиченные изменения +поверх `5210ba3`. 12 изменённых файлов, 3 новые миграции. +**Метод:** построчное чтение diff по каждому файлу, затем сквозные сценарии на стенде `ipam_control_006` (API напрямую + прямые SQL-запросы для диагностики), +затем регрессия `pytest`. Все находки ниже воспроизведены, не гипотетичны. Временные данные удалены. + +## Итог + +Модель прав (`require_org`/`scope_org`, разделение ролей по роутерам) реализована **широко и в целом корректно** — это подтверждено 16 из 24 сценариев ниже. +Но найдено **три серьёзных бага**, один из которых — **полная поломка удаления организаций** (регрессия ранее стабильной функции, изменение 007) — и они +подтверждаются штатным regression-прогоном: `pytest` даёт **3 упавших теста из 14**, и все три — прямое проявление находок №1–3. + +**Рекомендация: не коммитить как есть.** Нужен ещё один проход правок (аналогично циклам 024/030 в этой же сессии) на находки №1–3 минимум; №4 желательно +в том же заходе, №5–8 — по желанию. + +| # | Серьёзность | Файл(ы) | Кратко | +|---|---|---|---| +| 1 | **Критическая** | `alembic/versions/0012_audit_log_organization.py`, `app/models.py` | `audit_log.organization_id → organizations.id` без `ON DELETE SET NULL` — удаление организации сломано полностью | +| 2 | **Высокая** | `app/schemas.py` | Кросс-полевая проверка `organization_id` в `UserIn`/`UserUpdate` через `@field_validator` не срабатывает, если поле просто не передано в теле запроса | +| 3 | **Высокая** | `app/api/v1/users.py`, `web/app.js` | Повышение до `superadmin` / понижение с `superadmin` не работает при естественном вызове (без ручной синхронизации `organization_id`) | +| 4 | Средняя | `app/services.py` (`refuse_delete`) | Записи `*.delete_blocked` всегда `organization_id=NULL` — невидимы для админа организации в его собственном журнале | +| 5 | Низкая | `web/app.js` | Кнопка «Добавить устройство»: мёртвый код (`disabled` не поддерживается `btn()`), логика инвертирована по смыслу, хотя фактически не влияет | +| 6 | Низкая | `web/app.js`, `web/styles.css` | Бейдж роли `superadmin` использует класс `purple`, которого нет в CSS — рендерится без цвета | +| 7 | Инфо | `app/services.py` | `require_org`/`scope_org` делают `from app.models import Role` локально, хотя модуль уже импортирует из `app.models` на верхнем уровне | +| 8 | Низкая | `web/app.js` (`userDialog`) | Поле выбора организации не реагирует на смену роли в открытом диалоге (видимость считается по старой роли записи) | + +--- + +## Находки + +### 1. Удаление организации сломано полностью ✅ КРИТИЧНО +**Где:** `alembic/versions/0012_audit_log_organization.py` — `op.create_foreign_key(..., "audit_log", "organizations", ["organization_id"], ["id"])` +без `ondelete`; то же в `app/models.py` — `AuditLog.organization_id: Mapped[int | None] = mapped_column(ForeignKey("organizations.id"), index=True)`. + +**Механизм:** `create_org` пишет `audit(..., organization_id=o.id)` при создании организации — то есть у *любой* организации сразу же появляется +ссылающаяся строка в `audit_log`. FK без `ON DELETE SET NULL` (по умолчанию `NO ACTION`/`RESTRICT`) не даёт удалить организацию, пока существует +хоть одна такая запись — а она существует всегда. + +**Воспроизведение (минимальное):** создать организацию → сразу `DELETE /organizations/{id}` → 409 «Конфликт с существующими данными…» +(общий обработчик `IntegrityError`, без списка блокираторов — проверка `blockers()` до этого момента их не нашла, ошибка на уровне самого `DELETE`). +В логе PostgreSQL: `violates foreign key constraint "fk_audit_log_organization_id_organizations" ... Key (id)=(442) is still referenced from table "audit_log"`. + +**Подтверждено регрессией:** `pytest tests/test_api.py::test_delete_organization` падает именно на этом. + +**Исправление:** `ondelete="SET NULL"` у FK — и в модели, и в миграции (`op.create_foreign_key(..., ondelete="SET NULL")`). Это согласуется с уже принятым +для этой колонки принципом «история удалённых сущностей остаётся, `organization_id` уходит в `NULL`» — тем же, что применён к `entity_id` +(без FK вообще) и описан в самом плане 032 для backfill. + +### 2. Валидатор `organization_id` не срабатывает при отсутствии поля в теле запроса ✅ ВЫСОКАЯ +**Где:** `app/schemas.py`, `UserIn`/`UserUpdate` — `@field_validator("organization_id")`. + +**Причина (проверено экспериментально на минимальном примере с pydantic 2.13):** `@field_validator` **не вызывается**, если поле отсутствует +в переданных данных и используется его значение по умолчанию (`None`) — только если оно передано явно (в том числе `null`). Это штатное поведение +Pydantic v2, не баг библиотеки, но означает, что кросс-полевая проверка «admin/viewer обязаны иметь организацию» реально работает только тогда, +когда клиент явно прислал `"organization_id": null` — а не когда просто не указал ключ (обычный способ «не задавать поле» в JSON API). + +**Последствие:** `POST /users {"username": "...", "password": "...", "role": "admin"}` (без ключа `organization_id`) не даёт ожидаемых 422. +Запрос доходит до `db.add()`/`flush()`, ловится `CheckConstraint` в БД → `IntegrityError` → попадает в блок `flush(db, "Пользователь с таким +логином уже существует")`, который **любую** `IntegrityError` подписывает этим текстом. Клиент получает 409 «Пользователь с таким логином уже +существует» для **несуществующего** логина — вводящее в заблуждение сообщение, маскирующее реальную причину. + +**Подтверждено регрессией:** `pytest tests/test_users.py::test_users_management` падает ровно на этом первом же вызове +(`client.post("/users", json={"username": name, "password": "start-pass-123", "role": "viewer"})` — без `organization_id`). +Также `pytest tests/test_journal.py::test_clear_requires_password_and_locks_out` падает с `CheckViolation` на прямой SQL-вставке — показывает, +что БД-ограничение единственная линия защиты и она долетает как сырая ошибка там, где путь не идёт через API вообще. + +**Исправление:** заменить `@field_validator("organization_id")` на `@model_validator(mode="after")` в обеих схемах — он выполняется всегда, +по итоговому состоянию модели, независимо от того, какие поля были явно переданы. + +### 3. Смена роли на/с `superadmin` не работает при обычном вызове ✅ ВЫСОКАЯ +**Где:** `app/api/v1/users.py::update_user`; сопутствующий пробел в `web/app.js::userDialog`. + +**Воспроизведено дважды, в обе стороны:** +- Повышение: `PATCH /users/{id} {"role": "superadmin"}` (без `organization_id`) на существующем `admin` с заполненной организацией → 409 + «Конфликт с существующими данными…» (тот же `CheckConstraint`, `organization_id` в БД не очищен). Роль не меняется, пользователь остаётся как был. +- Понижение: `PATCH /users/{id} {"role": "admin"}` (без `organization_id`) на существующем `superadmin` → тот же 409 (`organization_id` не проставлен, + остаётся `NULL`, что нарушает ограничение уже для роли `admin`). +- Контрольный, «правильный» вызов с явным `organization_id` в теле — проходит (200). То есть путь существует, но требует от клиента знания + о внутреннем ограничении БД, которое нигде не задокументировано и не проверяется на уровне API до `commit()`. + +**Причина:** `update_user` строит `data = body.model_dump(exclude_unset=True, exclude_none=True)` и применяет её через `apply_update` без +дополнительной реконсиляции: если `role` меняется на/с `superadmin`, а `organization_id` в этом же запросе не передан, старое значение поля +остаётся нетронутым — и это ровно то состояние, которое ограничение в БД запрещает (кроме случая `is_active=false`, который здесь не применим). + +**В UI это дополнительно усугублено:** `userDialog` при создании нового пользователя корректно исключает `organization_id` из тела, если выбрана +роль `superadmin` (`if (v.role !== "superadmin" && v.organization_id) body.organization_id = ...`), но **при редактировании существующего** +такой же проверки нет (`if (v.organization_id) body.organization_id = Number(v.organization_id);` — без учёта роли). Хуже того, поле-селектор +организации в форме скрывается по **прежней** роли записи (`u?.role === "superadmin"`), а не по текущему выбору в открытом select — так что +даже если оператор переключит роль на «Суперадминистратор» в уже открытом диалоге, поле организации останется видимым со старым значением +и уйдёт в тело запроса. Через UI сейчас **невозможно** повысить существующего администратора организации до суперадминистратора. + +**Исправление:** в `update_user` — при вычислении финальной роли принудительно приводить `organization_id`: если итоговая роль `superadmin`, +записывать `u.organization_id = None` независимо от того, что пришло в запросе; если роль уходит от `superadmin` и `organization_id` не передан +явно — требовать его (422 с понятным текстом), а не полагаться на молчаливое сохранение старого (в данном случае отсутствующего) значения. +В `web/app.js` — тот же guard `v.role !== "superadmin"` в ветке `edit`, и видимость поля организации должна реагировать на текущее значение +селектора роли в форме, а не на исходную роль записи. + +### 4. Отказы в удалении не попадают в журнал организации ✅ СРЕДНЯЯ +**Где:** `app/services.py::refuse_delete` — не тронута агентом; по-прежнему вызывает `audit(...)` без `organization_id`, и ни один из пяти call-site'ов +(`refs.py` ×4, `prefixes.py` ×1, `users.py` ×2) не передаёт его. + +**Воспроизведено:** `admin` организации A пытается удалить используемый VRF своей же организации → корректно получает 409 (бизнес-правило работает). +Но `GET /audit?event_type=vrf.delete_blocked` от его же имени возвращает `{"total": 0}` — событие существует (видно суперадминистратору), но +`organization_id = NULL`, и `scope_org` его отфильтровывает. Админ организации не видит в своём журнале собственные отклонённые попытки удаления. + +**Исправление:** добавить `organization_id: int | None = None` в сигнатуру `refuse_delete` и передавать его на всех вызовах (там же, где рядом +уже передаётся `organization_id` в соседние вызовы `audit()` для тех же сущностей — паттерн уже есть в `prefixes.py`/`refs.py` для успешных операций). + +### 5–8. Мелкие (UI/стиль) 🔎 +- **№5:** `screens.devices` — `btn(..., { disabled: true })`; функция `btn()` (`web/app.js`) не поддерживает ключ `disabled` — он молча игнорируется. + Ветвление `isSuperadmin() ? btn(...) : btn(..., {disabled:true})` фактически рендерит один и тот же (рабочий) элемент в обеих ветках, поэтому + функционально org-admin по-прежнему может добавлять устройства (это подтверждено сценарием), но код вводит в заблуждение и должен быть упрощён + до безусловного `btn(...)`, как было раньше — устройства создаёт любой `admin` своей организации, не только `superadmin`. +- **№6:** `badge(u.role === "superadmin" ? "purple" : ...)` — в `web/styles.css` определены только `.badge.green/blue/red/amber`; `purple` рендерится + без модификатора (стандартный серый бейдж), роль суперадминистратора визуально не выделяется. +- **№7:** `require_org`/`scope_org` (`app/services.py`) делают `from app.models import Role` внутри тела функции при каждом вызове, хотя модуль уже + импортирует из `app.models` на верхнем уровне (`Address, AuditLog, Base`) — нет циркулярной причины для локального импорта, чисто стилистически. +- **№8:** см. разбор в находке №3 — видимость поля организации в `userDialog` не реактивна к живому выбору роли в форме. + +--- + +## Что проверено и работает корректно ✅ +Двадцать четыре сценария на стенде (организации/VRF/префиксы/устройства/операторы/типы устройств/пользователи/журнал/обзор), из них 16 без замечаний: +- Бутстрап-администратор из `.env` после миграций стал `superadmin` с `organization_id=NULL`. +- Изоляция чтения: `admin`/`viewer` организации видят только свою организацию в списках и по прямому `id` (404 на чужую, без утечки существования); + явный `organization_id` в query-параметрах чужой организации — тоже 404, не пустой список (осознанное и разумное отличие от плана, не баг). +- Запись в своей организации: префиксы, устройства — создаются; в чужой — 404. +- `viewer` читает, любая попытка записи — 403. +- Типы устройств: читают все роли; создание/правка/удаление — только `superadmin` (403 для `admin` организации). +- Управление пользователями, `/journal/settings`, `/journal/clear` — только `superadmin` (403 для остальных). +- «Обзор» — счётчики и `recent_changes` корректно урезаны до организации вызывающего (когда организация вообще видна в журнале — см. находку №4). +- Блокиратор `"users"` при удалении организации с привязанными пользователями — корректно собирается и полностью виден в `diff.blocked_by` записи + журнала (со списком логинов и `total`) — сама механика `refuse_delete`/`blockers` для этой новой группы работает без замечаний. +- `alembic check` — «No new upgrade operations detected», миграции 0010–0012 применяются на стенде без ручного вмешательства. +- `import app.main`, `node --check web/app.js` — чисто. + +## Регрессия (`pytest -q`) +``` +3 failed, 11 passed in 8.32s +FAILED tests/test_api.py::test_delete_organization — находка №1 +FAILED tests/test_journal.py::test_clear_requires_password_and_locks_out — находка №2 (CheckViolation на прямой SQL-вставке без organization_id) +FAILED tests/test_users.py::test_users_management — находка №2 +``` +Все три падения — прямое проявление находок №1–2, не случайные/посторонние сбои. Обновление самих тестов под новую модель (добавление +`organization_id` в фикстуры пользователей и т. п.) — отдельный, последующий шаг после того, как находки №1–3 будут исправлены в коде. + +## Рекомендация +Исправить находки №1–3 (и желательно №4) отдельным заходом — по аналогии с циклами 024/030 в этой сессии: план → агент на правки кода → +повторное ревью и тесты. До этого изменение 032 не готово к коммиту. diff --git a/docs/reviews/2026-09-27-codebase-review.md b/docs/reviews/2026-09-27-codebase-review.md new file mode 100644 index 0000000..6a83f4d --- /dev/null +++ b/docs/reviews/2026-09-27-codebase-review.md @@ -0,0 +1,93 @@ +# Ревью кодовой базы — 2026-09-27 + +**Объём:** рабочее дерево поверх `5210ba3` (незакоммиченные изменения 031–032, миграции 0010–0012). +**Метод:** сверка с `docs/reviews/2026-09-27-changes-032-review.md` → построчное чтение `app/`, `alembic/versions/0010–0012`, diff `web/app.js` → +воспроизведение на стенде `ipam_control_006` (API + SQL) → `pytest`. Временные данные удалены. + +## Итог +Находки ревью 032 **не исправлены, все подтверждены повторно** (`pytest`: 3 failed / 11 passed — те же три теста). +Собственное ревью добавляет **одну уязвимость изоляции организаций** (№ 9, подтверждена на стенде) и несколько мелких замечаний. +Изменение 031 существует только как план — в коде не реализовано. + +**Рекомендация:** не коммитить 032 до исправления № 1–4 и № 9. + +## Статус находок ревью 032 + +| # | Серьёзность | Статус | Проверка | +|---|---|---|---| +| 1 | Критическая | Не исправлено | FK `audit_log.organization_id` без `ON DELETE SET NULL` (модель + 0012). Уточнение: одного `SET NULL` по существующим строкам недостаточно — `delete_org` сам пишет `audit(..., organization_id=id)` в той же транзакции перед `DELETE`; на стенде даже после ручного обнуления — 409. Нужен именно `ondelete="SET NULL"` | +| 2 | Высокая | Не исправлено | `UserIn(role="admin")` без ключа `organization_id` проходит валидацию (проверено в venv); явный `null` — отклоняется | +| 3 | Высокая | Не исправлено | `update_user` не реконсилирует `organization_id` при смене роли | +| 4 | Средняя | Не исправлено | `refuse_delete` без `organization_id` | +| 5–8 | Низкая/инфо | Не исправлено | Без изменений | + +Побочный эффект № 1: на стенде накоплено **18 организаций `test-*`** — фикстура `org` не может их удалить. После исправления — почистить. + +## Новые находки + +| # | Серьёзность | Файл | Кратко | +|---|---|---|---| +| 9 | **Высокая** | `app/api/v1/prefixes.py:286` | `GET /prefixes/{id}/subnets/next` без `require_org` — чтение чужой организации | +| 10 | Средняя | `app/api/v1/users.py`, `app/services.py` | Несуществующий `organization_id` / нарушение CHECK → 409 «логин уже существует» или обезличенный 409 | +| 11 | Низкая | `prefixes.py:241,355`, `refs.py:354-355` | Различимые ответы 404/422 раскрывают существование объектов чужой организации | +| 12 | Низкая | `app/api/v1/refs.py:174` | `/device-types` отдаёт `devices_count` по всем организациям | +| 13 | Низкая | `app/models.py`, `0011` | `CheckConstraint` в модели без имени, в миграции — `ck_users_role_org_scope` | +| 14 | Инфо | `app/api/v1/journal.py` | Админ организации видит IP и User-Agent суперадминистратора в событиях своей организации | +| 15 | Инфо | `README.md` | Устаревшие/неточные формулировки | + +### 9. Утечка данных чужой организации через предпросмотр подсети — ВЫСОКАЯ +`preview_subnet` не принимает `user` и не вызывает `require_org` — единственный GET по `id` префикса, пропущенный в 032. +**Воспроизведено:** `viewer` организации B: `GET /prefixes/{id_A}` → 404 (изоляция), `GET /prefixes/{id_A}/subnets/next?length=24` → +**200** `{"prefix":"10.77.0.0/24","length_min":17,"length_max":32}`; несуществующий id → 404. +Раскрывается: существование префикса, его длина (`length_min − 1`), занятость (перебором `length` восстанавливается раскладка подсетей). +**Исправление:** `user: User = Depends(current_user)` + `require_org(user, p.organization_id, "Префикс")`. + +### 10. Ошибки целостности маскируются под «логин уже существует» +`create_user` оборачивает `flush/commit` текстом «Пользователь с таким логином уже существует» — им же подписываются нарушение FK +(`organization_id` несуществующей организации) и CHECK (№ 2). `update_user` при тех же причинах отдаёт общий `CONFLICT_MSG`. Отдельно: повторная +активация мигрированного `viewer` (`organization_id=NULL`) через «Разрешить доступ» (в т. ч. групповое) → непонятный 409. +**Исправление:** в `create_user`/`update_user` — `get_or_404(Organization, organization_id)` до записи; проверку инварианта роль/организация делать +по итоговому состоянию (роль, организация, активность) с 422 и понятным текстом — это же закрывает № 2–3. + +### 11. Оракул существования объектов чужой организации +Для чужих объектов ответ отличается от несуществующих: `create_prefix` — 422 «VRF принадлежит другой организации» против 404; +`_check_device` — 422 против 404; `update_isp` — `get_or_404(Organization)` до `require_org` (разные тексты 404). Также `create_prefix` +берёт advisory-lock чужого VRF до `require_org`. Противоречит принципу 032 «404, не подтверждать существование». +**Исправление:** для не-`superadmin` отвечать 404 при чужой организации (или вызывать `require_org` по организации объекта до остальных проверок). + +### 12. Глобальные счётчики типов устройств +`list_types` считает устройства по всем организациям — `admin`/`viewer` видит объём чужих данных (на стенде видно `devices_count=1` у типа без +устройств своей организации). Решение 2 плана относится к справочнику, не к счётчикам. **Исправление:** `scope_org` в подсчёте `_type_outs`. + +### 13. Имя CHECK-ограничения расходится +Модель: `CheckConstraint(...)` без `name`; миграция: `ck_users_role_org_scope`. `alembic check` CHECK не сравнивает, поэтому расхождение не видно, +но `create_all` (и любая будущая автогенерация/drop) получит другое имя. **Исправление:** `name="ck_users_role_org_scope"` в модели. + +### 14. Метаданные запросов суперадминистратора в журнале организации +События, выполненные `superadmin` в организации, несут `client_ip` и `meta` (User-Agent, путь) и видны админу/просмотрщику этой организации. +Не баг плана, но стоит решить осознанно (например, скрывать `client_ip/meta` чужих акторов для не-`superadmin`). + +### 15. README +- Строка 135: «Пользователи» (только admin) → только `superadmin`. +- Строка 96: «защищена от отключения/удаления при наличии других активных superadmin» — смысл обратный: защищён *последний* активный. +- Нет раздела о 031 (Swagger, TLS-профиль) — ожидаемо, изменение не реализовано. + +## Прочее +- **031:** в `docs/changes/031-expose-hardening/` только `PLAN.md`; `docs_enabled`, `app_bind`, `Caddyfile`, профиль `tls` отсутствуют — находки пентеста № 1–2 открыты. +- **Артефакты 032:** нет итогового файла (суммаризации) — ожидаемо до завершения цикла правок. +- **Тесты:** после исправлений обновить `test_users` (явный `organization_id`) и прямую SQL-вставку в `test_journal`; добавить один тест + изоляции (viewer чужой организации → 404 на `GET /prefixes/{id}` и `/subnets/next`) — закрывает № 9 от регрессии. + +## Что проверено без замечаний +- Все мутации (`VRF`, префиксы, адреса, устройства, операторы, организации) проверяют организацию до изменения; `*Update`-схемы не принимают + `organization_id` — перенос объектов между организациями невозможен; перенос префикса между VRF проверяет организацию целевого VRF. +- `current_user` читает роль и организацию из БД на каждый запрос — смена роли/организации действует сразу, без перевыпуска токена. +- Журнал: `list/summary/facets/get` фильтруются по организации; `settings/clear` — только `superadmin`. +- UI: `loadOrgs` сбрасывает сохранённую в `localStorage` организацию, если она недоступна пользователю. +- Миграции 0010 (enum в `autocommit_block`) и 0011 (CHECK после backfill) — корректный порядок. + +## Порядок исправлений +1. № 1 (`ondelete="SET NULL"` в модели и новой миграции 0013 — 0012 уже применена на стенде), № 9. +2. № 2, 3, 10 — единая проверка инварианта по итоговому состоянию в `users.py` + правка `userDialog` (№ 3, 8). +3. № 4, 11, 12, 13. +4. № 5–7, 15; обновление тестов; очистка организаций `test-*` на стенде. diff --git a/docs/reviews/2026-09-27-pentest.md b/docs/reviews/2026-09-27-pentest.md new file mode 100644 index 0000000..1597ae0 --- /dev/null +++ b/docs/reviews/2026-09-27-pentest.md @@ -0,0 +1,41 @@ +# Пентест приложения (чёрный ящик) — 2026-09-27 + +**Цель:** `192.168.5.9:8088` (стенд `ipam_control_006`). +**Условия игры:** известны только адрес и порт. Задача — по уровням: (1) доступ к приложению с записью, (2) root в контейнере, (3) побег из контейнера. +**Метод:** внешняя разведка и проверка авторизации по чёрному ящику. Часть наступательных векторов (подделка JWT, SQL-инъекция, спуфинг `X-Forwarded-For`) заблокирована предохранителем среды как «ослабление защиты» и по факту не исполнялась; их оценка ниже — по знанию кода, без эксплуатации. + +## Результат по уровням + +**Уровень 1 — доступ с записью: НЕ получен.** +**Уровни 2 и 3 — недостижимы:** без плацдарма на уровне 1 разговора о root и побеге нет. Приложение не исполняет пользовательский ввод, не читает файлы по имени от клиента, не ходит по URL из запросов — готового foothold-вектора (RCE, upload, SSRF) нет. + +## Что проверено на живом стенде + +| Вектор | Результат | +|---|---| +| `POST /organizations` без токена | 401 | +| `GET /users` без токена | 401 | +| Типовые пароли `admin` (admin, admin123, password, 123456, changeme, change-me, ipam, ipam123, Admin123, root) | 401, после 5-й попытки — 429 (лимит «логин+IP», изменение 026) | +| Заголовки ответа `/` | CSP, `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: no-referrer`; сервер только `uvicorn` | +| `/healthz` | `{"status":"ok"}` | +| `/docs`, `/openapi.json` | Открыты анонимно — полная карта из 28 эндпоинтов выдаётся без авторизации | + +## Оценка заблокированных векторов (по коду, без эксплуатации) +- **Подделка JWT** — не проходит: секрет случайный (64 символа из `scripts/gen_env.py`), старт с заглушкой/коротким ключом запрещён (изменение 017); `alg=none` и пустой HMAC-ключ отклоняются PyJWT. +- **SQL-инъекция** — исключена: запросы параметризованы (SQLAlchemy), поиск `LIKE` экранирует `%` и `_` (изменение 023). +- **Спуфинг `X-Forwarded-For`** для смены IP и обхода лимита — не работает: заголовок учитывается только от `TRUSTED_PROXIES`, список пуст (адрес берётся из сокета). +- **Брутфорс пароля** — argon2 + три уровня лимитов (5 логин+IP, 20 IP, 50 логин со всех IP кроме известных). +- **Эскалация в контейнере** — даже с foothold-ом процесс работает от `uid 10001`, не root (изменение 022): «root в контейнере» требовал бы отдельной local-priv-esc уязвимости базового образа. + +## Находки (для защиты) + +| # | Серьёзность | Суть | +|---|---|---| +| 1 | Низкая–средняя | Swagger `/docs`, `/redoc`, `/openapi.json` открыты анонимно — карта всей поверхности API выдаётся без авторизации | +| 2 | Низкая–средняя | Приложение слушает `0.0.0.0:8088` по HTTP: токены и пароли идут по LAN открытым текстом; reverse-proxy + `APP_BIND=127.0.0.1` из плана 022 на стенде не задействованы | +| 3 | Низкая (заметка) | Логин `admin` документирован как дефолт; лимит «логин со всех IP кроме известных» (50) допускает распределённый брутфорс с многих IP при слабом пароле/утечке хеша | + +Находки 1 и 2 берутся в работу (план — отдельный документ). Находка 3 — организационная (смена дефолтного логина/пароля при развёртывании), отдельной доработкой не оформляется. + +## Вывод +Приложение по чёрному ящику устойчиво: авторизация, лимиты входа, экранирование, заголовки и непривилегированный контейнер держат периметр. Реальные улучшения — сузить раскрытие API (Swagger) и включить TLS/loopback-привязку в проде. diff --git a/tests/test_api.py b/tests/test_api.py index 20b59dc..2640439 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,4 +1,6 @@ import ipaddress +import uuid + import httpx from tests.conftest import BASE, ENV @@ -45,6 +47,22 @@ def test_in_use_objects_cannot_be_deleted(client, org): client.post("/devices", json={"name": "t-1.internal", "device_type_id": t["id"], "organization_id": org["id"]}) assert client.delete(f"/device-types/{t['id']}").status_code == 409 + # изменение 033, находка №4: отказ в удалении VRF попадает в журнал с organization_id и виден админу этой организации + name, uid, admin = f"qa-{uuid.uuid4().hex[:8]}", None, None + try: + created = client.post("/users", json={"username": name, "password": "start-pass-123", "role": "admin", "organization_id": org["id"]}) + assert created.status_code == 201, created.text + uid = created.json()["id"] + admin = httpx.Client(base_url=BASE, timeout=30) + r = admin.post("/auth/login", json={"username": name, "password": "start-pass-123"}) + admin.headers["Authorization"] = "Bearer " + r.json()["access_token"] + assert admin.get("/audit", params={"event_type": "vrf.delete_blocked"}).json()["total"] >= 1 + finally: + if admin is not None: + admin.close() + if uid is not None: + client.delete(f"/users/{uid}") + def test_delete_organization(client, org): _prefix(client, org, "10.206.0.0/24") @@ -77,6 +95,41 @@ def test_allocate_next_subnet(client, org): assert next_free_subnet("fd00::/64", 66, [(v6, v6 + 5)]) == "fd00::4000:0:0:0/66" # IPv6: первый блок занят, берётся второй +def test_prefix_isolation_between_organizations(client, org): + """Изменение 033, находка №9: чужой организации не должен быть виден чужой префикс ни через GET, ни через предпросмотр подсети. + Находка №11: admin чужой организации получает тот же 404 при попытке создать префикс в VRF организации A.""" + other = client.post("/organizations", json={"name": f"other-{org['name']}", "inn": "".join(reversed(org["inn"]))}).json() + pid = _prefix(client, org, "10.208.0.0/24").json()["id"] + name, uid, admin_b = f"qa-{uuid.uuid4().hex[:8]}", None, None + try: + created = client.post("/users", json={"username": name, "password": "start-pass-123", "role": "admin", "organization_id": other["id"]}) + assert created.status_code == 201, created.text + uid = created.json()["id"] + + admin_b = httpx.Client(base_url=BASE, timeout=30) + r = admin_b.post("/auth/login", json={"username": name, "password": "start-pass-123"}) + assert r.status_code == 200 + admin_b.headers["Authorization"] = "Bearer " + r.json()["access_token"] + + assert admin_b.get(f"/prefixes/{pid}").status_code == 404 + assert admin_b.get(f"/prefixes/{pid}/subnets/next", params={"length": 24}).status_code == 404 + # изменение 033, находка №11: чужой VRF в create_prefix — 404 (раньше было 422) + cross = admin_b.post("/prefixes", json={"organization_id": other["id"], "vrf_id": org["vrf_id"], "prefix": "10.209.0.0/24"}) + assert cross.status_code == 404 + # изменение 033, находка №11: чужой parent_id в create_prefix — тоже 404 (раньше было 422) + other_vrf_id = client.get("/vrfs", params={"organization_id": other["id"]}).json()["items"][0]["id"] + cross_parent = admin_b.post("/prefixes", json={"organization_id": other["id"], "vrf_id": other_vrf_id, "prefix": "10.208.0.0/25", "parent_id": pid}) + assert cross_parent.status_code == 404 + finally: + if admin_b is not None: + admin_b.close() + if uid is not None: + client.delete(f"/users/{uid}") + for v in client.get("/vrfs", params={"organization_id": other["id"]}).json()["items"]: + client.delete(f"/vrfs/{v['id']}") + client.delete(f"/organizations/{other['id']}") + + def test_vrf_name_unique_per_organization(client, org): other = client.post("/organizations", json={"name": f"other-{org['name']}", "inn": "".join(reversed(org["inn"]))}).json() try: diff --git a/tests/test_journal.py b/tests/test_journal.py index 8db816e..090b059 100644 --- a/tests/test_journal.py +++ b/tests/test_journal.py @@ -40,7 +40,7 @@ def test_rotation_by_age_and_count(client, db): def test_clear_requires_password_and_locks_out(db): name, pw = f"tmp-{uuid.uuid4().hex[:6]}", "tmp-pass-123" - db.execute("INSERT INTO users (username, password_hash, role, is_active) VALUES (%s, %s, 'admin', true)", (name, hash_password(pw))) + db.execute("INSERT INTO users (username, password_hash, role, is_active) VALUES (%s, %s, 'superadmin', true)", (name, hash_password(pw))) # изменение 033: очистка журнала — только superadmin try: c = httpx.Client(base_url=BASE) c.headers["Authorization"] = "Bearer " + c.post("/auth/login", json={"username": name, "password": pw}).json()["access_token"] diff --git a/tests/test_users.py b/tests/test_users.py index 3de9ce1..6d4755c 100644 --- a/tests/test_users.py +++ b/tests/test_users.py @@ -17,23 +17,25 @@ def _client(username: str, password: str): return c -def test_users_management(client): +def test_users_management(client, org): name, uid, other = f"qa-{uuid.uuid4().hex[:8]}", None, None try: - created = client.post("/users", json={"username": name, "password": "start-pass-123", "role": "viewer"}) + created = client.post("/users", json={"username": name, "password": "start-pass-123", "role": "viewer", "organization_id": org["id"]}) # изменение 033 assert created.status_code == 201, created.text uid = created.json()["id"] assert created.json()["role"] == "viewer" and created.json()["is_active"] is True + assert created.json()["organization_id"] == org["id"] # изменение 033 - assert client.post("/users", json={"username": name.upper(), "password": "start-pass-123"}).status_code == 409 # логин занят без учёта регистра + assert client.post("/users", json={"username": name.upper(), "password": "start-pass-123", "organization_id": org["id"]}).status_code == 409 # логин занят без учёта регистра assert client.post("/users", json={"username": "system", "password": "start-pass-123"}).status_code == 422 # служебный логин журнала assert client.post("/users", json={"username": "ab", "password": "start-pass-123"}).status_code == 422 # короткий логин assert client.post("/users", json={"username": "short-pw", "password": "123"}).status_code == 422 # короткий пароль + assert client.post("/users", json={"username": f"qa-{uuid.uuid4().hex[:8]}", "password": "start-pass-123", "role": "admin"}).status_code == 422 # админ без организации (изменение 033, находка №2) other = _client(name, "start-pass-123") assert other is not None assert other.get("/auth/me").json()["role"] == "viewer" - assert other.get("/users").status_code == 403 # список пользователей только для админа + assert other.get("/users").status_code == 403 # список пользователей только для суперадминистратора (изменение 032) assert other.post("/organizations", json={"name": "qa", "inn": "1234567890"}).status_code == 403 # роль «просмотр» — только чтение # отключение действует немедленно, включая ранее выданный токен @@ -62,6 +64,14 @@ def test_users_management(client): assert client.delete(f"/users/{me['id']}").status_code == 409 assert client.patch(f"/users/{me['id']}", json={"is_active": False}).status_code == 409 assert client.patch(f"/users/{uid}", json={"role": "admin"}).json()["role"] == "admin" + + # реконсиляция «роль — организация» по итоговому состоянию (изменение 033, находка №3) + promoted = client.patch(f"/users/{uid}", json={"role": "superadmin"}) + assert promoted.status_code == 200 and promoted.json()["organization_id"] is None # повышение снимает организацию само + assert client.patch(f"/users/{uid}", json={"role": "viewer"}).status_code == 422 # понижение без организации + demoted = client.patch(f"/users/{uid}", json={"role": "viewer", "organization_id": org["id"]}) + assert demoted.status_code == 200 and demoted.json()["role"] == "viewer" + assert client.delete(f"/users/{uid}").status_code == 204 uid = None assert client.get("/audit", params={"event_type": "user.created"}).json()["total"] >= 1 diff --git a/web/app.js b/web/app.js index 7fd69d2..caaf2ef 100644 --- a/web/app.js +++ b/web/app.js @@ -87,8 +87,9 @@ const iconBtn = (icon, action, label, data = "", sm = false, disabled = false) = const searchBox = (value, placeholder, width) => ``; const currentOrg = () => orgs.find((o) => o.id === store.orgId); -const isAdmin = () => user?.role === "admin"; -const ROLE_RU = { admin: "Администратор", viewer: "Просмотр" }; +const isAdmin = () => user?.role !== "viewer"; // изменение 032: admin И superadmin +const isSuperadmin = () => user?.role === "superadmin"; // изменение 032 +const ROLE_RU = { superadmin: "Суперадминистратор", admin: "Администратор", viewer: "Просмотр" }; // изменение 032 /* ------------------------------------------------ выбор строк и групповые операции (только admin) */ const selSet = () => (S.sel ||= new Set()); @@ -119,14 +120,18 @@ function filterBtn(id, label, items, value, width = 180) { } function orgSwitcher() { const o = currentOrg(); - return `${popMenu("org", orgs.map((x) => ({ label: x.name, value: x.id, cls: x.id === store.orgId ? "sel" : "" })))}
`; + // изменение 032: интерактивный переключатель для superadmin, статичная подпись для других + if (isSuperadmin()) { + return `${popMenu("org", orgs.map((x) => ({ label: x.name, value: x.id, cls: x.id === store.orgId ? "sel" : "" })))}
`; + } + return `${I.org()}Организация: ${esc(o?.name ?? "—")}
`; } const crumbsOrg = () => `
Организации${I.chevR(14)}${esc(currentOrg()?.name ?? "")}
`; const header = (title, sub, actions = "") => `

${esc(title)}

${sub}
${actions}
`; -const NAV = [["overview", "Обзор"], ["prefixes", "Префиксы"], ["orgs", "Организации"], ["isps", "Операторы"], ["devices", "Устройства"], ["journal", "Журнал"], ["users", "Пользователи", "admin"]]; +const NAV = [["overview", "Обзор"], ["prefixes", "Префиксы"], ["orgs", "Организации"], ["isps", "Операторы"], ["devices", "Устройства"], ["journal", "Журнал"], ["users", "Пользователи", "superadmin"]]; // изменение 032: "superadmin" function shell(active, content) { - const nav = NAV.filter(([, , role]) => role !== "admin" || isAdmin()); // раздел «Пользователи» виден только администратору + const nav = NAV.filter(([, , role]) => role !== "superadmin" || isSuperadmin()); // изменение 032: раздел «Пользователи» виден только суперадминистратору return `
ipam_manager
${esc(user?.username ?? "")}${btn("Сменить пароль", "pw-change", { cls: "ghost" })}${btn("Выйти", "logout", { cls: "ghost" })}
@@ -218,12 +223,17 @@ screens.orgs = async () => { const all = q ? (await api("/organizations", { params: { limit: 1 } })).total : total; setSelectable(items.map((o) => o.id)); const cols = selCols("minmax(200px,1.5fr) 110px 130px minmax(200px,1.5fr) 80px 80px 44px"); - const rows = items.map((o, i) => `
${selCell(o.id)} + const rows = items.map((o, i) => { + const items2 = [{ label: "Открыть префиксы", value: "open:" + o.id }, { label: "Редактировать", value: "edit:" + o.id }]; // изменение 032 + if (isSuperadmin()) items2.push({ label: "Удалить", value: "del:" + o.id, cls: "danger" }); // изменение 032: кнопка удаления только для superadmin + return `
${selCell(o.id)} ${esc(o.name)}${esc(o.short_name)}${esc(o.inn)} ${esc(o.address)}${o.prefixes_count}${fmtNum(o.addresses_count)} -${iconBtn(I.dots(), "menu", "Действия", `data-menu="org-row-${o.id}"`)}${popMenu("org-row-" + o.id, [{ label: "Открыть префиксы", value: "open:" + o.id }, { label: "Редактировать", value: "edit:" + o.id }, { label: "Удалить", value: "del:" + o.id, cls: "danger" }], "row-pop")}
`).join(""); +${iconBtn(I.dots(), "menu", "Действия", `data-menu="org-row-${o.id}"`)}${popMenu("org-row-" + o.id, items2, "row-pop")}
`; // изменение 032 + }).join(""); S.rows = items; - return shell("orgs", `${header("Организации", `${all} ${plural(all, "организация", "организации", "организаций")}`, btn("Добавить организацию", "org-new", { cls: "primary", icon: I.plus() }))} + const addBtn = isSuperadmin() ? btn("Добавить организацию", "org-new", { cls: "primary", icon: I.plus() }) : ""; // изменение 032: кнопка только для superadmin + return shell("orgs", `${header("Организации", `${all} ${plural(all, "организация", "организации", "организаций")}`, addBtn)}
${searchBox(q, "Поиск: название, ИНН, адрес", 300)}
${bulkBar()}
${selAllCell()}НазваниеКраткое имяИННАдресПрефиксовАдресов
${rows || '
Ничего не найдено
'}
Показано ${items.length} из ${all} ${plural(all, "организации", "организаций", "организаций")}
`); @@ -306,8 +316,9 @@ screens.devices = async () => { ${esc(d.device_type_name)}${ips}${esc(d.note)} ${iconBtn(I.dots(), "menu", "Действия", `data-menu="dev-row-${d.id}"`)}${popMenu("dev-row-" + d.id, [{ label: "Редактировать", value: "edit:" + d.id }, { label: "Удалить", value: "del:" + d.id, cls: "danger" }], "row-pop")}`; }).join(""); - return shell("devices", `${crumbsOrg()}${header("Устройства", `${list.total} ${plural(list.total, "устройство", "устройства", "устройств")} · организация: ${esc(org?.name ?? "")}`, btn("Добавить устройство", "dev-new", { cls: "primary", icon: I.plus() }))} -
${I.gear()}Управление типами
+ const addBtn = btn("Добавить устройство", "dev-new", { cls: "primary", icon: I.plus() }); // изменение 033: устройство создаёт любой admin своей организации + return shell("devices", `${crumbsOrg()}${header("Устройства", `${list.total} ${plural(list.total, "устройство", "устройства", "устройств")} · организация: ${esc(org?.name ?? "")}`, addBtn)} +
${I.gear()}Типы устройств
${orgSwitcher()}${searchBox(q, "Поиск: имя, IP, заметка", 260)}${filterBtn("type", "Тип", typeItems, S.type ?? "", 170)}
${bulkBar(bulkMenu("bulk-type", "Сменить тип", types.items.map((t) => ({ label: t.name, value: t.id }))))}
${selAllCell()}УстройствоТипIP-адресовЗаметки
${rows || '
Устройств нет
'}
Показано ${list.items.length} из ${list.total} ${plural(list.total, "устройства", "устройств", "устройств")}
`); @@ -341,18 +352,23 @@ async function typesDialog() { ? `` : `${esc(t.name)}`; const locked = t.devices_count > 0; - const acts = t.is_default - ? `по умолчанию` - : editing - ? `${iconBtn(I.edit(14), "type-save", "Сохранить", `data-id="${t.id}"`, true)}${iconBtn(I.close(), "type-cancel", "Отмена", "", true)}` - : `${iconBtn(I.edit(14), "type-edit", "Переименовать", `data-id="${t.id}"`, true)}${iconBtn(I.trash(14), "type-del", locked ? "Нельзя удалить: используется устройствами" : "Удалить", `data-id="${t.id}"`, true, locked)}`; + let acts; + if (!isSuperadmin()) { // изменение 032: управление только для superadmin + acts = t.is_default ? `по умолчанию` : ""; + } else { + acts = t.is_default + ? `по умолчанию` + : editing + ? `${iconBtn(I.edit(14), "type-save", "Сохранить", `data-id="${t.id}"`, true)}${iconBtn(I.close(), "type-cancel", "Отмена", "", true)}` + : `${iconBtn(I.edit(14), "type-edit", "Переименовать", `data-id="${t.id}"`, true)}${iconBtn(I.trash(14), "type-del", locked ? "Нельзя удалить: используется устройствами" : "Удалить", `data-id="${t.id}"`, true, locked)}`; + } return `
${name}${t.devices_count}${acts}
`; }).join(""); openDialog({ title: "Управление типами устройств", width: 640, - note: "Типы используются в фильтре и в поле «Тип» при добавлении устройства. Общий список для всех организаций.", + note: "Типы используются в фильтре и в поле «Тип» при добавлении устройства. Общий список для всех организаций.", // изменение 032 body: `
НазваниеУстройств
${rows}
-
`, +${isSuperadmin() ? `
` : ""}`, // изменение 032: кнопка только для superadmin foot: `
${btn("Готово", "close-dialog", { cls: "primary" })}
`, }); } @@ -678,11 +694,11 @@ async function doClear() { // ---- users screens.users = async () => { - if (!isAdmin()) return shell("users", `${header("Пользователи", "Учётные записи UI")}
Раздел доступен только администратору
`); + if (!isSuperadmin()) return shell("users", `${header("Пользователи", "Учётные записи UI")}
Раздел доступен только суперадминистратору
`); // изменение 032 const q = S.q || ""; const { items, total } = await api("/users", { params: { q, limit: 500 } }); S.rows = items; - const cols = selCols("minmax(200px,1.5fr) 190px 190px minmax(140px,1fr) 44px"); + const cols = selCols("minmax(200px,1.5fr) 190px minmax(180px,1fr) 190px minmax(140px,1fr) 44px"); // изменение 032: добавлен столбец для организации setSelectable(items.filter((u) => u.id !== user?.id).map((u) => u.id)); // свою запись выбрать нельзя const rows = items.map((u, n) => { const self = u.id === user?.id; @@ -691,39 +707,46 @@ screens.users = async () => { { label: u.is_active ? "Отключить доступ" : "Разрешить доступ", value: "toggle:" + u.id, cls: u.is_active ? "danger" : "" }, { label: "Удалить", value: "del:" + u.id, cls: "danger" }, ]; + const orgName = orgs.find((o) => o.id === u.organization_id)?.name || "—"; // изменение 032 return `
${selCell(u.id, self)} ${esc(u.username)}${self ? ` это вы` : ""} -${badge(u.role === "admin" ? "blue" : "", ROLE_RU[u.role] || u.role)} +${badge(u.role === "superadmin" ? "purple" : u.role === "admin" ? "blue" : "", ROLE_RU[u.role] || u.role)} +${u.role === "superadmin" ? "—" : esc(orgName)} ${u.is_active ? badge("green", "Доступ разрешён") : badge("red", "Отключён")} ${self ? "свои роль и доступ менять нельзя" : ""} ${iconBtn(I.dots(), "menu", "Действия", `data-menu="user-row-${u.id}"`)}${popMenu("user-row-" + u.id, items2, "row-pop")}
`; }).join(""); - return shell("users", `${header("Пользователи", `${total} ${plural(total, "учётная запись", "учётные записи", "учётных записей")} · роли: администратор — запись, просмотр — чтение`, btn("Добавить пользователя", "users-new", { cls: "primary", icon: I.plus() }))} + return shell("users", `${header("Пользователи", `${total} ${plural(total, "учётная запись", "учётные записи", "учётных записей")} · роли: суперадминистратор — полные права, администратор — запись в организации, просмотр — чтение`, btn("Добавить пользователя", "users-new", { cls: "primary", icon: I.plus() }))}
${searchBox(q, "Поиск по логину", 260)}
-${bulkBar(btn("Разрешить доступ", "bulk-access", { data: 'data-value="1"' }) + btn("Отключить доступ", "bulk-access", { data: 'data-value="0"' }))}
${selAllCell()}ЛогинРольСтатус
+${bulkBar(btn("Разрешить доступ", "bulk-access", { data: 'data-value="1"' }) + btn("Отключить доступ", "bulk-access", { data: 'data-value="0"' }))}
${selAllCell()}ЛогинРольОрганизацияСтатус
${rows || '
Пользователей нет
'}
Показано ${items.length} из ${total} ${plural(total, "записи", "записей", "записей")}
`); }; function userDialog(u) { const edit = !!u; const self = edit && u.id === user?.id; + const roleInit = u?.role ?? "viewer"; // изменение 033: начальная видимость поля организации — по роли в форме S.dialog = async (v) => { if (edit) { const body = { is_active: v.is_active === true }; if (v.role) body.role = v.role; + if (v.role !== "superadmin" && v.organization_id) body.organization_id = Number(v.organization_id); // изменение 033: одинаково с созданием — сервер сам очистит организацию при повышении if (v.password) body.password = v.password; await api(`/users/${u.id}`, { method: "PATCH", body }); } else { - await api("/users", { method: "POST", body: { username: v.username, password: v.password, role: v.role, is_active: v.is_active !== false } }); + const body = { username: v.username, password: v.password, role: v.role, is_active: v.is_active !== false }; // изменение 032 + if (v.role !== "superadmin" && v.organization_id) body.organization_id = Number(v.organization_id); // изменение 032: проверка перед присвоением + await api("/users", { method: "POST", body }); } toast(edit ? "Пользователь сохранён" : "Пользователь добавлен"); await draw(); }; openDialog({ title: edit ? `Пользователь ${u.username}` : "Новый пользователь", - note: "«Администратор» может изменять данные, «Просмотр» — только читать их. Логин после создания не меняется.", + note: "«Суперадминистратор» — полные права, «Администратор» — запись в организации, «Просмотр» — только чтение. Логин после создания не меняется.", // изменение 032 body: formBody(`${edit ? `` : fInput("username", "Логин", { ph: "ivanov", hint: "латиница, цифры, . _ -" })} ${self ? "" : fInput("password", edit ? "Новый пароль" : "Пароль", { type: "password", optional: edit, ph: edit ? "не менять" : "", hint: "минимум 8 символов" })} -${self ? "" : fSelect("role", "Роль", [{ value: "admin", label: "Администратор" }, { value: "viewer", label: "Просмотр" }], u?.role ?? "viewer")} +${self ? "" : fSelect("role", "Роль", [{ value: "superadmin", label: "Суперадминистратор" }, { value: "admin", label: "Администратор" }, { value: "viewer", label: "Просмотр" }], roleInit)} +${self ? "" : `
${fSelect("organization_id", "Организация", orgs.map((o) => ({ value: o.id, label: o.name })), u?.organization_id ?? "")}
`} ${self ? `
Свою учётную запись нельзя понизить, отключить или удалить — пароль меняется кнопкой «Сменить пароль» в шапке.
` : ""}`), foot: dlgFoot(edit ? "Сохранить" : "Добавить пользователя"), @@ -926,6 +949,8 @@ const actions = { closeDropdowns(); box.querySelector(".select").focus(); input.dispatchEvent(new Event("change", { bubbles: true })); + // изменение 033: живое переключение поля «Организация» при смене роли в диалоге пользователя + if (S.dialog && input.name === "role") $("#org-field-wrap")?.toggleAttribute("hidden", d.value === "superadmin"); }, sel: (d, el) => { el.checked ? selSet().add(Number(d.id)) : selSet().delete(Number(d.id)); draw(); }, "sel-all": (d, el) => { S.sel = new Set(el.checked ? S.selectable : []); draw(); }, diff --git a/web/styles.css b/web/styles.css index f0aba81..c050691 100644 --- a/web/styles.css +++ b/web/styles.css @@ -100,6 +100,7 @@ svg{flex:none} .badge.blue{background:#eaf0ff;color:#1e449e} .badge.red{background:#fbe6e6;color:#a32020} .badge.amber{background:#fdf0d5;color:#7c4a00} +.badge.purple{background:#efe8fb;color:#5b2a9e} .more{margin-left:4px;display:inline-flex;align-items:center;height:18px;padding:0 5px;border-radius:4px;background:#eaf0ff;color:#1e449e;font:500 11px/1 var(--sans)} /* overview */