From 13e17fbb472cfee05bf2c9c588a0e9be03c9cce2 Mon Sep 17 00:00:00 2001 From: ayurishchev Date: Sat, 26 Sep 2026 21:33:50 +0300 Subject: [PATCH] =?UTF-8?q?=D0=97=D0=B0=D0=B4=D0=B0=D1=87=D0=B8=20011-024:?= =?UTF-8?q?=20=D0=B4=D0=BE=D1=80=D0=B0=D0=B1=D0=BE=D1=82=D0=BA=D0=B8=20?= =?UTF-8?q?=D0=BF=D0=BE=20=D1=80=D0=B5=D0=B2=D1=8C=D1=8E=20=D0=BA=D0=BE?= =?UTF-8?q?=D0=B4=D0=BE=D0=B2=D0=BE=D0=B9=20=D0=B1=D0=B0=D0=B7=D1=8B=20?= =?UTF-8?q?=D0=B8=20=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD?= =?UTF-8?q?=D0=B8=D0=B5=20=D0=BD=D0=B0=D1=85=D0=BE=D0=B4=D0=BE=D0=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью кодовой базы (docs/reviews/2026-09-26-codebase-review.md) и планы по каждой находке: 011 IP уникален в VRF и хранится в самом узком префиксе (addresses.vrf_id, составной FK с каскадом при переносе VRF, миграция 0007 с остановкой на дублях). 012 Ограничение попыток входа (login_attempts, 429 + Retry-After), выравнивание времени ответа, журнал без вытеснения анонимными событиями (миграция 0006). 013 Границы пагинации: отрицательные/чрезмерные limit/offset дают 422 вместо 500. 014 Экран адресов: страница свободных адресов арифметикой, пагинация в SQL. 015 Запрет адреса сети/broadcast, загрузка не выше 100 %. 016 Роль по умолчанию — viewer. 017 Проверка JWT_SECRET/ADMIN_PASSWORD при старте. 018 null в PATCH очищает текстовые поля; нейтральный текст конфликта БД. 019 Пакетная загрузка в списках вместо N+1. 020 Автоназначение адреса вне вложенных префиксов, с блокировкой префикса. 021 Advisory-lock при снятии прав администратора, уникальный lower(username) (миграция 0008). 022 Контейнер не от root, healthcheck, блокировка миграций, requirements.lock. 023 Экранирование LIKE, журнал отказов очистки, заголовки безопасности, учёт force-удаления, отзыв токенов при смене пароля (claim pv, миграция 0005). 024 Исправление находок ревью 011-023 (docs/reviews/2026-09-26-changes-011-023-review.md): сериализация попыток входа, запрет переноса адресов в адрес сети/broadcast, журнал входов, запрет смены своего пароля через PATCH, валидация PATCH устройства, обновлён тест токенов. Тесты: 14 passed. Документация: README.md, docs/changes/011-024, docs/reviews. Co-Authored-By: Claude Opus 5.5 --- .env.example | 2 + Dockerfile | 8 +- README.md | 48 +++++- alembic/env.py | 15 +- alembic/versions/0005_password_changed_at.py | 20 +++ alembic/versions/0006_login_attempts.py | 29 ++++ alembic/versions/0007_address_vrf_unique.py | 54 ++++++ alembic/versions/0008_users_lower_username.py | 23 +++ app/api/v1/auth.py | 76 ++++++++- app/api/v1/journal.py | 13 +- app/api/v1/prefixes.py | 159 ++++++++++++----- app/api/v1/refs.py | 139 +++++++++------ app/api/v1/users.py | 28 ++- app/config.py | 12 +- app/main.py | 40 ++++- app/models.py | 27 ++- app/rotation.py | 9 +- app/schemas.py | 61 +++++-- app/security.py | 23 ++- app/services.py | 76 +++++++-- docker-compose.yml | 11 +- .../changes/011-address-unique-in-vrf/PLAN.md | 36 ++++ .../011-address-unique-in-vrf/SUMMARY.md | 15 ++ docs/changes/012-login-rate-limit/PLAN.md | 35 ++++ docs/changes/012-login-rate-limit/SUMMARY.md | 16 ++ docs/changes/013-pagination-bounds/PLAN.md | 24 +++ docs/changes/013-pagination-bounds/SUMMARY.md | 9 + .../014-addresses-listing-performance/PLAN.md | 30 ++++ .../SUMMARY.md | 11 ++ .../015-network-broadcast-addresses/PLAN.md | 23 +++ .../SUMMARY.md | 10 ++ docs/changes/016-default-role-viewer/PLAN.md | 22 +++ .../016-default-role-viewer/SUMMARY.md | 8 + .../changes/017-jwt-secret-validation/PLAN.md | 25 +++ .../017-jwt-secret-validation/SUMMARY.md | 10 ++ docs/changes/018-patch-null-handling/PLAN.md | 23 +++ .../018-patch-null-handling/SUMMARY.md | 11 ++ docs/changes/019-n-plus-one-queries/PLAN.md | 28 +++ .../changes/019-n-plus-one-queries/SUMMARY.md | 10 ++ docs/changes/020-next-free-address/PLAN.md | 23 +++ docs/changes/020-next-free-address/SUMMARY.md | 10 ++ docs/changes/021-concurrency-locks/PLAN.md | 24 +++ docs/changes/021-concurrency-locks/SUMMARY.md | 10 ++ docs/changes/022-ops-hardening/PLAN.md | 26 +++ docs/changes/022-ops-hardening/SUMMARY.md | 13 ++ docs/changes/023-minor-hardening/PLAN.md | 28 +++ docs/changes/023-minor-hardening/SUMMARY.md | 14 ++ docs/changes/024-review-fixes-011-023/PLAN.md | 73 ++++++++ .../024-review-fixes-011-023/SUMMARY.md | 67 ++++++++ .../2026-09-26-changes-011-023-review.md | 107 ++++++++++++ docs/reviews/2026-09-26-codebase-review.md | 160 ++++++++++++++++++ requirements-dev.txt | 1 + requirements.lock | 96 +++++++++++ scripts/find_duplicate_addresses.py | 14 ++ scripts/find_unusable_addresses.py | 17 ++ tests/test_users.py | 7 +- web/app.js | 5 +- 57 files changed, 1748 insertions(+), 166 deletions(-) create mode 100644 alembic/versions/0005_password_changed_at.py create mode 100644 alembic/versions/0006_login_attempts.py create mode 100644 alembic/versions/0007_address_vrf_unique.py create mode 100644 alembic/versions/0008_users_lower_username.py create mode 100644 docs/changes/011-address-unique-in-vrf/PLAN.md create mode 100644 docs/changes/011-address-unique-in-vrf/SUMMARY.md create mode 100644 docs/changes/012-login-rate-limit/PLAN.md create mode 100644 docs/changes/012-login-rate-limit/SUMMARY.md create mode 100644 docs/changes/013-pagination-bounds/PLAN.md create mode 100644 docs/changes/013-pagination-bounds/SUMMARY.md create mode 100644 docs/changes/014-addresses-listing-performance/PLAN.md create mode 100644 docs/changes/014-addresses-listing-performance/SUMMARY.md create mode 100644 docs/changes/015-network-broadcast-addresses/PLAN.md create mode 100644 docs/changes/015-network-broadcast-addresses/SUMMARY.md create mode 100644 docs/changes/016-default-role-viewer/PLAN.md create mode 100644 docs/changes/016-default-role-viewer/SUMMARY.md create mode 100644 docs/changes/017-jwt-secret-validation/PLAN.md create mode 100644 docs/changes/017-jwt-secret-validation/SUMMARY.md create mode 100644 docs/changes/018-patch-null-handling/PLAN.md create mode 100644 docs/changes/018-patch-null-handling/SUMMARY.md create mode 100644 docs/changes/019-n-plus-one-queries/PLAN.md create mode 100644 docs/changes/019-n-plus-one-queries/SUMMARY.md create mode 100644 docs/changes/020-next-free-address/PLAN.md create mode 100644 docs/changes/020-next-free-address/SUMMARY.md create mode 100644 docs/changes/021-concurrency-locks/PLAN.md create mode 100644 docs/changes/021-concurrency-locks/SUMMARY.md create mode 100644 docs/changes/022-ops-hardening/PLAN.md create mode 100644 docs/changes/022-ops-hardening/SUMMARY.md create mode 100644 docs/changes/023-minor-hardening/PLAN.md create mode 100644 docs/changes/023-minor-hardening/SUMMARY.md create mode 100644 docs/changes/024-review-fixes-011-023/PLAN.md create mode 100644 docs/changes/024-review-fixes-011-023/SUMMARY.md create mode 100644 docs/reviews/2026-09-26-changes-011-023-review.md create mode 100644 docs/reviews/2026-09-26-codebase-review.md create mode 100644 requirements.lock create mode 100644 scripts/find_duplicate_addresses.py create mode 100644 scripts/find_unusable_addresses.py diff --git a/.env.example b/.env.example index 1b08e9f..76f9a49 100644 --- a/.env.example +++ b/.env.example @@ -2,10 +2,12 @@ POSTGRES_DB=ipam POSTGRES_USER=ipam POSTGRES_PASSWORD=change-me +# JWT_SECRET — не короче 32 символов (иначе приложение не стартует); ADMIN_PASSWORD — не короче 8 JWT_SECRET=change-me ADMIN_USERNAME=admin ADMIN_PASSWORD=change-me APP_PORT=8088 DB_HOST_PORT=55432 +# APP_BIND=127.0.0.1 — публиковать порт приложения только на loopback (за reverse-proxy с TLS); по умолчанию 0.0.0.0 # CIDR доверенных reverse-proxy через запятую (у них берётся X-Forwarded-For); по умолчанию пусто — IP клиента = адрес сокета TRUSTED_PROXIES= diff --git a/Dockerfile b/Dockerfile index d588685..873585f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,14 @@ FROM python:3.11-slim WORKDIR /srv ENV PYTHONUNBUFFERED=1 PIP_NO_CACHE_DIR=1 PYTHONPATH=/srv -COPY requirements.txt . -RUN pip install -r requirements.txt +COPY requirements.txt requirements.lock ./ +RUN pip install -r requirements.lock COPY alembic.ini . COPY alembic alembic COPY app app COPY web web +RUN useradd --system --uid 10001 --no-create-home app +USER app +HEALTHCHECK --interval=15s --timeout=5s --start-period=30s --retries=5 \ + CMD python -c "import sys, urllib.request; sys.exit(0 if urllib.request.urlopen('http://127.0.0.1:8000/healthz', timeout=3).status == 200 else 1)" CMD ["sh", "-c", "alembic upgrade head && uvicorn app.main:app --host 0.0.0.0 --port 8000"] diff --git a/README.md b/README.md index ffafac0..05c2592 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,9 @@ python3 -m venv venv && venv/bin/pip install -r requirements-dev.txt venv/bin/python scripts/seed_demo.py # (по желанию) демо-данные из макетов ``` - UI: http://127.0.0.1:8088/ · Swagger: http://127.0.0.1:8088/docs (порт приложения — `APP_PORT` в `.env`, слушает 0.0.0.0; порт БД — только 127.0.0.1) -- Логин/пароль администратора — `ADMIN_USERNAME` / `ADMIN_PASSWORD` из `.env` (создаётся при первом старте). +- Логин/пароль администратора — `ADMIN_USERNAME` / `ADMIN_PASSWORD` из `.env` (создаётся при первом старте; пароль не короче 8 символов). +- `JWT_SECRET` обязателен и не короче 32 символов (заглушки отклоняются): без него приложение не стартует (изменение 017). `scripts/gen_env.py` генерирует корректные значения. +- Порт приложения публикуется на `${APP_BIND:-0.0.0.0}` (в `.env` можно задать `APP_BIND=127.0.0.1` — только loopback, за reverse-proxy с TLS, см. «Публикация»). ## Архитектура | Слой | Технологии | @@ -24,14 +26,21 @@ venv/bin/python scripts/seed_demo.py # (по желанию) демо-да ``` app/ main.py config.py db.py security.py models.py schemas.py services.py api/v1/{auth,refs,prefixes,overview}.py web/ index.html styles.css app.js -alembic/ scripts/{gen_env,seed_demo}.py tests/ docs/changes/ +alembic/ scripts/{gen_env,seed_demo,find_duplicate_addresses,find_unusable_addresses}.py requirements.lock tests/ docs/changes/ docs/reviews/ ``` ## Модель данных `organizations` → `vrfs` (по организации, «default» создаётся автоматически) → `prefixes` (дерево через `parent_id`, вложенность определяется автоматически) → `addresses`; `devices` + `device_types`; `isps` + `isp_networks`; `users`; `audit_log`. - Ёмкость листового префикса — размер подсети (IPv4 без сетевого/broadcast); родителя — сумма вложенных листьев. -- «Свободные» адреса не хранятся, а вычисляются; в списке адресов они показываются для подсетей до /20. +- «Свободные» адреса не хранятся, а вычисляются; в списке адресов они показываются для подсетей до /20. Страница `status=free` считается арифметически + (без перебора адресов, `offset` ≤ 10 000 000), остальные фильтры и постраничная выдача — в SQL (изменение 014). +- **IP уникален в пределах VRF и хранится в самом узком префиксе** (изменение 011): `addresses.vrf_id` + составной FK `(prefix_id, vrf_id)` (обновляется каскадом при переносе VRF), + уникальность `(vrf_id, address)`. Адрес из диапазона вложенного префикса в родителе назначить нельзя (422), дубль в VRF → 409; новый вложенный префикс забирает адреса родителя + из своего диапазона (`moved_addresses` в журнале); перенос префикса в VRF с тем же адресом → 409. Миграция 0007 останавливается при дублях — найти: `scripts/find_duplicate_addresses.py`. +- Адрес сети и broadcast (IPv4, префикс ≤ /30) назначить нельзя (422, изменение 015); уже внесённые найдёт `scripts/find_unusable_addresses.py`; загрузка не превышает 100 %. + Перенос адресов при создании вложенного префикса или смене VRF, из-за которого адрес стал бы сетевым/broadcast в новом месте, тоже отклоняется 422 с перечислением адресов и целевых + префиксов (изменение 024): создание префикса откатывается целиком, смена VRF — без частичных изменений. - Обзор считает использование только по IPv4. - VRF — часть адресного плана организации: имя уникально **в пределах организации** (без учёта регистра), в разных организациях имена могут совпадать; в одном VRF может быть много префиксов. Принадлежность VRF организации префикса гарантирует составной FK в БД. @@ -45,17 +54,24 @@ alembic/ scripts/{gen_env,seed_demo}.py tests/ docs/changes/ ## API (`/api/v1`) `POST /auth/login` · `GET /auth/me` · `GET /overview` CRUD: `/organizations`, `/vrfs`, `/isps`, `/device-types`, `/devices`, `/prefixes`, `/addresses/{id}`, `/users` -Адреса префикса: `GET|POST /prefixes/{id}/addresses` (`status`, `q`, `limit`, `offset`), `POST …/addresses/next` — автоназначение из пула. +Адреса префикса: `GET|POST /prefixes/{id}/addresses` (`status`, `q`, `limit`, `offset`), `POST …/addresses/next` — автоназначение из пула +(первый свободный адрес вне вложенных префиксов, без адреса сети/broadcast; префикс блокируется на время выдачи — изменение 020). +Пагинация во всех списках: `limit` ≥ 1 (и не больше предела эндпоинта), `offset` от 0 до 10 000 000; иначе 422 (изменение 013). +`PATCH`: `null` в текстовом поле очищает его (пустая строка), `null` в `status` адреса → 422, `device_id: null` отвязывает устройство (изменение 018). +`PATCH /devices/{id}` проверяет `name` (hostname/FQDN) и `mac` (формат, нормализация к `AA:BB:CC:DD:EE:FF`) так же, как создание — раньше PATCH пропускал некорректные значения (изменение 024). +Поиск (`q`) экранирует `%` и `_`. Ответы содержат заголовки `Content-Security-Policy` (кроме `/docs`), `X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy` (изменение 023). Ошибки: `{code, message, fields}` (для блокировок/лимитов — дополнительные поля `attempts_left`, `retry_after_seconds`). Каждое изменение пишется в `audit_log`. ### Пользователи и роли - `GET|POST /users`, `PATCH|DELETE /users/{id}` — управление учётными записями, только `admin`: логин (3–100 символов, латиница, цифры, `. _ -`), роль, доступ и пароль (при сбросе администратором). Логин после создания не меняется — он же `sub` в токене. -- `POST /users/me/password {current_password, new_password}` — смена своего пароля, доступна любой роли; неверный текущий пароль → 403. +- `POST /users/me/password {current_password, new_password}` — смена своего пароля, доступна любой роли; неверный текущий пароль → 403. Ответ содержит новый `access_token`: + прежние токены пользователя после смены пароля (самим или администратором) недействительны (изменение 023, claim `pv`). +- `PATCH /users/{id}` с `password` для **своей же** учётной записи → 422: свой пароль меняется только через `/users/me/password` (там обязательно подтверждение текущего пароля; изменение 024). - Занятый логин (в том числе в другом регистре) → 409; служебные логины `system` и `anonymous`, короткий логин или пароль → 422. - Свою учётную запись нельзя понизить, отключить или удалить, как и последнего активного администратора → 409. -- Отключение действует немедленно (токен проверяется по `users.is_active` на каждом запросе), а смена пароля уже выданные токены - не отзывает — они живут до истечения `JWT_TTL_MINUTES`. Роль `viewer` видит реестр и журнал, но любые изменения получает с 403. +- Отключение действует немедленно (токен проверяется по `users.is_active` на каждом запросе). Роль `viewer` видит реестр и журнал, но любые изменения получает с 403. +- Роль по умолчанию при создании — `viewer` (изменение 016). Логин уникален без учёта регистра и в БД (изменение 021); снятие прав администратора и удаление пользователей идут под advisory-lock. - Изменения пишутся в журнал: `user.created`, `user.updated`, `user.password_reset`, `user.deleted` (значения паролей не сохраняются). ### Журнал @@ -69,7 +85,23 @@ CRUD: `/organizations`, `/vrfs`, `/isps`, `/device-types`, `/devices`, `/prefixe системные события (ротация) — без IP. Фильтр `GET /audit?client_ip=` принимает IP или подсеть (`192.168.5.0/24`), текстовый поиск `q` ищет и по началу IP. IP берётся из адреса сокета. `X-Forwarded-For` учитывается только от прокси из `TRUSTED_PROXIES` (CIDR через запятую в `.env`, по умолчанию пусто) — иначе IP можно подделать. Запросы с самой машины через `127.0.0.1` Docker показывает адресом шлюза сети (`172.x.0.1`); с LAN-адреса и удалённых хостов виден реальный источник. -- В журнал пишутся также входы (`session.login`, `session.failed` — актор `anonymous`) и служебные события (`journal.*`, актор `system`). +- В журнал пишутся также входы (`session.login`, `session.failed` — актор `anonymous`, только первая неудача в окне **по этому логину**; `session.locked` — блокировка, `diff.distinct_logins` — + число разных логинов с этого IP в окне, если сработал лимит по IP) и служебные события (`journal.*`, актор `system`; неверный пароль очистки — `journal.clear_failed` / `journal.clear_locked`). + Удаление префикса с `force=true` фиксирует `addresses_deleted`. +- **Вход:** 5 неудач на логин и 20 на IP за 10 минут → блокировка на 10 минут (429, `Retry-After`, `retry_after_seconds`; изменение 012). Для несуществующего логина время ответа выравнивается. + Пароль ≤ 128 символов. Попытки одного логина сериализованы (`pg_advisory_xact_lock`) — параллельные запросы не обходят лимит (изменение 024). При ротации по количеству первыми удаляются + `session.failed` и `session.locked`. За reverse-proxy без `TRUSTED_PROXIES` все клиенты делят один IP — лимит по IP заденет всех. + +## Публикация и эксплуатация +- Контейнер приложения работает от непривилегированного пользователя (uid 10001), у сервиса есть healthcheck (`/healthz`), сервисы перезапускаются (`restart: unless-stopped`). +- Миграции при старте выполняются под advisory-lock — параллельные реплики не гоняют их одновременно. Зависимости зафиксированы в `requirements.lock` + (обновление: `venv/bin/pip-compile --strip-extras -o requirements.lock requirements.txt`). +- TLS: приложение отдаёт HTTP; для эксплуатации поставьте reverse-proxy (пример для Caddy) и задайте `APP_BIND=127.0.0.1`, `TRUSTED_PROXIES=<адрес прокси/сеть Docker>`: + ``` + ipam.example.com { + reverse_proxy 127.0.0.1:8088 + } + ``` ## Поведение таблиц UI Администратор может выбирать строки чекбоксами (в шапке — «выбрать все») на экранах «Организации», «Операторы», «Устройства», «Префиксы» (листовые), «Адреса» (кроме «Свободен») и «Пользователи» (кроме себя); diff --git a/alembic/env.py b/alembic/env.py index 89c6a7e..ca906fa 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -1,5 +1,5 @@ from alembic import context -from sqlalchemy import engine_from_config, pool +from sqlalchemy import engine_from_config, pool, text from app.config import settings from app.models import Base @@ -7,14 +7,19 @@ from app.models import Base config = context.config config.set_main_option("sqlalchemy.url", settings.database_url.replace("%", "%%")) target_metadata = Base.metadata +MIGRATION_LOCK = 703000 # advisory-lock: одна реплика мигрирует, остальные ждут и видят актуальную схему def run(): engine = engine_from_config(config.get_section(config.config_ini_section), prefix="sqlalchemy.", poolclass=pool.NullPool) - with engine.connect() as conn: - context.configure(connection=conn, target_metadata=target_metadata) - with context.begin_transaction(): - context.run_migrations() + with engine.connect() as lock_conn: # отдельное соединение держит блокировку до конца миграций + lock_conn.execute(text("SELECT pg_advisory_lock(:k)"), {"k": MIGRATION_LOCK}) + with engine.connect() as conn: + context.configure(connection=conn, target_metadata=target_metadata) + with context.begin_transaction(): + context.run_migrations() + lock_conn.execute(text("SELECT pg_advisory_unlock(:k)"), {"k": MIGRATION_LOCK}) + lock_conn.commit() run() diff --git a/alembic/versions/0005_password_changed_at.py b/alembic/versions/0005_password_changed_at.py new file mode 100644 index 0000000..e0c85b8 --- /dev/null +++ b/alembic/versions/0005_password_changed_at.py @@ -0,0 +1,20 @@ +"""users.password_changed_at: токены, выданные до смены пароля, недействительны + +Revision ID: 0005 +Revises: 0004 +""" +from alembic import op +import sqlalchemy as sa + +revision = "0005" +down_revision = "0004" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("users", sa.Column("password_changed_at", sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + op.drop_column("users", "password_changed_at") diff --git a/alembic/versions/0006_login_attempts.py b/alembic/versions/0006_login_attempts.py new file mode 100644 index 0000000..d428ea0 --- /dev/null +++ b/alembic/versions/0006_login_attempts.py @@ -0,0 +1,29 @@ +"""login_attempts: ограничение перебора паролей + +Revision ID: 0006 +Revises: 0005 +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0006" +down_revision = "0005" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "login_attempts", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("ts", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.Column("client_ip", postgresql.INET(), nullable=True), + sa.Column("username", sa.String(100), nullable=False), + ) + op.create_index("ix_login_attempts_ip_ts", "login_attempts", ["client_ip", "ts"]) + op.create_index("ix_login_attempts_user_ts", "login_attempts", ["username", "ts"]) + + +def downgrade() -> None: + op.drop_table("login_attempts") diff --git a/alembic/versions/0007_address_vrf_unique.py b/alembic/versions/0007_address_vrf_unique.py new file mode 100644 index 0000000..4013c6a --- /dev/null +++ b/alembic/versions/0007_address_vrf_unique.py @@ -0,0 +1,54 @@ +"""IP-адрес уникален в пределах VRF и хранится в самом узком префиксе + +Revision ID: 0007 +Revises: 0006 +""" +from alembic import op +import sqlalchemy as sa + +revision = "0007" +down_revision = "0006" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + bind = op.get_bind() + dup = bind.execute(sa.text( + "SELECT p.vrf_id, host(a.address) AS ip, array_agg(p.prefix::text ORDER BY p.id) AS prefixes " + "FROM addresses a JOIN prefixes p ON p.id = a.prefix_id GROUP BY p.vrf_id, host(a.address) HAVING count(*) > 1")).all() + if dup: + lines = "; ".join(f"VRF {r.vrf_id}: {r.ip} в {', '.join(r.prefixes)}" for r in dup[:20]) + raise RuntimeError(f"Один IP записан несколько раз в одном VRF ({len(dup)} шт.): {lines}. Разберите дубли (scripts/find_duplicate_addresses.py) и повторите миграцию") + + op.create_unique_constraint("uq_prefixes_id_vrf_id", "prefixes", ["id", "vrf_id"]) + op.add_column("addresses", sa.Column("vrf_id", sa.Integer(), nullable=True)) + bind.execute(sa.text("UPDATE addresses a SET vrf_id = p.vrf_id FROM prefixes p WHERE p.id = a.prefix_id")) + op.alter_column("addresses", "vrf_id", nullable=False) + # адрес — в самом узком префиксе своего VRF (раньше он мог остаться в родителе) + bind.execute(sa.text( + "UPDATE addresses a SET prefix_id = t.best FROM (" + " SELECT a2.id AS aid, (SELECT p.id FROM prefixes p WHERE p.vrf_id = a2.vrf_id AND p.prefix >>= a2.address " + " ORDER BY masklen(p.prefix) DESC LIMIT 1) AS best FROM addresses a2) t " + "WHERE a.id = t.aid AND t.best IS NOT NULL AND t.best <> a.prefix_id")) + # предупреждение (изменение 024, находка №2): перенос мог сделать адрес адресом сети/broadcast его нового префикса + # (правило появилось позже, в изменении 015); миграцию не останавливаем, только сообщаем — найти такие поможет find_unusable_addresses.py + bad = bind.execute(sa.text( + "SELECT host(a.address) AS ip, p.prefix::text AS cidr FROM addresses a JOIN prefixes p ON p.id = a.prefix_id " + "WHERE family(p.prefix) = 4 AND masklen(p.prefix) <= 30 " + "AND (host(a.address) = host(network(p.prefix)) OR host(a.address) = host(broadcast(p.prefix)))")).all() + if bad: + lines = "; ".join(f"{r.ip} в {r.cidr}" for r in bad[:20]) + print(f"ВНИМАНИЕ: после переноса в самый узкий префикс {len(bad)} адрес(ов) стали адресом сети/broadcast: {lines}" + + (" …" if len(bad) > 20 else "")) + op.drop_constraint("addresses_prefix_id_fkey", "addresses", type_="foreignkey") + op.create_foreign_key("fk_addresses_prefix_vrf", "addresses", "prefixes", ["prefix_id", "vrf_id"], ["id", "vrf_id"], ondelete="CASCADE", onupdate="CASCADE") + op.create_unique_constraint("uq_addresses_vrf_address", "addresses", ["vrf_id", "address"]) + + +def downgrade() -> None: + op.drop_constraint("uq_addresses_vrf_address", "addresses", type_="unique") + op.drop_constraint("fk_addresses_prefix_vrf", "addresses", type_="foreignkey") + op.create_foreign_key("addresses_prefix_id_fkey", "addresses", "prefixes", ["prefix_id"], ["id"], ondelete="CASCADE") + op.drop_column("addresses", "vrf_id") + op.drop_constraint("uq_prefixes_id_vrf_id", "prefixes", type_="unique") diff --git a/alembic/versions/0008_users_lower_username.py b/alembic/versions/0008_users_lower_username.py new file mode 100644 index 0000000..7774fe5 --- /dev/null +++ b/alembic/versions/0008_users_lower_username.py @@ -0,0 +1,23 @@ +"""users: логин уникален без учёта регистра + +Revision ID: 0008 +Revises: 0007 +""" +from alembic import op +import sqlalchemy as sa + +revision = "0008" +down_revision = "0007" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + dup = op.get_bind().execute(sa.text("SELECT lower(username), array_agg(username) FROM users GROUP BY 1 HAVING count(*) > 1")).all() + if dup: + raise RuntimeError(f"Логины, совпадающие без учёта регистра: {[(r[0], r[1]) for r in dup]}. Переименуйте или удалите дубли и повторите миграцию") + op.create_index("uq_users_lower_username", "users", [sa.text("lower(username)")], unique=True) + + +def downgrade() -> None: + op.drop_index("uq_users_lower_username", table_name="users") diff --git a/app/api/v1/auth.py b/app/api/v1/auth.py index 63ae53c..a9d50c4 100644 --- a/app/api/v1/auth.py +++ b/app/api/v1/auth.py @@ -1,23 +1,91 @@ +"""Вход в UI. Перебор паролей ограничен: {MAX_PER_LOGIN} неудач на логин и {MAX_PER_IP} на IP за {WINDOW_MIN} минут → 429 (изменение 012).""" +from datetime import datetime, timedelta, timezone + from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import select +from sqlalchemy import delete, func, select from sqlalchemy.orm import Session from app.db import get_db -from app.models import User +from app.models import LoginAttempt, User +from app.request_context import request_meta from app.schemas import LoginIn, TokenOut, UserOut -from app.security import create_token, current_user, verify_password +from app.security import create_token, current_user, hash_password, verify_password from app.services import ANONYMOUS, audit router = APIRouter(prefix="/auth", tags=["auth"]) +WINDOW = timedelta(minutes=10) +MAX_PER_LOGIN = 5 +MAX_PER_IP = 20 +__doc__ = __doc__.format(MAX_PER_LOGIN=MAX_PER_LOGIN, MAX_PER_IP=MAX_PER_IP, WINDOW_MIN=int(WINDOW.total_seconds() // 60)) +_DUMMY_HASH = hash_password("dummy-password-for-timing") # выравнивает время ответа для несуществующего логина + + +def _condition(login: str, ip: str | None, scope: str): + return (LoginAttempt.username == login) if scope == "login" else (LoginAttempt.client_ip == ip) + + +def _retry_after(db: Session, login: str, ip: str | None) -> tuple[int, str | None, dict[str, int]]: + """(секунд до конца блокировки, причина 'login'|'ip', {"login": n, "ip": m} — число неудач в каждой области, до лимита). + Блокировка длится окно после последней неудачи. Счётчики по областям отдельно (изменение 024, находка №4): + решение о записи в журнал принимается по конкретному логину, а не по максимуму среди логина и IP.""" + now = datetime.now(timezone.utc) + worst, why, counts = 0, None, {} + for scope, limit in (("login", MAX_PER_LOGIN), ("ip", MAX_PER_IP)): + if scope == "ip" and ip is None: + continue + rows = db.scalars(select(LoginAttempt.ts).where(_condition(login, ip, scope), LoginAttempt.ts > now - WINDOW).order_by(LoginAttempt.ts.desc()).limit(limit)).all() + counts[scope] = len(rows) + if len(rows) >= limit: + left = int((rows[0] + WINDOW - now).total_seconds()) + 1 + if left > worst: + worst, why = left, scope + return worst, why, counts + + +def _distinct_logins(db: Session, ip: str) -> int: + """Число различных логинов, для которых была неудачная попытка с этого IP в окне (для diff записи session.locked).""" + now = datetime.now(timezone.utc) + return db.scalar(select(func.count(func.distinct(LoginAttempt.username))).where(LoginAttempt.client_ip == ip, LoginAttempt.ts > now - WINDOW)) or 0 + + +def _locked(retry: int) -> HTTPException: + return HTTPException(429, {"message": f"Слишком много неудачных попыток входа. Повторите через {max(1, -(-retry // 60))} мин.", "retry_after_seconds": retry}, + headers={"Retry-After": str(retry)}) @router.post("/login", response_model=TokenOut) def login(body: LoginIn, db: Session = Depends(get_db)): + ctx = request_meta.get() + ip = ctx["client_ip"] if ctx else None + name = body.username.strip().lower() + # сериализация попыток одного логина (изменение 024, находка №1): без этого параллельные запросы + # проходят проверку блокировки одновременно, и лимит «N за окно» превращается в «N + степень параллелизма» + db.execute(select(func.pg_advisory_xact_lock(func.hashtext(name)))) + retry, _, _ = _retry_after(db, name, ip) + if retry: # блокировка: пароль не проверяем, в журнал не пишем (запись о блокировке уже есть) + raise _locked(retry) user = db.scalar(select(User).where(User.username == body.username, User.is_active)) + if user is None: + verify_password(body.password, _DUMMY_HASH) if user is None or not verify_password(body.password, user.password_hash): - audit(db, ANONYMOUS, "session", None, "failed", body.username[:100], message="Неудачная попытка входа в UI") + _, _, before = _retry_after(db, name, ip) + db.add(LoginAttempt(client_ip=ip, username=name)) + db.flush() + retry, why, after = _retry_after(db, name, ip) + if retry and before[why] < after[why]: # именно этот запрос впервые пересёк лимит — запись пишем один раз + attempts = after[why] + diff = {"scope": why, "attempts": attempts, "retry_after_seconds": retry} + if why == "ip": + diff["distinct_logins"] = _distinct_logins(db, ip) + audit(db, ANONYMOUS, "session", None, "locked", body.username[:100], diff, + message=f"Вход заблокирован на {max(1, -(-retry // 60))} мин.: {attempts} неудачных попыток ({'по логину' if why == 'login' else 'с IP'})") + elif before["login"] == 0: # в журнал — только первая неудача по этому логину в окне (не по IP: иначе перебор логинов с одного IP её не оставит) + audit(db, ANONYMOUS, "session", None, "failed", body.username[:100], message="Неудачная попытка входа в UI") db.commit() + if retry: + raise _locked(retry) raise HTTPException(401, "Неверный логин или пароль") + db.execute(delete(LoginAttempt).where(LoginAttempt.username == name)) audit(db, user, "session", None, "login", user.username, message=f"Вход в UI: {user.username}") db.commit() return TokenOut(access_token=create_token(user)) diff --git a/app/api/v1/journal.py b/app/api/v1/journal.py index 28f3492..e35c76c 100644 --- a/app/api/v1/journal.py +++ b/app/api/v1/journal.py @@ -13,15 +13,14 @@ 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 audit, commit +from app.services import MAX_OFFSET, audit, commit, like_escape router = APIRouter(dependencies=[Depends(current_user)], tags=["journal"]) MAX_ATTEMPTS = 5 LOCK_MINUTES = 10 -def _escape_like(q: str) -> str: - return q.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") +_escape_like = like_escape def _filtered(stmt, event_type: str, entity_type: str, actor: str, date_from: date | None, date_to: date | None, q: str, client_ip: str = ""): @@ -57,7 +56,7 @@ 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, le=500), offset: int = 0, 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), ): stmt = _filtered(select(AuditLog), event_type, entity_type, actor, date_from, date_to, q, client_ip) total = db.scalar(select(func.count()).select_from(stmt.subquery())) or 0 @@ -153,13 +152,19 @@ def _lock_state(db: Session, user_id: int) -> tuple[int, int]: def clear_journal(body: ClearIn, db: Session = Depends(get_db), user: User = Depends(admin_user)): used, retry = _lock_state(db, user.id) if retry: + audit(db, user, "journal", None, "clear_locked", "clear", {"retry_after_seconds": retry}, message="Очистка журнала: попытка во время блокировки") + db.commit() raise HTTPException(429, {"message": "Слишком много неверных попыток", "retry_after_seconds": retry}) if not verify_password(body.password, user.password_hash): db.add(ClearAttempt(user_id=user.id)) db.commit() used, retry = _lock_state(db, user.id) if retry: + audit(db, user, "journal", None, "clear_locked", "clear", {"retry_after_seconds": retry}, message="Очистка журнала: неверный пароль, доступ заблокирован") + db.commit() raise HTTPException(429, {"message": "Слишком много неверных попыток", "retry_after_seconds": retry}) + audit(db, user, "journal", None, "clear_failed", "clear", {"attempts_left": MAX_ATTEMPTS - used}, message="Очистка журнала: неверный пароль") + db.commit() raise HTTPException(403, {"message": "Неверный пароль", "attempts_left": MAX_ATTEMPTS - used}) deleted = db.execute(delete(AuditLog)).rowcount db.execute(delete(ClearAttempt).where(ClearAttempt.user_id == user.id)) diff --git a/app/api/v1/prefixes.py b/app/api/v1/prefixes.py index 4002117..5e859ec 100644 --- a/app/api/v1/prefixes.py +++ b/app/api/v1/prefixes.py @@ -1,7 +1,7 @@ import ipaddress from fastapi import APIRouter, Depends, HTTPException, Query -from sqlalchemy import String, and_, cast, func, or_, select, update +from sqlalchemy import String, and_, cast, func, or_, select, text, update from sqlalchemy.orm import Session from app import schemas as s @@ -9,7 +9,7 @@ from app.db import get_db from app.models import Address, AddressStatus, Device, Organization, Prefix, PrefixStatus, User, Vrf from app.security import admin_user, current_user from app.services import ( - MAX_CAPACITY, apply_update, audit, blockers, capacity, commit, count, flush, get_or_404, next_free, next_free_subnet, refuse_delete, + MAX_CAPACITY, 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, ) @@ -72,6 +72,7 @@ def _prefix_outs(db: Session, rows: list[Prefix]) -> list[s.PrefixOut]: usage = _usage(db, [r.id for r in rows]) depths: dict[int, int] = {} caps: dict[int, int] = {} + vrf_names = dict(db.execute(select(Vrf.id, Vrf.name).where(Vrf.id.in_(list({r.vrf_id for r in rows})))).all()) if rows else {} for org_id in {r.organization_id for r in rows}: depths.update(_depths(db, org_id)) caps.update(_capacities(db, org_id)) @@ -80,7 +81,7 @@ def _prefix_outs(db: Session, rows: list[Prefix]) -> list[s.PrefixOut]: used, stored = usage.get(r.id, (0, 0)) cap = caps.get(r.id, capacity(str(r.prefix))) out.append(s.PrefixOut( - id=r.id, organization_id=r.organization_id, vrf_id=r.vrf_id, vrf_name=r.vrf.name, + id=r.id, organization_id=r.organization_id, vrf_id=r.vrf_id, vrf_name=vrf_names[r.vrf_id], prefix=str(r.prefix), family=ipaddress.ip_network(str(r.prefix)).version, description=r.description, status=r.status, parent_id=r.parent_id, depth=depths.get(r.id, 0), is_pool=r.is_pool, note=r.note, used=used, capacity=cap, @@ -111,6 +112,40 @@ def attach_to_tree(db: Session, p: Prefix, keep_parent: bool = False, exclude: f c.parent_id = p.id +def _narrowest_prefix(db: Session, vrf_id: int, ip: str) -> Prefix | None: + """Самый узкий префикс VRF, содержащий адрес.""" + return db.scalar(select(Prefix).where(Prefix.vrf_id == vrf_id, Prefix.prefix.op(">>=")(ip)).order_by(func.masklen(Prefix.prefix).desc()).limit(1)) + + +def _unusable_after_rehome(db: Session, p: Prefix) -> list[tuple[str, str]]: + """(адрес, CIDR целевого префикса) для адресов VRF префикса p (его диапазон), которые при переносе в самый узкий + целевой префикс (как в rehome_addresses) окажутся его адресом сети или broadcast — такой перенос запрещён + (изменение 024, находка №2). Целевой префикс — не обязательно p: при переносе VRF им может оказаться уже существующий + вложенный префикс целевого VRF, поэтому в паре возвращается именно он, а не p.""" + db.flush() + rows = db.execute(text( + "SELECT host(a2.address) AS ip, (SELECT x.prefix FROM prefixes x WHERE x.vrf_id = a2.vrf_id AND x.prefix >>= a2.address " + " ORDER BY masklen(x.prefix) DESC LIMIT 1) AS cidr " + "FROM addresses a2 WHERE a2.vrf_id = :vrf AND a2.address <<= :cidr"), {"vrf": p.vrf_id, "cidr": str(p.prefix)}).all() + return [(ip, cidr) for ip, cidr in rows if network_role(ipaddress.ip_network(cidr), ipaddress.ip_address(ip))] + + +def _unusable_message(bad: list[tuple[str, str]]) -> str: + items = ", ".join(f"{ip} ({cidr})" for ip, cidr in bad) + return f"Адреса {items} станут адресом сети/broadcast: освободите их или выберите другой префикс" + + +def rehome_addresses(db: Session, p: Prefix) -> int: + """Приводит адреса диапазона префикса к правилу «адрес — в самом узком префиксе VRF» + (новый вложенный забирает адреса родителя из своего диапазона; перенесённый префикс — адреса целевого VRF). Возвращает число перенесённых.""" + db.flush() + return db.execute(text( + "UPDATE addresses a SET prefix_id = t.best FROM (" + " SELECT a2.id AS aid, (SELECT x.id FROM prefixes x WHERE x.vrf_id = a2.vrf_id AND x.prefix >>= a2.address ORDER BY masklen(x.prefix) DESC LIMIT 1) AS best " + " FROM addresses a2 WHERE a2.vrf_id = :vrf AND a2.address <<= :cidr) t " + "WHERE a.id = t.aid AND t.best IS NOT NULL AND t.best <> a.prefix_id"), {"vrf": p.vrf_id, "cidr": str(p.prefix)}).rowcount + + def _subtree(db: Session, root: Prefix) -> list[Prefix]: """Префикс и все вложенные по цепочке parent_id.""" result, frontier = [root], [root.id] @@ -130,12 +165,25 @@ def _move_to_vrf(db: Session, p: Prefix, vrf_id: int) -> tuple[dict, int]: taken = db.scalars(select(Prefix.prefix).where(Prefix.vrf_id == vrf.id, Prefix.prefix.in_([str(m.prefix) for m in subtree]))).all() if taken: raise HTTPException(409, f"В VRF «{vrf.name}» уже есть: {', '.join(str(x) for x in taken)}") + clash = db.scalars( + select(func.host(Address.address)).where( + Address.prefix_id.in_([m.id for m in subtree]), + Address.address.in_(select(Address.address).where(Address.vrf_id == vrf.id)), + ).limit(10) + ).all() + if clash: + raise HTTPException(409, f"В VRF «{vrf.name}» уже назначены адреса: {', '.join(clash)}") old_name = p.vrf.name for m in subtree: m.vrf = vrf p.parent_id = None db.flush() + db.expire_all() # vrf_id адресов обновлён каскадом БД attach_to_tree(db, p, exclude=frozenset(m.id for m in subtree)) + bad = _unusable_after_rehome(db, p) + if bad: # commit ещё не выполнялся — исключение уходит без частичных изменений (rollback при закрытии сессии) + raise HTTPException(422, _unusable_message(bad)) + rehome_addresses(db, p) return {"vrf": f"{old_name} → {vrf.name}", "moved": len(subtree)}, len(subtree) @@ -143,7 +191,7 @@ 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, le=1000), offset: int = 0, 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), ): stmt = select(Prefix) if organization_id: @@ -155,7 +203,7 @@ def list_prefixes( if family: stmt = stmt.where(func.family(Prefix.prefix) == (4 if family == 4 else 6)) if q: - stmt = stmt.where(or_(cast(Prefix.prefix, String).ilike(f"%{q.strip()}%"), Prefix.description.ilike(f"%{q.strip()}%"))) + stmt = stmt.where(or_(contains(cast(Prefix.prefix, String), q), contains(Prefix.description, q))) total = count(db, stmt) rows = db.scalars(stmt.order_by(Prefix.vrf_id, Prefix.prefix).limit(limit).offset(offset)).all() return s.Page(items=_prefix_outs(db, list(rows)), total=total) @@ -184,22 +232,34 @@ def create_prefix(body: s.PrefixIn, db: Session = Depends(get_db), user: User = db.add(p) flush(db, "Такой префикс уже есть в этом VRF") attach_to_tree(db, p, keep_parent=parent_id is not None) - audit(db, user, "prefix", p, "created", str(p.prefix), {"vrf": vrf.name}) + bad = _unusable_after_rehome(db, p) + if bad: + msg = _unusable_message(bad) # до rollback: после него объект p истекает + 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 {})}) commit(db, "Такой префикс уже есть в этом VRF") return _prefix_outs(db, [p])[0] +def _busy_ranges(db: Session, p: Prefix) -> list[tuple[int, int]]: + """Занятые диапазоны внутри префикса: вложенные префиксы того же VRF (любой глубины) и адреса, записанные в самом префиксе.""" + busy = [] + for c in db.scalars(select(Prefix.prefix).where(Prefix.vrf_id == p.vrf_id, Prefix.id != p.id, Prefix.prefix.op("<<")(str(p.prefix)))): + n = ipaddress.ip_network(str(c)) + busy.append((int(n.network_address), int(n.broadcast_address))) + busy += [(int(ipaddress.ip_address(a)),) * 2 for a in db.scalars(select(func.host(Address.address)).where(Address.prefix_id == p.id))] + return busy + + def _find_subnet(db: Session, parent: Prefix, length: int) -> tuple[str | None, int, int]: """(свободный блок | None, min длина, max длина) для вложенного префикса в parent.""" net = ipaddress.ip_network(str(parent.prefix)) lo, hi = net.prefixlen + 1, net.max_prefixlen if not lo <= length <= hi: raise HTTPException(422, f"Размер вложенного префикса: от /{lo} до /{hi}" if lo <= hi else "Префикс нельзя дробить: это одиночный адрес") - busy = [(int(n.network_address), int(n.broadcast_address)) for n in - (ipaddress.ip_network(str(c)) for c in db.scalars( - select(Prefix.prefix).where(Prefix.vrf_id == parent.vrf_id, Prefix.id != parent.id, Prefix.prefix.op("<<")(str(parent.prefix)))))] - busy += [(int(ipaddress.ip_address(a)),) * 2 for a in db.scalars(select(func.host(Address.address)).where(Address.prefix_id == parent.id))] - return next_free_subnet(str(parent.prefix), length, busy), lo, hi + return next_free_subnet(str(parent.prefix), length, _busy_ranges(db, parent)), lo, hi @router.get("/prefixes/{id}/subnets/next", response_model=s.SubnetPreview) @@ -223,7 +283,8 @@ def allocate_subnet(id: int, body: s.SubnetNextIn, db: Session = Depends(get_db) db.add(p) flush(db, "Такой префикс уже есть в этом VRF, повторите запрос") attach_to_tree(db, p, keep_parent=True) - audit(db, user, "prefix", p, "created", found, {"vrf": parent.vrf.name, "allocated_from": str(parent.prefix)}) + 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 {})}) commit(db, "Такой префикс уже есть в этом VRF, повторите запрос") return _prefix_outs(db, [p])[0] @@ -248,7 +309,8 @@ def delete_prefix(id: int, force: bool = False, db: Session = Depends(get_db), u if used: refuse_delete(db, user, "prefix", p, str(p.prefix), "В префиксе есть адреса; удалите их или используйте force=true", {"addresses": used}) db.execute(update(Prefix).where(Prefix.parent_id == id).values(parent_id=p.parent_id)) - audit(db, user, "prefix", p, "deleted", str(p.prefix)) + 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) db.delete(p) commit(db) @@ -271,10 +333,11 @@ 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, le=500), offset: int = 0, + 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), ): - """status: assigned | reserved | deprecated | free | пусто (все; свободные подмешиваются для малых подсетей).""" + """status: assigned | reserved | deprecated | free | пусто (все; свободные подмешиваются для малых подсетей). + Пагинация — в SQL; страница «свободных» считается арифметически (без перебора адресов подсети).""" p = get_or_404(db, Prefix, id, "Префикс") cap = capacity(str(p.prefix)) counts = dict(db.execute(select(Address.status, func.count()).where(Address.prefix_id == id).group_by(Address.status)).all()) @@ -285,39 +348,47 @@ def list_addresses( ) if status and status not in {"free", *(x.value for x in AddressStatus)}: raise HTTPException(422, "Неизвестный статус") - - stmt = select(Address, Device.name).outerjoin(Device, Device.id == Address.device_id).where(Address.prefix_id == id) - if status and status != "free": - stmt = stmt.where(Address.status == AddressStatus(status)) - if q: - like = f"%{q.strip()}%" - stmt = stmt.where(or_(func.host(Address.address).ilike(like), Address.dns_name.ilike(like), Address.description.ilike(like))) - rows = [_addr_out(a, dn) for a, dn in db.execute(stmt.order_by(Address.address)).all()] if status != "free" else [] - net = ipaddress.ip_network(str(p.prefix)) - want_free = status == "free" or (not status and not q and cap <= FREE_LISTING_LIMIT) - if want_free: - used = {ipaddress.ip_address(s.ip_text(a)) for a in db.scalars(select(Address.address).where(Address.prefix_id == id))} - hosts = net.hosts() if net.version == 4 and net.prefixlen <= 30 else iter(net) - need = offset + limit if status == "free" else cap - free = [] - for ip in hosts: - if ip not in used: - free.append(s.AddressOut(id=None, prefix_id=id, address=str(ip), status="free")) - if len(free) >= need: - break - rows = sorted(rows + free, key=lambda r: ipaddress.ip_address(r.address)) - total = summary.free if status == "free" else (len(rows) if want_free or q or status else stored) - return s.AddressPage(items=rows[offset:offset + limit], total=total, summary=summary) + + def occupied() -> list[int]: + return sorted(int(ipaddress.ip_address(s.ip_text(a))) for a in db.scalars(select(Address.address).where(Address.prefix_id == id))) + + def free_rows(ips: list[str]) -> list[s.AddressOut]: + return [s.AddressOut(id=None, prefix_id=id, address=ip, status="free") for ip in ips] + + if status == "free": + return s.AddressPage(items=free_rows(free_page(net, occupied(), offset, limit)), total=summary.free, summary=summary) + + flt = [Address.prefix_id == id] + if status: + flt.append(Address.status == AddressStatus(status)) + if q: + flt.append(or_(contains(func.host(Address.address), q), contains(Address.dns_name, q), contains(Address.description, q))) + stmt = select(Address, Device.name).outerjoin(Device, Device.id == Address.device_id).where(*flt) + mixed = not status and not q and cap <= FREE_LISTING_LIMIT # малая подсеть: занятые и свободные вперемешку (список ограничен размером подсети) + if mixed: + rows = [_addr_out(a, dn) for a, dn in db.execute(stmt.order_by(Address.address)).all()] + rows = sorted(rows + free_rows(free_page(net, occupied(), 0, cap)), key=lambda r: ipaddress.ip_address(r.address)) + return s.AddressPage(items=rows[offset:offset + limit], total=len(rows), summary=summary) + total = count(db, select(Address.id).where(*flt)) + page = db.execute(stmt.order_by(Address.address).limit(limit).offset(offset)).all() + return s.AddressPage(items=[_addr_out(a, dn) for a, dn in page], total=total, summary=summary) @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 = get_or_404(db, Prefix, id, "Префикс") - if ipaddress.ip_address(body.address) not in ipaddress.ip_network(str(p.prefix)): + 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}") + role = network_role(net, ip) + if role: + raise HTTPException(422, f"Адрес {body.address} — {'адрес сети' if role == 'network' else 'broadcast'} префикса {p.prefix}, назначить его нельзя") + 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) - a = Address(prefix_id=id, **body.model_dump()) + 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) @@ -331,15 +402,17 @@ def allocate_next( id: int, body: s.AddressUpdate | None = None, db: Session = Depends(get_db), user: User = Depends(admin_user) ): """Автоназначение первого свободного адреса; только для префиксов с флагом is_pool.""" - p = get_or_404(db, Prefix, id, "Префикс") + p = db.scalar(select(Prefix).where(Prefix.id == id).with_for_update()) # параллельные запросы получают разные адреса + if p is None: + raise HTTPException(404, "Префикс не найден") if not p.is_pool: raise HTTPException(422, "Префикс не является пулом для автоназначения") - ip = next_free(db, id, str(p.prefix)) + 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")) - a = Address(prefix_id=id, address=ip, **data) + 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) diff --git a/app/api/v1/refs.py b/app/api/v1/refs.py index 61bc08a..cb92e39 100644 --- a/app/api/v1/refs.py +++ b/app/api/v1/refs.py @@ -1,7 +1,7 @@ """Справочники: организации, VRF, операторы, типы устройств, устройства.""" from fastapi import APIRouter, Depends, Query from sqlalchemy import String, cast, delete, func, or_, select -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, selectinload from app import schemas as s from app.db import get_db @@ -9,34 +9,41 @@ 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 apply_update, audit, blockers, commit, count, flush, get_or_404, refuse_delete +from app.services import MAX_OFFSET, apply_update, audit, blockers, commit, contains, count, flush, get_or_404, refuse_delete from fastapi import HTTPException router = APIRouter(dependencies=[Depends(current_user)]) -def _like(q: str) -> str: - return f"%{q.strip()}%" - - # ---------------------------------------------------------------- organizations -def _org_out(db: Session, o: Organization) -> s.OrgOut: - out = s.OrgOut.model_validate(o) - out.prefixes_count = count(db, select(Prefix.id).where(Prefix.organization_id == o.id)) - out.addresses_count = count( - db, select(Address.id).join(Prefix).where(Prefix.organization_id == o.id, Address.status == AddressStatus.assigned) - ) +def _org_outs(db: Session, rows: list[Organization]) -> list[s.OrgOut]: + """Счётчики одним GROUP BY на страницу (без запросов на каждую строку).""" + ids = [o.id for o in rows] + prefixes = dict(db.execute(select(Prefix.organization_id, func.count()).where(Prefix.organization_id.in_(ids)).group_by(Prefix.organization_id)).all()) if ids else {} + addresses = dict(db.execute( + select(Prefix.organization_id, func.count(Address.id)).join(Prefix, Prefix.id == Address.prefix_id) + .where(Prefix.organization_id.in_(ids), Address.status == AddressStatus.assigned).group_by(Prefix.organization_id) + ).all()) if ids else {} + out = [] + for o in rows: + item = s.OrgOut.model_validate(o) + item.prefixes_count, item.addresses_count = prefixes.get(o.id, 0), addresses.get(o.id, 0) + out.append(item) return out +def _org_out(db: Session, o: Organization) -> s.OrgOut: + return _org_outs(db, [o])[0] + + @router.get("/organizations", response_model=s.Page[s.OrgOut], tags=["organizations"]) -def list_orgs(q: str = "", limit: int = Query(100, le=500), offset: int = 0, 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)): stmt = select(Organization) if q: - stmt = stmt.where(or_(*(c.ilike(_like(q)) for c in (Organization.name, Organization.short_name, Organization.inn, Organization.address)))) + stmt = stmt.where(or_(*(contains(c, q) for c in (Organization.name, Organization.short_name, Organization.inn, Organization.address)))) total = count(db, stmt) rows = db.scalars(stmt.order_by(Organization.id).limit(limit).offset(offset)).all() - return s.Page(items=[_org_out(db, o) for o in rows], total=total) + return s.Page(items=_org_outs(db, list(rows)), total=total) @router.get("/organizations/{id}", response_model=s.OrgOut, tags=["organizations"]) @@ -82,19 +89,28 @@ def delete_org(id: int, db: Session = Depends(get_db), user: User = Depends(admi # ------------------------------------------------------------------------- VRF -def _vrf_out(db: Session, v: Vrf) -> s.VrfOut: - out = s.VrfOut.model_validate(v) - out.prefixes_count = count(db, select(Prefix.id).where(Prefix.vrf_id == v.id)) +def _vrf_outs(db: Session, rows: list[Vrf]) -> list[s.VrfOut]: + ids = [v.id for v in rows] + counts = dict(db.execute(select(Prefix.vrf_id, func.count()).where(Prefix.vrf_id.in_(ids)).group_by(Prefix.vrf_id)).all()) if ids else {} + out = [] + for v in rows: + item = s.VrfOut.model_validate(v) + item.prefixes_count = counts.get(v.id, 0) + out.append(item) return out +def _vrf_out(db: Session, v: Vrf) -> s.VrfOut: + return _vrf_outs(db, [v])[0] + + @router.get("/vrfs", response_model=s.Page[s.VrfOut], tags=["vrf"]) def list_vrfs(organization_id: int | None = None, db: Session = Depends(get_db)): stmt = select(Vrf) if organization_id: stmt = stmt.where(Vrf.organization_id == organization_id) rows = db.scalars(stmt.order_by(Vrf.id)).all() - return s.Page(items=[_vrf_out(db, v) for v in rows], total=len(rows)) + return s.Page(items=_vrf_outs(db, list(rows)), total=len(rows)) @router.post("/vrfs", response_model=s.VrfOut, status_code=201, tags=["vrf"]) @@ -129,16 +145,25 @@ def delete_vrf(id: int, db: Session = Depends(get_db), user: User = Depends(admi # ---------------------------------------------------------------- device types -def _type_out(db: Session, t: DeviceType) -> s.DeviceTypeOut: - out = s.DeviceTypeOut.model_validate(t) - out.devices_count = count(db, select(Device.id).where(Device.device_type_id == t.id)) +def _type_outs(db: Session, rows: list[DeviceType]) -> list[s.DeviceTypeOut]: + 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 {} + out = [] + for t in rows: + item = s.DeviceTypeOut.model_validate(t) + item.devices_count = counts.get(t.id, 0) + out.append(item) return out +def _type_out(db: Session, t: DeviceType) -> s.DeviceTypeOut: + return _type_outs(db, [t])[0] + + @router.get("/device-types", response_model=s.Page[s.DeviceTypeOut], tags=["devices"]) def list_types(db: Session = Depends(get_db)): rows = db.scalars(select(DeviceType).order_by(DeviceType.id)).all() - return s.Page(items=[_type_out(db, t) for t in rows], total=len(rows)) + return s.Page(items=_type_outs(db, list(rows)), total=len(rows)) @router.post("/device-types", response_model=s.DeviceTypeOut, status_code=201, tags=["devices"]) @@ -174,23 +199,36 @@ def delete_type(id: int, db: Session = Depends(get_db), user: User = Depends(adm # --------------------------------------------------------------------- devices +def _device_outs(db: Session, devices: list[Device]) -> list[s.DeviceOut]: + """Адреса и названия типов — двумя запросами на страницу.""" + ids = [d.id for d in devices] + by_device: dict[int, list] = {} + if ids: + for dev_id, addr, prefix_id, status in db.execute( + select(Address.device_id, Address.address, Address.prefix_id, Address.status).where(Address.device_id.in_(ids)).order_by(Address.address) + ): + by_device.setdefault(dev_id, []).append((addr, prefix_id, status)) + type_names = dict(db.execute(select(DeviceType.id, DeviceType.name)).all()) + out = [] + for d in devices: + rows = by_device.get(d.id, []) + out.append(s.DeviceOut( + id=d.id, name=d.name, device_type_id=d.device_type_id, device_type_name=type_names[d.device_type_id], + organization_id=d.organization_id, mac=d.mac, note=d.note, + ip_addresses=[s.ip_text(r[0]) for r in rows], first_prefix_id=rows[0][1] if rows else None, + all_deprecated=bool(rows) and all(r[2] == AddressStatus.deprecated for r in rows), + )) + return out + + def _device_out(db: Session, d: Device) -> s.DeviceOut: - rows = db.execute( - select(Address.address, Address.prefix_id, Address.status).where(Address.device_id == d.id).order_by(Address.address) - ).all() - t = db.get(DeviceType, d.device_type_id) - return s.DeviceOut( - id=d.id, name=d.name, device_type_id=d.device_type_id, device_type_name=t.name, - organization_id=d.organization_id, mac=d.mac, note=d.note, - ip_addresses=[s.ip_text(r[0]) for r in rows], first_prefix_id=rows[0][1] if rows else None, - all_deprecated=bool(rows) and all(r[2] == AddressStatus.deprecated for r in rows), - ) + return _device_outs(db, [d])[0] @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, le=500), offset: int = 0, 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), ): stmt = select(Device) if organization_id: @@ -198,11 +236,11 @@ def list_devices( if device_type_id: stmt = stmt.where(Device.device_type_id == device_type_id) if q: - ip_match = select(Address.device_id).where(func.host(Address.address).ilike(_like(q))) - stmt = stmt.where(or_(Device.name.ilike(_like(q)), Device.note.ilike(_like(q)), Device.id.in_(ip_match))) + ip_match = select(Address.device_id).where(contains(func.host(Address.address), q)) + stmt = stmt.where(or_(contains(Device.name, q), contains(Device.note, q), Device.id.in_(ip_match))) total = count(db, stmt) rows = db.scalars(stmt.order_by(Device.id).limit(limit).offset(offset)).all() - return s.Page(items=[_device_out(db, d) for d in rows], total=total) + return s.Page(items=_device_outs(db, list(rows)), total=total) @router.post("/devices", response_model=s.DeviceOut, status_code=201, tags=["devices"]) @@ -243,30 +281,35 @@ def delete_device(id: int, db: Session = Depends(get_db), user: User = Depends(a # ------------------------------------------------------------------------ ISPs -def _isp_out(db: Session, i: Isp) -> s.IspOut: - org = db.get(Organization, i.organization_id) - return s.IspOut( - id=i.id, name=i.name, organization_id=i.organization_id, organization_name=org.name, +def _isp_outs(db: Session, rows: list[Isp]) -> list[s.IspOut]: + ids = {i.organization_id for i in rows} + names = dict(db.execute(select(Organization.id, Organization.name).where(Organization.id.in_(ids))).all()) if ids else {} + return [s.IspOut( + id=i.id, name=i.name, organization_id=i.organization_id, organization_name=names[i.organization_id], networks=[str(n.cidr) for n in i.networks], hotline=i.hotline, contract_number=i.contract_number, note=i.note, - ) + ) for i in rows] + + +def _isp_out(db: Session, i: Isp) -> s.IspOut: + return _isp_outs(db, [i])[0] @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, le=500), offset: int = 0, + 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), ): stmt = select(Isp) if organization_id: stmt = stmt.where(Isp.organization_id == organization_id) if q: - nets = select(IspNetwork.isp_id).where(cast(IspNetwork.cidr, String).ilike(_like(q))) - orgs = select(Organization.id).where(Organization.name.ilike(_like(q))) - stmt = stmt.where(or_(Isp.name.ilike(_like(q)), Isp.id.in_(nets), Isp.organization_id.in_(orgs))) + nets = select(IspNetwork.isp_id).where(contains(cast(IspNetwork.cidr, String), q)) + orgs = select(Organization.id).where(contains(Organization.name, q)) + stmt = stmt.where(or_(contains(Isp.name, q), Isp.id.in_(nets), Isp.organization_id.in_(orgs))) total = count(db, stmt) - rows = db.scalars(stmt.order_by(Isp.id).limit(limit).offset(offset)).all() - return s.Page(items=[_isp_out(db, i) for i in rows], total=total) + rows = db.scalars(stmt.options(selectinload(Isp.networks)).order_by(Isp.id).limit(limit).offset(offset)).all() + return s.Page(items=_isp_outs(db, list(rows)), total=total) @router.post("/isps", response_model=s.IspOut, status_code=201, tags=["isps"]) diff --git a/app/api/v1/users.py b/app/api/v1/users.py index db8d0f6..60c9991 100644 --- a/app/api/v1/users.py +++ b/app/api/v1/users.py @@ -4,6 +4,8 @@ свою учётную запись и последнего активного администратора; зарезервированные логины `system` и `anonymous` запрещены — журнал различает по ним служебные события (`actor_of` в app/services.py). """ +from datetime import datetime, timezone + from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy import func, select from sqlalchemy.orm import Session @@ -11,12 +13,19 @@ 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, current_user, hash_password, verify_password -from app.services import apply_update, audit, commit, count, flush, get_or_404, refuse_delete +from app.security import admin_user, create_token, current_user, hash_password, 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: изменения прав администраторов идут по одному (иначе двое отключат друг друга одновременно) + + +def _lock_admins(db: Session) -> None: + db.execute(select(func.pg_advisory_xact_lock(USERS_ADMIN_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)) @@ -24,12 +33,12 @@ def _other_active_admins(db: Session, user_id: int) -> int: @router.get("/users", response_model=s.Page[s.UserOut]) def list_users( - q: str = "", limit: int = Query(100, le=500), offset: int = 0, + 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), ): stmt = select(User) if q.strip(): - stmt = stmt.where(User.username.ilike(f"%{q.strip()}%")) + stmt = stmt.where(contains(User.username, q)) total = count(db, stmt) rows = db.scalars(stmt.order_by(User.username).limit(limit).offset(offset)).all() return s.Page(items=[s.UserOut.model_validate(u) for u in rows], total=total) @@ -50,8 +59,12 @@ def create_user(body: s.UserIn, db: Session = Depends(get_db), admin: User = Dep @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)): - u = get_or_404(db, User, id, "Пользователь") data = body.model_dump(exclude_unset=True, exclude_none=True) + if "role" in data or "is_active" in data: + _lock_admins(db) # до чтения пользователя и подсчёта администраторов + u = get_or_404(db, User, id, "Пользователь") + if u.id == admin.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) @@ -62,6 +75,7 @@ def update_user(id: int, body: s.UserUpdate, db: Session = Depends(get_db), admi 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) if pwd: @@ -72,6 +86,7 @@ def update_user(id: int, body: s.UserUpdate, db: Session = Depends(get_db), admi @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) u = get_or_404(db, User, id, "Пользователь") if u.id == admin.id: refuse_delete(db, admin, "user", u, u.username, "Нельзя удалить свою учётную запись") @@ -88,6 +103,7 @@ def change_own_password(body: s.PasswordChange, db: Session = Depends(get_db), u if not verify_password(body.current_password, user.password_hash): raise HTTPException(403, "Неверный текущий пароль") user.password_hash = hash_password(body.new_password) + user.password_changed_at = datetime.now(timezone.utc) audit(db, user, "user", user, "password_reset", user.username, message=f"{user.username}: пароль изменён пользователем") commit(db) - return {"ok": True} + return {"ok": True, "access_token": create_token(user)} # прежние токены недействительны — текущая сессия получает новый diff --git a/app/config.py b/app/config.py index bc16bb7..30c4fa7 100644 --- a/app/config.py +++ b/app/config.py @@ -5,7 +5,7 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") database_url: str = "postgresql+psycopg://ipam:ipam@localhost:55432/ipam" - jwt_secret: str = "dev-only-secret" + jwt_secret: str = "" # обязателен: проверяется при старте приложения (validate_secrets), миграциям не нужен jwt_ttl_minutes: int = 480 admin_username: str = "admin" admin_password: str = "" @@ -13,3 +13,13 @@ class Settings(BaseSettings): settings = Settings() + +_WEAK_SECRETS = {"change-me", "changeme", "dev-only-secret", "secret", "password"} + + +def validate_secrets(s: Settings = settings) -> None: + """Fail-fast при старте: подпись токенов известным/коротким ключом позволяет подделать токен любого пользователя.""" + if len(s.jwt_secret) < 32 or s.jwt_secret.lower() in _WEAK_SECRETS: + raise RuntimeError("JWT_SECRET не задан, короче 32 символов или является заглушкой: сгенерируйте значение (python scripts/gen_env.py)") + if s.admin_password and len(s.admin_password) < 8: + raise RuntimeError("ADMIN_PASSWORD короче 8 символов: пароль слабее, чем допускает UI") diff --git a/app/main.py b/app/main.py index 6865546..5fab2b4 100644 --- a/app/main.py +++ b/app/main.py @@ -1,4 +1,5 @@ import asyncio +import logging from contextlib import asynccontextmanager from pathlib import Path @@ -10,20 +11,26 @@ from sqlalchemy import select from sqlalchemy.exc import IntegrityError from app.api.v1 import auth, journal, overview, prefixes, refs, users -from app.config import settings +from app.config import settings, validate_secrets from app.db import SessionLocal from app.models import DeviceType, Role, User from app.request_context import RequestContextMiddleware from app.rotation import rotation_loop from app.security import hash_password +validate_secrets() # приложение не стартует с небезопасной конфигурацией (изменение 017) + +log = logging.getLogger("ipam") DEFAULT_TYPES = ["Сервер", "Сетевое оборудование", "Сетевое хранилище", "Рабочая станция", "Другое"] def seed(): with SessionLocal() as db: - if not db.scalar(select(User.id).limit(1)) and settings.admin_password: - db.add(User(username=settings.admin_username, password_hash=hash_password(settings.admin_password), role=Role.admin)) + 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)) + else: + log.warning("В БД нет пользователей и ADMIN_PASSWORD не задан: администратор не создан, войти в UI нельзя") if not db.scalar(select(DeviceType.id).limit(1)): db.add_all(DeviceType(name=n, is_default=(n == "Другое")) for n in DEFAULT_TYPES) db.commit() @@ -37,8 +44,35 @@ async def lifespan(_: FastAPI): task.cancel() +CSP = "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self'; frame-ancestors 'none'" +DOCS_PATHS = ("/docs", "/redoc", "/openapi.json") # Swagger UI грузит ресурсы с CDN — CSP для него не ставим + + +class SecurityHeadersMiddleware: + """ASGI-middleware: заголовки безопасности на все ответы (изменение 023).""" + + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if scope["type"] != "http": + return await self.app(scope, receive, send) + docs = scope["path"].startswith(DOCS_PATHS) + + async def send_with_headers(message): + if message["type"] == "http.response.start": + extra = [(b"x-content-type-options", b"nosniff"), (b"referrer-policy", b"no-referrer")] + if not docs: + extra += [(b"content-security-policy", CSP.encode()), (b"x-frame-options", b"DENY")] + message["headers"] = [*message.get("headers", []), *extra] + await send(message) + + await self.app(scope, receive, send_with_headers) + + app = FastAPI(title="IPAM Manager API", version="1.0.0", lifespan=lifespan) app.add_middleware(RequestContextMiddleware) +app.add_middleware(SecurityHeadersMiddleware) api = APIRouter(prefix="/api/v1") for r in (auth.router, overview.router, refs.router, prefixes.router, journal.router, users.router): diff --git a/app/models.py b/app/models.py index 51568da..537f414 100644 --- a/app/models.py +++ b/app/models.py @@ -62,6 +62,7 @@ class Prefix(Base): __tablename__ = "prefixes" __table_args__ = ( UniqueConstraint("vrf_id", "prefix"), + UniqueConstraint("id", "vrf_id", name="uq_prefixes_id_vrf_id"), # цель составного FK адресов ForeignKeyConstraint(["vrf_id", "organization_id"], ["vrfs.id", "vrfs.organization_id"], name="fk_prefixes_vrf_org"), ) id: Mapped[int] = mapped_column(primary_key=True) @@ -116,9 +117,15 @@ class Device(Base): class Address(Base): __tablename__ = "addresses" - __table_args__ = (UniqueConstraint("prefix_id", "address"),) + # vrf_id дублирует VRF префикса (составной FK, обновляется каскадом при переносе): так БД гарантирует уникальность IP в VRF + __table_args__ = ( + UniqueConstraint("prefix_id", "address"), + UniqueConstraint("vrf_id", "address", name="uq_addresses_vrf_address"), + ForeignKeyConstraint(["prefix_id", "vrf_id"], ["prefixes.id", "prefixes.vrf_id"], name="fk_addresses_prefix_vrf", ondelete="CASCADE", onupdate="CASCADE"), + ) id: Mapped[int] = mapped_column(primary_key=True) - prefix_id: Mapped[int] = mapped_column(ForeignKey("prefixes.id", ondelete="CASCADE"), index=True) + prefix_id: Mapped[int] = mapped_column(index=True) + vrf_id: Mapped[int] = mapped_column() address: Mapped[str] = mapped_column(INET) status: Mapped[AddressStatus] = mapped_column(Enum(AddressStatus, name="address_status"), default=AddressStatus.assigned) dns_name: Mapped[str] = mapped_column(String(255), default="") @@ -133,8 +140,12 @@ class User(Base): 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.admin) + role: Mapped[Role] = mapped_column(Enum(Role, name="user_role"), default=Role.viewer) is_active: Mapped[bool] = mapped_column(Boolean, default=True) + password_changed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) # токены с iat раньше — недействительны + + +Index("uq_users_lower_username", func.lower(User.username), unique=True) # логин уникален без учёта регистра class AuditLog(Base): @@ -161,6 +172,16 @@ class AppSetting(Base): value: Mapped[dict] = mapped_column(JSONB) +class LoginAttempt(Base): + """Неудачные попытки входа (для ограничения перебора паролей); записи старше суток удаляет ротация.""" + __tablename__ = "login_attempts" + id: Mapped[int] = mapped_column(primary_key=True) + ts: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + client_ip: Mapped[str | None] = mapped_column(INET) + username: Mapped[str] = mapped_column(String(100)) # в нижнем регистре + __table_args__ = (Index("ix_login_attempts_ip_ts", "client_ip", "ts"), Index("ix_login_attempts_user_ts", "username", "ts")) + + class ClearAttempt(Base): """Неудачные попытки подтверждения пароля при очистке журнала (для блокировки).""" __tablename__ = "clear_attempts" diff --git a/app/rotation.py b/app/rotation.py index 1a0eaca..1d4078d 100644 --- a/app/rotation.py +++ b/app/rotation.py @@ -7,7 +7,7 @@ from sqlalchemy import delete, func, select from sqlalchemy.orm import Session from app.db import SessionLocal -from app.models import AppSetting, AuditLog +from app.models import AppSetting, AuditLog, LoginAttempt from app.services import SYSTEM, audit log = logging.getLogger("ipam.rotation") @@ -44,7 +44,12 @@ def rotate(db: Session) -> dict | None: if total > cfg["max_entries"]: # удаляем с запасом в одну запись — под сводную запись о ротации, чтобы итог не превышал лимит n = total - cfg["max_entries"] + 1 - by_count = db.execute(delete(AuditLog).where(AuditLog.id.in_(select(AuditLog.id).order_by(AuditLog.id).limit(n)))).rowcount + # сначала неудачные и заблокированные входы (их может нагенерировать кто угодно), затем самые старые записи остальных типов (изменение 024, находка №4) + noisy = db.execute(delete(AuditLog).where(AuditLog.id.in_( + select(AuditLog.id).where(AuditLog.entity_type == "session", AuditLog.action.in_(("failed", "locked"))).order_by(AuditLog.id).limit(n)))).rowcount + rest = n - noisy + by_count = noisy + (db.execute(delete(AuditLog).where(AuditLog.id.in_(select(AuditLog.id).order_by(AuditLog.id).limit(rest)))).rowcount if rest > 0 else 0) + db.execute(delete(LoginAttempt).where(LoginAttempt.ts < datetime.now(timezone.utc) - timedelta(days=1))) if by_age or by_count: parts = [] if by_age: diff --git a/app/schemas.py b/app/schemas.py index 90ff0b1..dd96306 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -30,6 +30,22 @@ _FQDN = re.compile(r"^(?=.{1,253}$)([A-Za-z0-9_]([A-Za-z0-9_-]{0,61}[A-Za-z0-9_] _MAC = re.compile(r"^([0-9A-Fa-f]{2}[:-]){5}[0-9A-Fa-f]{2}$") +def _device_name(v: str | None) -> str | None: + """Имя устройства (hostname/FQDN); общая для DeviceIn и DeviceUpdate (изменение 024, находка №6). None — поле не задано в PATCH.""" + if v is not None and not _FQDN.match(v): + raise ValueError("Некорректное имя устройства (hostname)") + return v + + +def _device_mac(v: str | None) -> str | None: + """MAC: формат и нормализация к AA:BB:CC:DD:EE:FF; общая для DeviceIn и DeviceUpdate. Пустая строка допустима (mac не указан).""" + if v is None: + return v + if v and not _MAC.match(v): + raise ValueError("Некорректный MAC-адрес (AA:BB:CC:DD:EE:FF)") + return v.upper().replace("-", ":") + + class Page(BaseModel, Generic[T]): items: list[T] total: int @@ -41,8 +57,8 @@ class ORM(BaseModel): # --- auth class LoginIn(BaseModel): - username: str - password: str + username: str = Field(max_length=100) + password: str = Field(max_length=128) class TokenOut(BaseModel): @@ -76,7 +92,7 @@ class UserOut(ORM): class UserIn(BaseModel): username: Login password: str = Field(min_length=8, max_length=128) - role: Role = Role.admin + role: Role = Role.viewer # наименьшие права по умолчанию (изменение 016) is_active: bool = True @@ -129,11 +145,18 @@ class VrfIn(BaseModel): note: str = "" +def _blank(v): + """null в PATCH текстового поля = «очистить» (в колонке NOT NULL хранится пустая строка).""" + return "" if v is None else v + + class VrfUpdate(BaseModel): name: str | None = Field(None, min_length=1, max_length=100) route_target: str | None = Field(None, max_length=50, pattern=r"^(\d+:\d+)?$") note: str | None = None + _blank_text = field_validator("route_target", "note")(_blank) + class VrfOut(ORM): id: int @@ -163,19 +186,8 @@ class DeviceIn(BaseModel): mac: str = "" note: str = "" - @field_validator("name") - @classmethod - def _name(cls, v): - if not _FQDN.match(v): - raise ValueError("Некорректное имя устройства (hostname)") - return v - - @field_validator("mac") - @classmethod - def _mac(cls, v): - if v and not _MAC.match(v): - raise ValueError("Некорректный MAC-адрес (AA:BB:CC:DD:EE:FF)") - return v.upper().replace("-", ":") + _name_valid = field_validator("name")(_device_name) + _mac_valid = field_validator("mac")(_device_mac) class DeviceUpdate(BaseModel): @@ -184,6 +196,10 @@ class DeviceUpdate(BaseModel): mac: str | None = None note: str | None = None + _blank_text = field_validator("mac", "note")(_blank) # сначала null → "", затем проверка формата + _name_valid = field_validator("name")(_device_name) + _mac_valid = field_validator("mac")(_device_mac) + class DeviceOut(ORM): id: int @@ -252,6 +268,8 @@ class PrefixUpdate(BaseModel): is_pool: bool | None = None note: str | None = None + _blank_text = field_validator("description", "note")(_blank) + class PrefixOut(ORM): id: int @@ -293,9 +311,18 @@ class AddressUpdate(BaseModel): status: AddressStatus | None = None dns_name: str | None = None description: str | None = Field(None, max_length=500) - device_id: int | None = None + device_id: int | None = None # null — отвязать устройство note: str | None = None + _blank_text = field_validator("dns_name", "description", "note")(_blank) + + @field_validator("status") + @classmethod + def _status_required(cls, v): + if v is None: + raise ValueError("Статус нельзя очистить") + return v + class AddressOut(BaseModel): id: int | None diff --git a/app/security.py b/app/security.py index 6a42e44..e357192 100644 --- a/app/security.py +++ b/app/security.py @@ -27,9 +27,21 @@ def verify_password(password: str, hashed: str) -> bool: return False +_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) + + +def _password_version(user: User) -> int: + """Версия пароля в токене: смена пароля меняет её, и выданные ранее токены перестают действовать (без зависимости от точности часов). + Целочисленно (изменение 024, находка №7): без float, хотя результат и был детерминирован.""" + changed = user.password_changed_at + return (changed - _EPOCH) // timedelta(microseconds=1) if changed else 0 + + def create_token(user: User) -> str: - exp = datetime.now(timezone.utc) + timedelta(minutes=settings.jwt_ttl_minutes) - return jwt.encode({"sub": user.username, "exp": exp}, settings.jwt_secret, algorithm="HS256") + now = datetime.now(timezone.utc) + exp = now + timedelta(minutes=settings.jwt_ttl_minutes) + # iat — информационное поле (не проверяется при разборе токена); отзыв выданных токенов работает по pv (версия пароля) + return jwt.encode({"sub": user.username, "iat": now, "exp": exp, "pv": _password_version(user)}, settings.jwt_secret, algorithm="HS256") def current_user( @@ -38,12 +50,15 @@ def current_user( if cred is None: raise HTTPException(401, "Требуется авторизация") try: - username = jwt.decode(cred.credentials, settings.jwt_secret, algorithms=["HS256"])["sub"] - except jwt.PyJWTError: + payload = jwt.decode(cred.credentials, settings.jwt_secret, algorithms=["HS256"]) + username = payload["sub"] + except (jwt.PyJWTError, KeyError): raise HTTPException(401, "Недействительный токен") user = db.scalar(select(User).where(User.username == username, User.is_active)) if user is None: raise HTTPException(401, "Пользователь не найден") + if payload.get("pv", 0) != _password_version(user): + raise HTTPException(401, "Пароль изменён, войдите заново") # токен выдан до смены пароля return user diff --git a/app/services.py b/app/services.py index 5854e67..e5fcb5f 100644 --- a/app/services.py +++ b/app/services.py @@ -10,6 +10,7 @@ from app.request_context import request_meta from app.models import Address, AuditLog, Base MAX_CAPACITY = 2**53 - 1 +MAX_OFFSET = 10_000_000 # верхняя граница offset во всех списках SYSTEM = SimpleNamespace(username="system") @@ -70,7 +71,10 @@ def refuse_delete(db: Session, user, entity_type: str, entity, label: str, reaso raise HTTPException(409, reason) -def commit(db: Session, conflict_msg: str = "Запись с такими значениями уже существует"): +CONFLICT_MSG = "Конфликт с существующими данными: проверьте уникальность значений и связанные объекты" + + +def commit(db: Session, conflict_msg: str = CONFLICT_MSG): try: db.commit() except IntegrityError: @@ -78,7 +82,7 @@ def commit(db: Session, conflict_msg: str = "Запись с такими зна raise HTTPException(409, conflict_msg) -def flush(db: Session, conflict_msg: str = "Запись с такими значениями уже существует"): +def flush(db: Session, conflict_msg: str = CONFLICT_MSG): """flush с тем же переводом нарушений уникальности в 409, что и commit.""" try: db.flush() @@ -103,7 +107,17 @@ def capacity(prefix: str) -> int: def utilization(used: int, cap: int) -> int: - return round(used * 100 / cap) if cap else 0 + return min(round(used * 100 / cap), 100) if cap else 0 # не больше 100 %, даже если в подсети остались адреса сети/broadcast + + +def network_role(net, ip) -> str | None: + """"network" / "broadcast" для IPv4 с длиной префикса ≤ 30 (как в capacity()), иначе None.""" + if net.version == 4 and net.prefixlen <= 30: + if ip == net.network_address: + return "network" + if ip == net.broadcast_address: + return "broadcast" + return None def apply_update(obj, data: dict) -> dict: @@ -115,6 +129,15 @@ def apply_update(obj, data: dict) -> dict: return changed +def like_escape(q: str) -> str: + return q.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + + +def contains(column, q: str): + """column ILIKE '%q%' с экранированием % и _ (иначе поиск «%» возвращает всё).""" + return column.ilike(f"%{like_escape(q.strip())}%", escape="\\") + + def count(db: Session, stmt) -> int: return db.scalar(select(func.count()).select_from(stmt.subquery())) or 0 @@ -137,11 +160,42 @@ def next_free_subnet(parent: str, length: int, occupied: list[tuple[int, int]]) return str(ipaddress.ip_network((type(net.network_address)(cand), length))) -def next_free(db: Session, prefix_id: int, prefix: str) -> str | None: - net = ipaddress.ip_network(prefix) - used = {ipaddress.ip_address(a) for a in db.scalars(select(Address.address).where(Address.prefix_id == prefix_id))} - hosts = net.hosts() if net.version == 4 and net.prefixlen <= 30 else iter(net) - for ip in hosts: - if ip not in used: - return str(ip) - return None +def free_page(net, occupied: list[int], offset: int, limit: int) -> list[str]: + """Страница свободных адресов сети: `occupied` — отсортированные занятые адреса (int). Пропуск `offset` — арифметикой по промежуткам, + без перебора адресов (безопасно для IPv6). Для IPv4 с длиной ≤ 30 адреса сети и broadcast не считаются.""" + first, last = int(net.network_address), int(net.broadcast_address) + if net.version == 4 and net.prefixlen <= 30: + first, last = first + 1, last - 1 + cls, out, skip, cur = type(net.network_address), [], offset, first + for ip in [*occupied, last + 1]: + if ip < cur: + continue + gap_end = min(ip - 1, last) + gap = gap_end - cur + 1 + if gap > 0: + if skip >= gap: + skip -= gap + else: + start = cur + skip + skip = 0 + out.extend(str(cls(a)) for a in range(start, min(gap_end, start + (limit - len(out)) - 1) + 1)) + if len(out) >= limit: + break + cur = max(cur, ip + 1) + return out + + +def next_free_address(net, occupied: list[tuple[int, int]]) -> str | None: + """Первый свободный адрес сети; `occupied` — занятые диапазоны (включительно), без перебора адресов. + Для IPv4 с длиной ≤ 30 адреса сети и broadcast не выдаются (как в capacity()).""" + first, last = int(net.network_address), int(net.broadcast_address) + if net.version == 4 and net.prefixlen <= 30: + first, last = first + 1, last - 1 + cand = first + for start, end in sorted(occupied): + if end < cand: + continue + if start > cand: + break + cand = end + 1 + return str(type(net.network_address)(cand)) if cand <= last else None diff --git a/docker-compose.yml b/docker-compose.yml index 316104c..769eece 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,6 +5,7 @@ services: POSTGRES_DB: ${POSTGRES_DB} POSTGRES_USER: ${POSTGRES_USER} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} + restart: unless-stopped volumes: [pgdata:/var/lib/postgresql/data] ports: ["127.0.0.1:${DB_HOST_PORT}:5432"] healthcheck: @@ -13,6 +14,7 @@ services: retries: 20 app: build: . + restart: unless-stopped depends_on: db: {condition: service_healthy} environment: @@ -21,6 +23,13 @@ services: ADMIN_USERNAME: ${ADMIN_USERNAME} ADMIN_PASSWORD: ${ADMIN_PASSWORD} TRUSTED_PROXIES: ${TRUSTED_PROXIES:-} - ports: ["0.0.0.0:${APP_PORT}:8000"] + # APP_BIND: 0.0.0.0 — доступ из LAN (по умолчанию, как раньше); 127.0.0.1 — только через reverse-proxy с TLS (см. README «Публикация») + ports: ["${APP_BIND:-0.0.0.0}:${APP_PORT}:8000"] + healthcheck: + test: ["CMD", "python", "-c", "import sys, urllib.request; sys.exit(0 if urllib.request.urlopen('http://127.0.0.1:8000/healthz', timeout=3).status == 200 else 1)"] + interval: 15s + timeout: 5s + start_period: 30s + retries: 5 volumes: pgdata: diff --git a/docs/changes/011-address-unique-in-vrf/PLAN.md b/docs/changes/011-address-unique-in-vrf/PLAN.md new file mode 100644 index 0000000..abde318 --- /dev/null +++ b/docs/changes/011-address-unique-in-vrf/PLAN.md @@ -0,0 +1,36 @@ +# Уникальность IP-адреса в VRF (изменение 011) + +Находка ревью № 1 (`docs/reviews/2026-09-26-codebase-review.md`), серьёзность — высокая. + +## Context +Один и тот же IP можно завести и в родительском, и в дочернем префиксе одного VRF: уникальна только пара `(prefix_id, address)`. +Воспроизведено: `198.51.100.5` создаётся в `/24` и во вложенном `/25` (оба 201), у родителя `used` завышен. Нужен инвариант: +**адрес хранится только в самом узком префиксе своего VRF и уникален в VRF**, причём на уровне БД. + +## Решение +1. **Схема** (новая миграция Alembic): + - `prefixes`: уникальное ограничение `(id, vrf_id)` — цель составного FK; + - `addresses`: колонка `vrf_id` (NOT NULL, заполняется из `prefixes`), составной FK `(prefix_id, vrf_id) → prefixes(id, vrf_id)` **ON UPDATE CASCADE** + (перенос префикса в другой VRF сам обновит адреса), уникальный индекс `(vrf_id, address)`. + - Перед созданием индекса миграция ищет дубли и при их наличии **останавливается** с перечнем `VRF · адрес · префиксы` (данные не удаляются автоматически). + Для разбора — скрипт `scripts/find_duplicate_addresses.py` (только чтение). +2. **API** (`app/api/v1/prefixes.py`): + - `create_address`: если адрес попадает в дочерний префикс того же VRF — 422 «Адрес принадлежит вложенному префиксу X, назначьте его там»; дубль в VRF — 409 (через `flush`). + - `attach_to_tree` (создание префикса, `allocate_subnet`, перенос VRF): адреса родителя из диапазона нового префикса переносятся в него + (`UPDATE addresses SET prefix_id = :new WHERE prefix_id = :parent AND address <<= :cidr`); число перенесённых — в `diff` записи `prefix.created` (`moved_addresses`). + - `_move_to_vrf`: конфликт адресов в целевом VRF — 409 с перечнем (как для префиксов), без частичных изменений. +3. **Модель** `Address`: `vrf_id` + ограничения из п. 1; `Address(...)` заполняет `vrf_id` из префикса. +4. `_usage` не меняется: после инварианта двойного счёта нет. + +## Файлы +`alembic/versions/_address_vrf_unique.py`, `app/models.py`, `app/api/v1/prefixes.py`, `scripts/find_duplicate_addresses.py`, `tests/test_api.py`, `README.md` (Модель данных). + +## Тест (один сценарий) +Родитель `/24` с адресом `.5`, создание дочернего `/25` → адрес переехал в дочерний; повторный `.5` в родителе → 422; `.5` в дочернем → 409; перенос дочернего в VRF с `.5` → 409. + +## Проверка +- Миграция на копии данных стенда `ipam_control_006`: без дублей — проходит; с искусственным дублем — останавливается с понятным сообщением. +- `pytest -q`; вручную в UI: «Назначить адрес» в родителе на адрес из дочернего — понятная ошибка. + +## Риски +Изменение модели данных; на рабочей БД до миграции запустить скрипт поиска дублей. Зависимость: № 020 (автоназначение) опирается на этот инвариант. diff --git a/docs/changes/011-address-unique-in-vrf/SUMMARY.md b/docs/changes/011-address-unique-in-vrf/SUMMARY.md new file mode 100644 index 0000000..f6d5db9 --- /dev/null +++ b/docs/changes/011-address-unique-in-vrf/SUMMARY.md @@ -0,0 +1,15 @@ +# Итог: уникальность IP-адреса в VRF (изменение 011) + +## Что сделано +- **Миграция `0007`:** `prefixes` — уникальность `(id, vrf_id)`; `addresses.vrf_id` (заполнен из префиксов), составной FK `(prefix_id, vrf_id)` → `prefixes` (`ON DELETE CASCADE`, `ON UPDATE CASCADE` — при переносе префикса в другой VRF адреса следуют автоматически), уникальность `(vrf_id, address)`. + Перед изменениями миграция ищет дубли и **останавливается** с перечнем; найденные ранее в родителе адреса, попадающие в дочерний префикс, переносятся в самый узкий префикс. `scripts/find_duplicate_addresses.py` — поиск дублей (только чтение). +- **API (`prefixes.py`):** `create_address` — адрес из диапазона вложенного префикса → 422 «назначьте его там»; дубль в VRF → 409; `rehome_addresses()` — создание префикса, `allocate_subnet` и перенос VRF забирают адреса родителя из своего диапазона (число — `moved_addresses` в журнале); + перенос префикса в VRF с тем же адресом → 409 с перечнем (без частичных изменений). +- `app/models.py`: `Address.vrf_id`, ограничения; `README.md`: раздел «Модель данных». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. .5 в родителе → 201; создание дочернего /25 переносит .5; повторный .5 в родителе → 422, в дочернем → 409; перенос префикса в VRF с тем же адресом → 409; миграция на стенде (без дублей) прошла. +Не проверено: остановка миграции на реальном дубле, перенос префикса без конфликта (rehome при смене VRF). + +## Риски +Изменение модели данных. Перед применением на рабочей БД — запустить `scripts/find_duplicate_addresses.py` и сделать резервную копию. diff --git a/docs/changes/012-login-rate-limit/PLAN.md b/docs/changes/012-login-rate-limit/PLAN.md new file mode 100644 index 0000000..9e4a18a --- /dev/null +++ b/docs/changes/012-login-rate-limit/PLAN.md @@ -0,0 +1,35 @@ +# Ограничение попыток входа и защита журнала от вытеснения (изменение 012) + +Находка ревью № 2, серьёзность — высокая. + +## Context +`POST /auth/login` не ограничен: возможен перебор паролей без авторизации. Каждая неудача пишет `session.failed` в журнал, а ротация по количеству +(`max_entries`, 100 000) удаляет самые старые записи — анонимный клиент может вытеснить историю изменений. Дополнительно: при несуществующем логине argon2 не +вызывается (разница во времени ответа раскрывает существование логина), у пароля нет ограничения длины. + +## Решение +1. **Учёт попыток** — таблица `login_attempts (id, ts, client_ip INET, username)` (новая миграция), по образцу `ClearAttempt`; индексы по `(client_ip, ts)` и `(lower(username), ts)`. +2. **Лимиты** (константы в `app/api/v1/auth.py`, при необходимости — в `Settings`): + - по логину: 5 неудач за 10 минут → блокировка входа для этого логина на 10 минут; + - по IP: 20 неудач за 10 минут → блокировка IP на 10 минут. + Ответ при блокировке — 429 `{"message", "retry_after_seconds"}` + заголовок `Retry-After`; верный пароль во время блокировки тоже отклоняется. Успешный вход очищает счётчик логина. +3. **Время ответа:** для несуществующего логина — проверка по фиктивному argon2-хэшу (вычисляется один раз при старте). `LoginIn`: `username` ≤ 100, `password` ≤ 128 символов. +4. **Журнал без вытеснения:** + - `session.failed` пишется только для первой неудачи логина/IP в окне; при срабатывании блокировки — одна запись `session.locked` (`diff`: логин, IP, число попыток, до какого времени); + - `rotate()` при превышении `max_entries` сначала удаляет самые старые `session.failed`, затем остальное — события изменений данных уходят последними. +5. **Очистка** `login_attempts` старше суток — в том же часовом цикле ротации. +6. **UI:** на экране входа — текст 429 с временем до разблокировки (уже выводится `err.message`). + +## Файлы +`alembic/versions/_login_attempts.py`, `app/models.py`, `app/api/v1/auth.py`, `app/schemas.py`, `app/rotation.py`, `web/app.js` (при необходимости), `tests/test_journal.py` или новый `tests/test_auth.py`, `README.md`. + +## Тест (один сценарий) +6 неверных паролей для временного пользователя → на 6-й 429 с `retry_after_seconds`; верный пароль тоже 429; в журнале одна `session.failed` и одна `session.locked`. +Очистка блокировки в тесте — прямым SQL (фикстура `db`). + +## Проверка +`pytest -q`; вручную: серия неверных входов в UI → сообщение о блокировке; через 10 минут вход работает. + +## Риски +Блокировка по логину позволяет «запереть» чужую учётную запись перебором — поэтому окно короткое (10 минут) и событие видно в журнале. +Если стенд за reverse-proxy, без `TRUSTED_PROXIES` все клиенты делят один IP — лимит по IP заденет всех; отметить в README. diff --git a/docs/changes/012-login-rate-limit/SUMMARY.md b/docs/changes/012-login-rate-limit/SUMMARY.md new file mode 100644 index 0000000..15fd11d --- /dev/null +++ b/docs/changes/012-login-rate-limit/SUMMARY.md @@ -0,0 +1,16 @@ +# Итог: ограничение попыток входа (изменение 012) + +## Что сделано +- Миграция `0006`: таблица `login_attempts` (`ts`, `client_ip`, `username`); модель `LoginAttempt`. +- `app/api/v1/auth.py`: 5 неудач на логин и 20 на IP за 10 минут → блокировка на 10 минут (429, заголовок `Retry-After`, `retry_after_seconds`, текст с минутами); во время блокировки пароль не проверяется. Успешный вход очищает попытки логина. + Для несуществующего логина проверяется фиктивный argon2-хэш (выравнивание времени). `LoginIn`: логин ≤ 100, пароль ≤ 128. +- Журнал: `session.failed` — только первая неудача в окне, при срабатывании блокировки одна запись `session.locked` (область, число попыток, время); красный бейдж в UI. +- `app/rotation.py`: при превышении `max_entries` первыми удаляются `session.failed`; `login_attempts` старше суток чистятся в часовом цикле. +- `README.md`: раздел «Журнал». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. 5 неверных паролей → 401×4 и 429; верный пароль при блокировке → 429 + `Retry-After`; в журнале по одной записи `session.failed` и `session.locked`; пароль > 128 символов → 422. +Не проверено: снятие блокировки по истечении 10 минут и лимит по IP (20 попыток). + +## Ограничения +За reverse-proxy без `TRUSTED_PROXIES` все клиенты делят один IP — лимит по IP заденет всех (описано в README). Блокировка по логину позволяет «запереть» чужую учётную запись — событие видно в журнале. diff --git a/docs/changes/013-pagination-bounds/PLAN.md b/docs/changes/013-pagination-bounds/PLAN.md new file mode 100644 index 0000000..af64bc1 --- /dev/null +++ b/docs/changes/013-pagination-bounds/PLAN.md @@ -0,0 +1,24 @@ +# Границы пагинации: отрицательные limit/offset дают 500 (изменение 013) + +Находка ревью № 3, серьёзность — средняя. + +## Context +Во всех списках `limit: int = Query(…, le=…)` и `offset: int = 0` без нижней границы. `GET /audit?limit=-1` и `GET /isps?offset=-1` отвечают 500 +(PostgreSQL отвергает отрицательные LIMIT/OFFSET). Ожидаемо — 422 с указанием поля. + +## Решение +1. `app/api/v1/__init__.py` (или `app/services.py`): общая зависимость + `Paging(limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0, le=10_000_000))`, с параметризацией верхнего `limit` там, где он отличается + (`/prefixes` — 1000, `/organizations` — 500). +2. Заменить объявления в `refs.py` (организации, устройства, операторы), `prefixes.py` (префиксы, адреса), `journal.py` (`/audit`), `users.py`. + Формат ответа, значения по умолчанию и верхние пределы остаются прежними — меняется только отказ на недопустимых значениях. +3. Верхний предел `offset` в `list_addresses` — см. № 014 (там он существенен для ресурсоёмкой ветки `status=free`). + +## Файлы +`app/api/v1/{refs,prefixes,journal,users}.py`, модуль с зависимостью, `tests/test_api.py`, `README.md` (раздел API: параметры пагинации). + +## Тест +Параметризованный: для 3–4 списков `limit=-1`, `limit=0`, `offset=-1` → 422 с полем в `fields`. + +## Проверка +`pytest -q`; UI не затронут (передаёт корректные значения) — пройти основные экраны. diff --git a/docs/changes/013-pagination-bounds/SUMMARY.md b/docs/changes/013-pagination-bounds/SUMMARY.md new file mode 100644 index 0000000..9536708 --- /dev/null +++ b/docs/changes/013-pagination-bounds/SUMMARY.md @@ -0,0 +1,9 @@ +# Итог: границы пагинации (изменение 013) + +## Что сделано +- Во всех списках (`/organizations`, `/devices`, `/isps`, `/users`, `/prefixes`, `/prefixes/{id}/addresses`, `/audit`): `limit` — `ge=1` с прежними верхними пределами, `offset` — `ge=0, le=MAX_OFFSET` (10 000 000, константа в `app/services.py`). Недопустимые значения → 422 вместо 500. +- Отклонение от плана: вместо общей зависимости-класса пагинации использованы единые параметры `Query` и константа `MAX_OFFSET` (проще и без изменения сигнатур). +- `README.md`: раздел API. + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. `limit=-1` и `offset=-1` → 422; `offset` выше предела → 422. diff --git a/docs/changes/014-addresses-listing-performance/PLAN.md b/docs/changes/014-addresses-listing-performance/PLAN.md new file mode 100644 index 0000000..f63de4f --- /dev/null +++ b/docs/changes/014-addresses-listing-performance/PLAN.md @@ -0,0 +1,30 @@ +# Экран адресов: ресурсоёмкие ветки list_addresses (изменение 014) + +Находка ревью № 4, серьёзность — средняя (DoS с ролью viewer). + +## Context +`GET /prefixes/{id}/addresses` (`app/api/v1/prefixes.py`, `list_addresses`): +- при `status=free` собирает в память `offset + limit` свободных адресов; `offset` не ограничен → на IPv6-префиксе один запрос с `offset=10^8` занимает CPU и память воркера; +- без фильтров основной запрос идёт **без LIMIT/OFFSET** — на крупных подсетях загружаются все адреса ради одной страницы. + +## Решение +1. **Свободные адреса без материализации:** функция `free_page(net, occupied_sorted, offset, limit)` в `app/services.py` — идёт по занятым диапазонам, + считает длины свободных промежутков арифметически, пропускает `offset` без перебора и возвращает только `limit` адресов. Для IPv4 ≤ /30 учитывает исключение адреса сети и broadcast. + Занятые адреса — одним запросом `SELECT address … ORDER BY address` (для пула из 10⁵ адресов — приемлемо; при росте — курсор по ключу). +2. **Режимы:** + - `status` = assigned/reserved/deprecated или поиск `q` → пагинация в SQL (`ORDER BY address LIMIT/OFFSET`, `total` — `count()`); + - без фильтров и `cap <= FREE_LISTING_LIMIT` (4096) → как сейчас, смешанный список (он ограничен размером подсети); + - без фильтров и `cap > FREE_LISTING_LIMIT` → только записанные адреса, пагинация в SQL (сейчас — выборка всего и срез в Python); + - `status=free` → `free_page`, `total = summary.free`. +3. `offset` — верхний предел как в № 013; для `status=free` дополнительно `offset <= summary.free`. +4. Ответ API и поведение UI (кнопка «Показать ещё 100») не меняются. + +## Файлы +`app/services.py`, `app/api/v1/prefixes.py`, `tests/test_api.py`, `README.md`. + +## Тест +- Чистая функция `free_page`: IPv4 /24 с занятыми `.1–.3` → offset 0 даёт `.4…`; offset за пределами → пусто; IPv6 /64 с `offset=10^12` — мгновенно. +- API: `status=free&offset=1000000` на IPv6 /64 отвечает быстро (порог времени в тесте не ставим — проверяем корректность адресов). + +## Проверка +`pytest -q`; экран адресов демо-префиксов `10.10.1.0/24` и `fd00::/8` в UI: списки, фильтр «Свободен», «Показать ещё 100». Замер времени ответа до/после на `/16` с 60 000 адресов (скрипт в scratchpad). diff --git a/docs/changes/014-addresses-listing-performance/SUMMARY.md b/docs/changes/014-addresses-listing-performance/SUMMARY.md new file mode 100644 index 0000000..ccb50a0 --- /dev/null +++ b/docs/changes/014-addresses-listing-performance/SUMMARY.md @@ -0,0 +1,11 @@ +# Итог: экран адресов — ресурсоёмкие ветки (изменение 014) + +## Что сделано +- `app/services.py`: `free_page(net, occupied, offset, limit)` — страница свободных адресов арифметикой по промежуткам (без перебора; IPv4 ≤ /30 без адреса сети и broadcast). +- `list_addresses` переписан: `status=free` → `free_page`, `total = summary.free`; фильтры `status`/`q` и подсети > /20 без фильтров — пагинация и `count` в SQL (раньше загружались все адреса и нарезались в Python); малые подсети без фильтров — смешанный список, как раньше (ограничен размером подсети). + `offset` ≤ 10 000 000 (изменение 013). Ответ API и поведение UI не менялись. +- `README.md`: раздел «Модель данных». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. IPv6 /48, `status=free&offset=9000000` → 200 за ~0,02 с; `offset` выше предела → 422; первые свободные адреса, смешанный список /25 (126 строк) корректны. +Не проверено: замер на /16 с десятками тысяч адресов; экран адресов в браузере. diff --git a/docs/changes/015-network-broadcast-addresses/PLAN.md b/docs/changes/015-network-broadcast-addresses/PLAN.md new file mode 100644 index 0000000..5de5449 --- /dev/null +++ b/docs/changes/015-network-broadcast-addresses/PLAN.md @@ -0,0 +1,23 @@ +# Запрет адреса сети и broadcast (изменение 015) + +Находка ревью № 5, серьёзность — средняя. + +## Context +`create_address` проверяет только `ip in network`: в `198.51.100.0/25` назначаются `.0` и `.127` (воспроизведено). При этом `capacity()` для IPv4 ≤ /30 +вычитает эти два адреса, поэтому `free = cap - stored` занижается, загрузка может превысить 100 %. + +## Решение +1. `app/services.py`: `usable(net, ip) -> bool` — для IPv4 с длиной ≤ 30 ложь для адреса сети и broadcast; /31, /32 и IPv6 — без ограничений (как `capacity()` и `next_free`). +2. `create_address` (`app/api/v1/prefixes.py`): при `not usable` — 422 «Адрес сети/broadcast нельзя назначить: 198.51.100.0 — адрес сети 198.51.100.0/25». +3. **Существующие данные:** миграция не нужна. Скрипт `scripts/find_unusable_addresses.py` (только чтение) выводит такие записи; решение по ним — за администратором + (в SUMMARY — результат по стенду). До очистки `utilization` ограничивается 100 % (`min(…, 100)` в `utilization()`), чтобы не показывать >100 %. +4. `allocate_next` уже использует `net.hosts()` — без изменений. + +## Файлы +`app/services.py`, `app/api/v1/prefixes.py`, `scripts/find_unusable_addresses.py`, `tests/test_api.py`, `README.md` (Модель данных). + +## Тест +`/25`: `.0` → 422, `.127` → 422, `.1` → 201; `/31`: оба адреса → 201. + +## Проверка +`pytest -q`; UI «Назначить адрес» с `.0` — сообщение в форме у поля. diff --git a/docs/changes/015-network-broadcast-addresses/SUMMARY.md b/docs/changes/015-network-broadcast-addresses/SUMMARY.md new file mode 100644 index 0000000..51de49c --- /dev/null +++ b/docs/changes/015-network-broadcast-addresses/SUMMARY.md @@ -0,0 +1,10 @@ +# Итог: адрес сети и broadcast (изменение 015) + +## Что сделано +- `app/services.py`: `network_role()` (IPv4, префикс ≤ /30 — как в `capacity()`); `utilization()` ограничена 100 %. +- `create_address`: адрес сети/broadcast → 422 с понятным текстом. Автоназначение уже использовало `hosts()`. +- `scripts/find_unusable_addresses.py` — поиск уже внесённых таких адресов (только чтение); на стенде найдено 0. +- `README.md`: раздел «Модель данных». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. `.0` и `.255` в /24 → 422. diff --git a/docs/changes/016-default-role-viewer/PLAN.md b/docs/changes/016-default-role-viewer/PLAN.md new file mode 100644 index 0000000..bf8d956 --- /dev/null +++ b/docs/changes/016-default-role-viewer/PLAN.md @@ -0,0 +1,22 @@ +# Роль по умолчанию — «Просмотр» (изменение 016) + +Находка ревью № 6, серьёзность — средняя. + +## Context +`POST /users` без поля `role` создаёт администратора (`UserIn.role = Role.admin`, воспроизведено); модель `User.role` тоже `default=Role.admin`. +UI передаёт роль явно, но клиенты API и скрипты по умолчанию получают максимальные права. Принцип наименьших привилегий требует `viewer`. + +## Решение +1. `app/schemas.py`: `UserIn.role: Role = Role.viewer`. +2. `app/models.py`: `User.role` — `default=Role.viewer`. Первичный администратор из `.env` создаётся в `seed()` с явным `role=Role.admin` (уже так) — не затрагивается. + Серверного значения по умолчанию в БД нет → миграция не нужна. +3. `README.md` (раздел «Пользователи и роли»): роль по умолчанию — `viewer`. + +## Файлы +`app/schemas.py`, `app/models.py`, `tests/test_users.py`, `README.md`. + +## Тест +В `test_users_management`: создание без `role` → `role == "viewer"`. + +## Проверка +`pytest -q`; UI «Новый пользователь» по-прежнему предлагает «Просмотр». diff --git a/docs/changes/016-default-role-viewer/SUMMARY.md b/docs/changes/016-default-role-viewer/SUMMARY.md new file mode 100644 index 0000000..86df213 --- /dev/null +++ b/docs/changes/016-default-role-viewer/SUMMARY.md @@ -0,0 +1,8 @@ +# Итог: роль по умолчанию — «Просмотр» (изменение 016) + +## Что сделано +- `app/schemas.py`: `UserIn.role` по умолчанию `Role.viewer`; `app/models.py`: `User.role` — `default=Role.viewer`. Первичный администратор из `.env` по-прежнему создаётся с явной ролью admin. Миграция не нужна. +- `README.md`: раздел «Пользователи и роли». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. `POST /users` без `role` → `viewer`. diff --git a/docs/changes/017-jwt-secret-validation/PLAN.md b/docs/changes/017-jwt-secret-validation/PLAN.md new file mode 100644 index 0000000..6a89e3c --- /dev/null +++ b/docs/changes/017-jwt-secret-validation/PLAN.md @@ -0,0 +1,25 @@ +# Проверка секретов при старте (изменение 017) + +Находка ревью № 7, серьёзность — средняя. + +## Context +`Settings.jwt_secret = "dev-only-secret"` (`app/config.py`). При запуске без `JWT_SECRET` (локально, другое развёртывание) токены подписываются +общеизвестным ключом — их можно подделать для любого логина. В compose пустое значение даёт 500 при входе (PyJWT отвергает пустой ключ) — ошибка всплывает поздно. +Аналогично: при пустой БД и пустом `ADMIN_PASSWORD` администратор молча не создаётся. + +## Решение +1. `app/config.py`: `jwt_secret: str` без значения по умолчанию; валидатор — не короче 32 символов и не из списка известных заглушек (`change-me`, `dev-only-secret`, `secret`). + Ошибка — при импорте настроек, т.е. при старте: контейнер падает с понятным сообщением «JWT_SECRET не задан или слишком короткий (python scripts/gen_env.py)». +2. `app/main.py` `seed()`: пустая таблица `users` и пустой `ADMIN_PASSWORD` → `logging.warning` с инструкцией (не падение: БД может заполняться иначе); + `ADMIN_PASSWORD` короче 8 символов → отказ старта (иначе создаётся пароль слабее, чем разрешает UI). +3. `.env.example`: пометка, что `JWT_SECRET` ≥ 32 символов; `scripts/gen_env.py` уже генерирует 64 символа — без изменений. +4. `alembic/env.py` импортирует `settings` — проверить, что миграции не требуют `JWT_SECRET` (при необходимости вынести `database_url` в отдельный доступ). + +## Файлы +`app/config.py`, `app/main.py`, `alembic/env.py` (при необходимости), `.env.example`, `README.md` (Быстрый старт). + +## Тест +Юнит-тест без БД: `Settings(jwt_secret="short")` → `ValidationError`; `Settings(jwt_secret="x"*32)` — успешно. + +## Проверка +Пересборка стенда с текущим `.env` — старт успешен; запуск контейнера с `JWT_SECRET=` → контейнер завершается с сообщением в логах; `alembic upgrade head` работает. diff --git a/docs/changes/017-jwt-secret-validation/SUMMARY.md b/docs/changes/017-jwt-secret-validation/SUMMARY.md new file mode 100644 index 0000000..9b9c71a --- /dev/null +++ b/docs/changes/017-jwt-secret-validation/SUMMARY.md @@ -0,0 +1,10 @@ +# Итог: проверка секретов при старте (изменение 017) + +## Что сделано +- `app/config.py`: `jwt_secret` без встроенного значения; `validate_secrets()` — длина ≥ 32 и не заглушка (`change-me`, `dev-only-secret` …), `ADMIN_PASSWORD` (если задан) ≥ 8. Вызывается при импорте `app/main.py`: приложение не стартует с небезопасной конфигурацией. +- Отклонение от плана: проверка вынесена из `Settings` в отдельную функцию, чтобы `alembic` (импортирует `settings`) не требовал `JWT_SECRET`. +- `app/main.py` `seed()`: пустая БД и пустой `ADMIN_PASSWORD` → предупреждение в логе. +- `.env.example`, `README.md` (быстрый старт). + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Импорт приложения с `JWT_SECRET=short` → `RuntimeError` с инструкцией; с текущим `.env` — старт успешен. diff --git a/docs/changes/018-patch-null-handling/PLAN.md b/docs/changes/018-patch-null-handling/PLAN.md new file mode 100644 index 0000000..8816105 --- /dev/null +++ b/docs/changes/018-patch-null-handling/PLAN.md @@ -0,0 +1,23 @@ +# PATCH адреса: null даёт ложный 409 (изменение 018) + +Находка ревью № 8, серьёзность — низкая. + +## Context +`update_address` (`app/api/v1/prefixes.py`) использует `model_dump(exclude_unset=True)` без `exclude_none`: `{"description": null}` пишет NULL в колонку NOT NULL → +`IntegrityError` → 409 «Запись с такими значениями уже существует» (воспроизведено). Остальные PATCH-эндпоинты исключают `None`. + +## Решение +1. Единая семантика PATCH: `null` для текстовых полей — «очистить» (записать `""`), для `status` — 422 (обязательное поле), для `device_id` — отвязать устройство (`None`, как сейчас). + Реализация — в `AddressUpdate` через `field_validator`: текстовые `None → ""`, `status` — тип `AddressStatus` без `None`-варианта (поле по-прежнему необязательное). +2. Проверить тем же правилом `PrefixUpdate`, `DeviceUpdate`, `VrfUpdate`, `UserUpdate`: где `exclude_none` скрывает намерение очистить поле (`note`, `description`) — применить то же приведение. +3. Общий обработчик `IntegrityError` оставить как страховку, но текст «Конфликт с существующими данными» (уже так в `main.py`); сообщение + «Запись с такими значениями уже существует» в `commit()` по умолчанию заменить на нейтральное — сейчас оно вводит в заблуждение при нарушении NOT NULL/FK. + +## Файлы +`app/schemas.py`, `app/api/v1/prefixes.py`, `app/services.py` (текст по умолчанию), `tests/test_api.py`. + +## Тест +PATCH адреса `{"description": null}` → 200 и `description == ""`; `{"status": null}` → 422. + +## Проверка +`pytest -q`; в UI правка адреса с очисткой описания. diff --git a/docs/changes/018-patch-null-handling/SUMMARY.md b/docs/changes/018-patch-null-handling/SUMMARY.md new file mode 100644 index 0000000..7d7f07d --- /dev/null +++ b/docs/changes/018-patch-null-handling/SUMMARY.md @@ -0,0 +1,11 @@ +# Итог: null в PATCH (изменение 018) + +## Что сделано +- `app/schemas.py`: общий валидатор `_blank` — `null` в текстовых полях `AddressUpdate` (`dns_name`, `description`, `note`), `PrefixUpdate` (`description`, `note`), `DeviceUpdate` (`mac`, `note`), `VrfUpdate` (`route_target`, `note`) означает «очистить» (пустая строка); + `status` адреса = `null` → 422; `device_id: null` по-прежнему отвязывает устройство. +- `app/services.py`: сообщение по умолчанию для конфликтов БД заменено на нейтральное (`CONFLICT_MSG`: «Конфликт с существующими данными: проверьте уникальность значений и связанные объекты»); + сообщения с явным текстом (дубль префикса, логина и т.п.) не менялись. +- `README.md`: раздел API. + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. `{"description": null}` → 200 и `""`; `{"status": null}` → 422. diff --git a/docs/changes/019-n-plus-one-queries/PLAN.md b/docs/changes/019-n-plus-one-queries/PLAN.md new file mode 100644 index 0000000..9a99c50 --- /dev/null +++ b/docs/changes/019-n-plus-one-queries/PLAN.md @@ -0,0 +1,28 @@ +# N+1 запросов в списках (изменение 019) + +Находка ревью № 9, серьёзность — низкая (производительность). + +## Context +Счётчики и связанные данные догружаются отдельными запросами на каждую строку: `_device_out` (2 запроса на устройство — до 1 000 на странице), +`_isp_out` (организация), `_vrf_out`, `_type_out`, `_org_out` (счётчики), «Обзор» (`_prefix_outs` для всех активных префиксов с `_depths`/`_capacities` по организациям). + +## Решение +1. **Пакетные выходные функции** — принимают список строк и делают фиксированное число запросов: + - устройства: один запрос адресов `WHERE device_id IN (…)` + словарь типов (типов мало — один `SELECT` всех); + - операторы: `selectinload(Isp.networks)` + словарь организаций по `IN`; + - VRF / типы / организации: счётчики одним `GROUP BY` по `IN (…)`; + - одиночные эндпоинты (`GET /devices/{id}` и т.п.) вызывают пакетную функцию со списком из одного элемента. +2. **Обзор:** считать ёмкость и загрузку одним SQL-запросом по листовым активным IPv4-префиксам (лист — префикс без детей) вместо полной сборки `PrefixOut` по всем префиксам; + `top_prefixes` — `_prefix_outs` только для 4 выбранных. +3. Формат ответов не меняется. +4. Контроль: в тестовом режиме — счётчик запросов через событие `before_cursor_execute` (фикстура), чтобы зафиксировать «не больше K запросов на список». + +## Файлы +`app/api/v1/refs.py`, `app/api/v1/overview.py`, `app/api/v1/prefixes.py` (при необходимости), `tests/test_api.py`. + +## Тест +Один тест уровня функций (сессия SQLAlchemy к БД стенда, счётчик через `before_cursor_execute`): пакетный вывод 20 устройств — не больше 3 запросов. +Внешние API-тесты счётчик не видят, поэтому тест вызывает функции напрямую. + +## Проверка +`pytest -q`; сравнение ответов API до/после на демо-данных (скрипт диффа JSON в scratchpad) — идентичны; замер времени `GET /devices?limit=500` на синтетических данных. diff --git a/docs/changes/019-n-plus-one-queries/SUMMARY.md b/docs/changes/019-n-plus-one-queries/SUMMARY.md new file mode 100644 index 0000000..a4d2fd1 --- /dev/null +++ b/docs/changes/019-n-plus-one-queries/SUMMARY.md @@ -0,0 +1,10 @@ +# Итог: N+1 запросов в списках (изменение 019) + +## Что сделано +- `app/api/v1/refs.py`: пакетные выходные функции `_org_outs`, `_vrf_outs`, `_type_outs`, `_device_outs`, `_isp_outs` — счётчики и связи одним `GROUP BY`/`IN` на страницу; операторы — `selectinload(Isp.networks)`. Одиночные `*_out` — обёртки над ними. +- `app/api/v1/prefixes.py`: `_prefix_outs` берёт названия VRF одним запросом (раньше — по запросу на каждый префикс, в том числе на «Обзоре»). +- Отклонение от плана: «Обзор» не переписан на единый SQL-запрос (ёмкость считается в Python по CIDR) — устранён только N+1 по VRF; остальные запросы «Обзора» — постоянное число на организацию. +- Формат ответов не менялся. + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Все списки и `/overview` отвечают 200; замеры количества запросов не проводились. diff --git a/docs/changes/020-next-free-address/PLAN.md b/docs/changes/020-next-free-address/PLAN.md new file mode 100644 index 0000000..e5a9d5e --- /dev/null +++ b/docs/changes/020-next-free-address/PLAN.md @@ -0,0 +1,23 @@ +# Автоназначение адреса с учётом вложенных префиксов (изменение 020) + +Находка ревью № 10 (и часть № 11 — блокировка), серьёзность — низкая. Зависит от № 011. + +## Context +`next_free` (`app/services.py`) исключает только адреса самого пула: если внутри пула есть дочерний префикс, выданный адрес может попасть в его диапазон. +Функция перебирает хосты подряд и загружает все занятые адреса в множество. `allocate_next` не блокирует префикс: при параллельных запросах один получает 409 «повторите». + +## Решение +1. `next_free_address(net, occupied)` — по образцу `next_free_subnet` (перескок за занятые диапазоны, без перебора): занятыми считаются адреса пула, + диапазоны дочерних префиксов того же VRF и (для IPv4 ≤ /30) адрес сети и broadcast. +2. `allocate_next`: `SELECT … FOR UPDATE` строки префикса (как в `allocate_subnet`) — параллельные запросы сериализуются и получают разные адреса. +3. Старую `next_free` удалить. + +## Файлы +`app/services.py`, `app/api/v1/prefixes.py`, `tests/test_api.py`, `README.md` (автоназначение). + +## Тест +Пул `/24` с дочерним `/30` (`.0–.3`) и занятым `.5`: `POST …/addresses/next` → `.4`; ещё раз → `.6`. +Параллельно 5 запросов (потоки) → 5 разных адресов, все 201. + +## Проверка +`pytest -q`; UI не меняется (кнопка автоназначения отсутствует в UI — проверка через Swagger). diff --git a/docs/changes/020-next-free-address/SUMMARY.md b/docs/changes/020-next-free-address/SUMMARY.md new file mode 100644 index 0000000..99e300e --- /dev/null +++ b/docs/changes/020-next-free-address/SUMMARY.md @@ -0,0 +1,10 @@ +# Итог: автоназначение адреса с учётом вложенных префиксов (изменение 020) + +## Что сделано +- `app/services.py`: `next_free_address(net, occupied)` — перескок за занятые диапазоны, без перебора; для IPv4 ≤ /30 без адреса сети и broadcast. Старая `next_free` удалена. +- `app/api/v1/prefixes.py`: общий `_busy_ranges()` (вложенные префиксы любой глубины и адреса префикса) используется и для выбора подсети, и для автоназначения адреса; `allocate_next` блокирует строку префикса (`FOR UPDATE`) — параллельные запросы получают разные адреса. +- `README.md`: раздел API. + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Пул /24 с дочерним /25: автоназначение выдало `198.51.100.128` (вне вложенного префикса). +Не проверено: параллельные запросы (5 потоков). diff --git a/docs/changes/021-concurrency-locks/PLAN.md b/docs/changes/021-concurrency-locks/PLAN.md new file mode 100644 index 0000000..576eea7 --- /dev/null +++ b/docs/changes/021-concurrency-locks/PLAN.md @@ -0,0 +1,24 @@ +# Гонки: последний администратор и регистр логина (изменение 021) + +Находка ревью № 11, серьёзность — низкая. Блокировка автоназначения адреса — в № 020. + +## Context +- `update_user` / `delete_user` проверяют «последний активный администратор» через `count()` без блокировки: два администратора, одновременно отключающие друг друга, + могут оставить систему без администратора. +- Логин уникален без учёта регистра только на уровне проверки в коде; в БД `users.username` уникален с учётом регистра — параллельное создание `Ivanov`/`ivanov` проходит. + +## Решение +1. **Сериализация изменений администраторов:** в `update_user` и `delete_user`, если операция может лишить учётную запись прав администратора (`loses_admin` / удаление активного admin), + до проверки выполнить `pg_advisory_xact_lock()`; проверка и изменение — в одной транзакции. Остальные изменения пользователей не блокируются. +2. **Уникальность логина в БД:** новая миграция — уникальный индекс `lower(username)`; перед созданием миграция проверяет дубли по регистру и останавливается с перечнем. + Предварительная проверка в `create_user` остаётся (даёт понятный 409), `flush` ловит гонку. +3. Вход: поиск пользователя остаётся точным по регистру (логин = `sub` токена); при желании — отдельное решение, не в этом изменении. + +## Файлы +`app/api/v1/users.py`, `alembic/versions/_users_lower_username.py`, `app/models.py` (`Index`), `tests/test_users.py`. + +## Тест +Два активных администратора (временные), параллельно (потоки) каждый отключает другого → ровно одна операция успешна, вторая 409; активный администратор остаётся. + +## Проверка +`pytest -q`; миграция на стенде проходит (дублей нет). diff --git a/docs/changes/021-concurrency-locks/SUMMARY.md b/docs/changes/021-concurrency-locks/SUMMARY.md new file mode 100644 index 0000000..2975ec9 --- /dev/null +++ b/docs/changes/021-concurrency-locks/SUMMARY.md @@ -0,0 +1,10 @@ +# Итог: гонки при изменении администраторов и регистр логина (изменение 021) + +## Что сделано +- `app/api/v1/users.py`: `pg_advisory_xact_lock(703002)` до чтения пользователя и подсчёта администраторов — в `update_user` (если меняются роль или доступ) и в `delete_user`: проверка «последний администратор» и изменение выполняются под блокировкой. +- Миграция `0008`: уникальный индекс `lower(username)` (перед созданием проверка дублей с остановкой и перечнем); `app/models.py`: `Index("uq_users_lower_username", …)`. +- `README.md`: раздел «Пользователи и роли». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Логин `SMOKE-U` при существующем `smoke-u` → 409; миграция на стенде прошла. +Не проверено: параллельное отключение двух администраторов. diff --git a/docs/changes/022-ops-hardening/PLAN.md b/docs/changes/022-ops-hardening/PLAN.md new file mode 100644 index 0000000..37712b3 --- /dev/null +++ b/docs/changes/022-ops-hardening/PLAN.md @@ -0,0 +1,26 @@ +# Эксплуатация: контейнер, миграции, TLS, зависимости (изменение 022) + +Находка ревью № 12, серьёзность — низкая. + +## Context +- `Dockerfile`: процесс работает от root, нет `HEALTHCHECK`; у сервиса `app` в compose нет `healthcheck`, хотя `/healthz` есть. +- `alembic upgrade head` в `CMD` выполняется при старте каждой реплики — при масштабировании миграции гоняются параллельно. +- Приложение отдаётся по HTTP на `0.0.0.0:8088`: JWT и пароли идут открытым текстом по LAN. +- Зависимости заданы диапазонами без lock-файла — сборки невоспроизводимы. + +## Решение +1. **Dockerfile:** пользователь `app` (uid 10001), `USER app`; `HEALTHCHECK` через `python -c` запрос к `/healthz` (curl в slim-образе отсутствует). +2. **Compose:** `healthcheck` у `app`; `restart: unless-stopped` у обоих сервисов; порт приложения по умолчанию — `127.0.0.1:${APP_PORT}` с переменной `APP_BIND` (по умолчанию 127.0.0.1) + для явного открытия в LAN. +3. **Миграции:** в `alembic/env.py` — `pg_advisory_lock` на время `run_migrations` (одна реплика мигрирует, остальные ждут и видят актуальную схему). +4. **TLS:** раздел README «Публикация» — пример блока Caddy (на хосте уже есть контейнер `caddy`) с reverse-proxy на `127.0.0.1:${APP_PORT}` и `TRUSTED_PROXIES` = адрес сети Docker прокси. + Конфигурацию Caddy на хосте не меняем — только документация. +5. **Зависимости:** `requirements.lock` через `pip-compile` (pip-tools в `requirements-dev.txt`); `Dockerfile` ставит из lock-файла; обновление — командой из README. + +## Файлы +`Dockerfile`, `docker-compose.yml`, `alembic/env.py`, `requirements.lock`, `requirements-dev.txt`, `.env.example`, `README.md`. + +## Проверка +- Пересборка стенда `ipam_control_006`: контейнер `healthy`, процесс не root (`docker exec … id`), тесты `pytest -q` проходят. +- Два одновременных `docker compose run app alembic upgrade head` на пустой БД — без ошибок. +- Изменение `APP_BIND` по умолчанию меняет доступность с LAN — **согласовать с пользователем перед внедрением** (сейчас UI открывается по 192.168.5.9:8088). diff --git a/docs/changes/022-ops-hardening/SUMMARY.md b/docs/changes/022-ops-hardening/SUMMARY.md new file mode 100644 index 0000000..825d9b1 --- /dev/null +++ b/docs/changes/022-ops-hardening/SUMMARY.md @@ -0,0 +1,13 @@ +# Итог: эксплуатация — контейнер, миграции, TLS, зависимости (изменение 022) + +## Что сделано +- `Dockerfile`: пользователь `app` (uid 10001), `HEALTHCHECK` через `urllib` к `/healthz`, установка из `requirements.lock`. +- `docker-compose.yml`: `restart: unless-stopped`, healthcheck у `app`, порт `${APP_BIND:-0.0.0.0}:${APP_PORT}:8000`. + **Отклонение от плана:** значение `APP_BIND` по умолчанию — `0.0.0.0` (как раньше), а не `127.0.0.1`, чтобы не отключить доступ из LAN (`192.168.5.9:8088`) без согласования; для закрытия порта задайте `APP_BIND=127.0.0.1` в `.env`. +- `alembic/env.py`: миграции выполняются под `pg_advisory_lock(703000)` на отдельном соединении. +- `requirements.lock` (pip-compile), `pip-tools` в `requirements-dev.txt`; `.env.example`. +- `README.md`: раздел «Публикация и эксплуатация» (пример Caddy, `TRUSTED_PROXIES`, обновление lock-файла). + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Контейнер `healthy`, процесс `uid=10001(app)`, миграции 0005–0008 применены под блокировкой. +Не проверено: одновременный запуск двух миграций; конфигурация Caddy (на хосте не менялась). diff --git a/docs/changes/023-minor-hardening/PLAN.md b/docs/changes/023-minor-hardening/PLAN.md new file mode 100644 index 0000000..552fcd8 --- /dev/null +++ b/docs/changes/023-minor-hardening/PLAN.md @@ -0,0 +1,28 @@ +# Мелкие улучшения безопасности и журнала (изменение 023) + +Находка ревью № 13, серьёзность — низкая. Пункты независимы, внедряются одним изменением. + +## Context и решение +1. **LIKE без экранирования.** `list_prefixes`, `list_users`, `_like()` в `refs.py` не экранируют `%` и `_` (в журнале это сделано — `_escape_like`). + → Перенести `_escape_like` в `app/services.py` как общий `like_pattern(q)` и использовать во всех поисках (`escape="\\"`). +2. **Неудачные попытки очистки журнала не журналируются.** → В `clear_journal` при неверном пароле — запись `journal.clear_failed` (осталось попыток), при блокировке — `journal.clear_locked`. + Бейджи в UI (`eventBadge`): `clear_failed`/`clear_locked` — красный. +3. **Нет заголовков безопасности.** → В `RequestContextMiddleware` (или отдельном ASGI-middleware) добавлять к ответам: + `Content-Security-Policy: default-src 'self'; img-src 'self' data:; style-src 'self'; frame-ancestors 'none'`, `X-Content-Type-Options: nosniff`, + `Referrer-Policy: no-referrer`. Проверить, что UI не использует inline-скрипты/стили (в `app.js` есть атрибуты `style="…"` → для них нужен `style-src 'self' 'unsafe-inline'`; + скрипты inline не используются). Swagger `/docs` грузит ресурсы с CDN → для путей `/docs`, `/redoc` CSP не ставится. +4. **Удаление префикса с `force=true`** не фиксирует число удалённых адресов. → Подсчёт до удаления, `diff: {"force": true, "addresses_deleted": N}`. +5. **Токены после смены пароля действуют до истечения.** → Колонка `users.password_changed_at` (миграция), в токен — `iat`; `current_user` отклоняет токены с `iat < password_changed_at`. + Смена пароля самим пользователем выдаёт новый токен в ответе (`/users/me/password` → `{"ok": true, "access_token": …}`), UI сохраняет его — текущая сессия не прерывается. + +## Файлы +`app/services.py`, `app/api/v1/{refs,prefixes,users,journal,auth}.py`, `app/request_context.py`, `app/security.py`, `app/models.py`, +`alembic/versions/_password_changed_at.py`, `web/app.js`, `tests/test_api.py`, `tests/test_users.py`, `README.md`. + +## Тесты (минимум) +- Поиск организаций по `%` не возвращает все записи. +- Смена пароля: старый токен → 401, новый из ответа → 200. +- Заголовки: `GET /` содержит `Content-Security-Policy` и `X-Content-Type-Options`. + +## Проверка +`pytest -q`; в браузере (Playwright) — пройти все экраны UI без ошибок CSP в консоли; журнал — записи `journal.clear_failed`. diff --git a/docs/changes/023-minor-hardening/SUMMARY.md b/docs/changes/023-minor-hardening/SUMMARY.md new file mode 100644 index 0000000..77f4b1a --- /dev/null +++ b/docs/changes/023-minor-hardening/SUMMARY.md @@ -0,0 +1,14 @@ +# Итог: мелкие улучшения безопасности и журнала (изменение 023) + +## Что сделано +1. **LIKE:** `contains()`/`like_escape()` в `app/services.py`; все поиски (`refs`, `prefixes`, `users`, журнал) экранируют `%` и `_`. +2. **Журнал очистки:** неверный пароль → `journal.clear_failed` (осталось попыток), блокировка → `journal.clear_locked`; красные бейджи в UI. +3. **Заголовки безопасности** (`SecurityHeadersMiddleware` в `app/main.py`): `Content-Security-Policy` (`default-src 'self'`, `style-src 'self' 'unsafe-inline'` — UI использует атрибуты `style`, `frame-ancestors 'none'`), `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`; для `/docs`, `/redoc`, `/openapi.json` CSP не ставится (Swagger с CDN). +4. **`force`-удаление префикса:** в журнале `diff = {"force": true, "addresses_deleted": N}`. +5. **Отзыв токенов при смене пароля:** миграция `0005` (`users.password_changed_at`); в токене claim `pv` (версия пароля), `current_user` отклоняет токены с другой версией (401). Отклонение от плана: вместо сравнения `iat` — версия пароля (не зависит от точности часов). + `POST /users/me/password` возвращает новый `access_token`, UI подхватывает его — текущая сессия не прерывается; сброс пароля администратором отзывает токены пользователя. +- **Побочный эффект:** `tests/test_users.py::test_users_management` (строка «выданный ранее токен продолжает работать») теперь падает — поведение изменено намеренно; тест обновить на этапе ревью с тестами. +- `README.md`: разделы API, «Пользователи и роли», «Журнал». + +## Проверка +Проверка: стенд `ipam_control_006` пересобран, миграции применены до 0008 (`alembic check` — расхождений нет); ручная проверка (скрипт во временной папке, в репозиторий не добавлялся); автотесты по условию этапа не писались. Заголовки на `/` есть, на `/docs` CSP нет; поиск `%` в организациях пуст; старый токен после смены пароля → 401, новый → 200; `addresses_deleted` в журнале. UI при этом не открывался в браузере — CSP проверить на этапе ревью (консоль браузера). diff --git a/docs/changes/024-review-fixes-011-023/PLAN.md b/docs/changes/024-review-fixes-011-023/PLAN.md new file mode 100644 index 0000000..b638780 --- /dev/null +++ b/docs/changes/024-review-fixes-011-023/PLAN.md @@ -0,0 +1,73 @@ +# Исправление находок ревью изменений 011–023 (изменение 024) + +Источник: `docs/reviews/2026-09-26-changes-011-023-review.md`. Изменения 011–023 не закоммичены; правки вносятся поверх них в рабочем дереве. +Новые автотесты не пишутся (отдельный этап). Исключение — п. 3: существующий тест приводится к намеренно изменённому поведению. + +## Находки и решения + +### 1. Параллельные запросы обходят лимит входа (средняя, 012) +**Где:** `app/api/v1/auth.py`, `login`. +**Суть:** проверка блокировки (`_retry_after`) идёт до проверки пароля, запись попытки — после. Параллельные запросы проходят проверку одновременно. +Воспроизведено: 16 параллельных попыток → 16 проверок пароля при лимите 5. +**Решение:** в начале `login`, до `_retry_after`, выполнить `db.execute(select(func.pg_advisory_xact_lock(func.hashtext(name))))`, где `name` — логин в нижнем регистре. +Блокировка держится до конца транзакции, после `commit` снимается. Попытки одного логина сериализуются; лимит по IP при этом считается корректно для каждого логина. +Остальную логику не менять. + +### 2. Перенос адресов нарушает запрет адреса сети/broadcast (низкая, 011 × 015) +**Где:** `app/api/v1/prefixes.py` — `create_prefix`, `_move_to_vrf`, `rehome_addresses`; `alembic/versions/0007_address_vrf_unique.py`. +**Суть:** при создании вложенного префикса адреса родителя из его диапазона переносятся в него, и адрес может стать адресом сети или broadcast нового префикса. +Воспроизведено: `.128` в `/24`, затем создание `.128/25` — адрес стал сетевым. +**Решение:** +- Функция `_unusable_after_rehome(db, p) -> list[str]`: адреса VRF префикса `p`, которые после переноса окажутся в префиксе, где они являются адресом сети или broadcast. + Проверять самый узкий целевой префикс; для IPv4 с длиной ≤ /30 использовать `network_role` из `app/services.py`. +- `create_prefix`: после `flush` и `attach_to_tree`, до `rehome_addresses`, при непустом списке — `db.rollback()` и 422: + «Адреса … станут адресом сети/broadcast префикса X: освободите их или выберите другой префикс». +- `_move_to_vrf`: та же проверка после смены VRF, до `rehome_addresses`, — 422 без частичных изменений (исключение внутри транзакции, `commit` не выполняется). +- `allocate_subnet` не трогать: адреса родителя уже считаются занятыми. +- Миграция 0007: после шага переноса адресов вывести предупреждение (`print` или `logging`) со списком адресов сети/broadcast в новых префиксах. Миграцию не останавливать. + Уже применённую на стенде миграцию не переписывать по смыслу — только добавить вывод. + +### 3. Устаревший тест токенов (низкая, 023) +**Где:** `tests/test_users.py`, строка с комментарием «выданный ранее токен продолжает работать». +**Решение:** привести тест к новой семантике. +- Старый токен `other` после `POST /users/me/password` → 401. +- Токен из ответа (`access_token`) → `GET /auth/me` 200. + +Больше тесты не менять. + +### 4. Полнота журнала входов (низкая, 012) +**Где:** `app/api/v1/auth.py`, `app/rotation.py`. +**Решение:** +- Решение о записи `session.failed` принимать по числу неудач **этого логина** в окне (до вставки). Для этого `_retry_after` должна возвращать счётчики по областям отдельно, + например `{"login": n, "ip": m}`, а не их максимум. +- В `diff` записи `session.locked` с областью `ip` добавить `distinct_logins` — число различных логинов с этого IP в окне. +- `rotate()`: первыми удалять записи `entity_type == "session"` с `action` в (`failed`, `locked`). + +### 5. Сброс своего пароля через PATCH (низкая, 023) +**Где:** `app/api/v1/users.py`, `update_user`. +**Решение:** если `u.id == admin.id` и в запросе есть `password` — 422 «Свой пароль меняется через /users/me/password (с подтверждением текущего)». + +### 6. Нет валидации PATCH устройства (низкая, найдено попутно) +**Где:** `app/schemas.py`, `DeviceUpdate`. +**Решение:** вынести проверки `DeviceIn._name` (FQDN) и `DeviceIn._mac` (формат и нормализация `AA:BB:…`) в модульные функции и применить их в `DeviceIn` и `DeviceUpdate`. +В `DeviceUpdate` `None` пропускать; пустая строка для `mac` допустима, как в `DeviceIn`. Порядок с `_blank` (null → "") сохранить: сначала `_blank`, затем проверка. + +### 7. Мелочи токена (инфо, 023) +**Где:** `app/security.py`. +**Решение:** +- `_password_version`: целочисленный расчёт `(changed - datetime(1970, 1, 1, tzinfo=timezone.utc)) // timedelta(microseconds=1)`. +- `iat` оставить как информационное поле: добавить комментарий, что отзыв работает по `pv`. + +## Артефакты +- `docs/changes/024-review-fixes-011-023/SUMMARY.md` — что сделано по каждому пункту, отклонения от плана, как проверено. +- `README.md` — дополнить там, где меняется поведение: + - 422 при создании префикса или переносе VRF; + - запрет сброса своего пароля через PATCH; + - валидация PATCH устройства. + +## Проверка +1. `venv/bin/python -c 'import app.main'` и `node --check web/app.js`. +2. `docker compose -p ipam_control_006 up -d --build app` — только проект `ipam_control_006`; контейнеры прежней поставки `ipam_control-*` не трогать. +3. `venv/bin/python -m pytest -q` — все тесты проходят. +4. Ручная проверка через API (скрипт в scratchpad, не в репозитории) по п. 1, 2, 5, 6. Временные данные удалить. + После проверки п. 1 очистить `login_attempts`, чтобы не блокировать вход. diff --git a/docs/changes/024-review-fixes-011-023/SUMMARY.md b/docs/changes/024-review-fixes-011-023/SUMMARY.md new file mode 100644 index 0000000..b2fea97 --- /dev/null +++ b/docs/changes/024-review-fixes-011-023/SUMMARY.md @@ -0,0 +1,67 @@ +# Итог: исправление находок ревью изменений 011–023 (изменение 024) + +Источник: `docs/reviews/2026-09-26-changes-011-023-review.md`, план: `PLAN.md` в этой же папке. Все 7 пунктов выполнены поверх незакоммиченных изменений 011–023 (не откатывались). + +## Что сделано + +### 1. Параллельные запросы обходят лимит входа (012) +`app/api/v1/auth.py`, `login`: в начале обработчика — `pg_advisory_xact_lock(hashtext(логин_в_нижнем_регистре))`, до `_retry_after`. Блокировка держится до `commit`/закрытия сессии, попытки одного логина сериализуются; лимит по IP не тронут. + +### 2. Перенос адресов может нарушить запрет адреса сети/broadcast (011 × 015) +`app/api/v1/prefixes.py`: `_unusable_after_rehome(db, p)` — та же логика выбора «самого узкого целевого префикса», что и в `rehome_addresses`, но без переноса: возвращает пары `(адрес, CIDR целевого префикса)`, где адрес окажется сетевым/broadcast (`network_role`). +- `create_prefix`: проверка после `attach_to_tree`, до `rehome_addresses`; при находках — `db.rollback()` и 422 с текстом и целевым префиксом на каждый адрес. +- `_move_to_vrf`: та же проверка после смены VRF и `attach_to_tree`, до `rehome_addresses`; исключение до `commit()` — частичных изменений нет (сессия откатывается при закрытии). +- `allocate_subnet` не тронут. +- Миграция `0007`: после шага переноса адресов — предупреждение (`print`) со списком адресов, ставших сетевым/broadcast в новом префиксе (SQL-запрос через `network()`/`broadcast()`); миграция не останавливается. Сам перенос в 0007 не переписан. + +**Отклонение от плана:** функция возвращает `list[tuple[str, str]]` (адрес + фактический целевой префикс), а не `list[str]`, как в тексте плана. Целевой префикс при переносе VRF не всегда совпадает с `p` (может быть уже существующий вложенный префикс целевого VRF) — сообщение вида «адрес X станет сетевым для p» было бы недостоверным; это подтвердилось на сценарии переноса при первом прогоне ручной проверки. Итоговое сообщение перечисляет каждый адрес с его настоящим целевым CIDR. + +### 3. Устаревший тест токенов (023) +`tests/test_users.py::test_users_management`: старый токен после `POST /users/me/password` → 401; токен из ответа (`access_token`) → `GET /auth/me` 200. Других тестов не касались. + +### 4. Полнота журнала входов (012) +`app/api/v1/auth.py`: `_retry_after` теперь возвращает счётчики по областям отдельно (`{"login": n, "ip": m}`), а не максимум. Решение о записи `session.failed` принимается по числу неудач именно этого логина (`before["login"] == 0`), а не по максимуму с IP. В `diff` записи `session.locked` с областью `ip` добавлено поле `distinct_logins` (число различных логинов с этого IP в окне, `_distinct_logins`). +`app/rotation.py`: условие удаления «шумных» записей первыми при ротации по количеству расширено с `action == "failed"` до `action.in_(("failed", "locked"))`, чтобы `session.locked` тоже вытеснялся первым. + +### 5. Сброс своего пароля через PATCH (023) +`app/api/v1/users.py`, `update_user`: если `u.id == admin.id` и в теле запроса есть `password` — 422 «Свой пароль меняется через /users/me/password (с подтверждением текущего)». Проверка — до извлечения `pwd` из данных. + +### 6. Нет валидации PATCH устройства (найдено попутно) +`app/schemas.py`: `DeviceIn._name`/`_mac` вынесены в модульные функции `_device_name`/`_device_mac` (обе принимают `None` и пропускают его — для `DeviceIn` это неактуально, так как поля обязательны). Применены в `DeviceIn` и `DeviceUpdate` через тот же идиом, что и `_blank` (`field_validator(...)( _device_name )`). В `DeviceUpdate` порядок сохранён: сначала `_blank_text` (null → "" для `mac`/`note`), затем проверка формата; пустая строка для `mac` допустима, как в `DeviceIn`. + +### 7. Мелочи токена (023) +`app/security.py`: `_password_version` считает целочисленно — `(changed - EPOCH) // timedelta(microseconds=1)`, без `float`. У `iat` в `create_token` добавлен комментарий: поле информационное, отзыв токенов работает по `pv`. + +## Изменённые файлы +`app/api/v1/auth.py`, `app/api/v1/prefixes.py`, `app/api/v1/users.py`, `app/schemas.py`, `app/security.py`, `app/rotation.py`, `alembic/versions/0007_address_vrf_unique.py`, `tests/test_users.py`, `README.md`. + +## Проверка +1. `venv/bin/python -c 'import app.main'` — успешно; `node --check web/app.js` — успешно. +2. `docker compose -p ipam_control_006 up -d --build app` — образ пересобран, `ipam_control_006-app-1` в статусе `healthy`; `ipam_control_006-db-1` не тронут, миграция `0007` повторно не выполнялась (была применена ранее; добавленный `print` сработает при применении «с нуля»). +3. `venv/bin/python -m pytest -q` → `14 passed`. +4. Ручная проверка скриптом во временной папке (не в репозитории), учётка администратора из `.env`, порт `APP_PORT`: + - **п.1**: 16 параллельных неверных входов для временного пользователя (`rv-brute-*`) → 4×401, 12×429, в `login_attempts` ровно 5 строк (только первые 5 попыток проверяют пароль — сериализация advisory-lock работает). `login_attempts` очищен после проверки. + - **п.2**: `.128` в `/24` → создание вложенного `.128/25` → 422 (адрес указан вместе с целевым `198.51.100.128/25`); перенос `/24` с адресом `.128` в VRF, где уже есть `203.0.113.128/25`, → 422 (адрес указан вместе с целевым `203.0.113.128/25`). Временные организация/VRF/префиксы удалены. + - **п.5**: `PATCH /users/{id}` на себя с `password` → 422 с текстом плана. + - **п.6**: `PATCH /devices/{id}` с `name="bad name!"` → 422; `mac="zz"` → 422; `mac="aa-bb-cc-dd-ee-ff"` → 200, `mac` в ответе `"AA:BB:CC:DD:EE:FF"`. Временные устройство/тип устройства/организация удалены. + Все четыре пункта — **PASS**. + +## Не проверено +- Снятие блокировки входа по истечении 10 минут и лимит по IP (20 попыток) — вне объёма ручной проверки этого этапа. +- Полное содержимое `session.locked.diff.distinct_logins` при переборе нескольких разных логинов с одного IP (сериализация по логину проверена; сценарий «шумного IP» с несколькими логинами вручную не воспроизводился). +- Повторное применение миграции `0007` «с нуля» (на новой базе) с реальными адресами сети/broadcast — предупреждение добавлено в код, но не исполнялось (на стенде миграция уже была применена ранее). + +## Сверка (независимая проверка после внедрения) +Проверено повторно на стенде `ipam_control_006`, без участия исполнителя: `pytest -q` — 14 passed; контейнер `healthy`; прежняя поставка `ipam_control-*` не тронута. +- П.1: 16 параллельных неверных входов → проверено ровно 5 паролей (4×401, остальные 429) — PASS. +- П.2: вложенный `.128/25` при адресе `.128` у родителя → 422, префикс не создан (откат); обычный вложенный `/25` создаётся и забирает `.5` (регрессии нет) — PASS. +- П.3: тест токенов обновлён, весь набор проходит — PASS. +- П.4: по коду — счётчики по областям раздельно, `distinct_logins`, ротация удаляет `failed` и `locked` первыми — соответствует плану (поведение «шумного IP» не прогонялось). +- П.5: свой пароль через PATCH → 422, чужой → 200 — PASS. +- П.6: `name="bad name!"`/`mac="zz"` → 422, `aa-bb-…` → `AA:BB:…`, `mac: null` → `""` — PASS. +- П.7: целочисленная версия пароля, комментарий к `iat` — соответствует плану. + +Остаточные замечания (не блокируют): +- Лимит по IP не сериализуется между разными логинами: параллельный перебор многих логинов с одного IP может немного превысить 20 попыток. +- `_unusable_after_rehome` проверяет все адреса диапазона, включая уже лежащие в своём самом узком префиксе: адрес сети/broadcast, внесённый до изменения 015, заблокирует создание охватывающего префикса (найти такие — `scripts/find_unusable_addresses.py`, на стенде 0). +- Смена формулы `pv` (float → целое) может однократно разлогинить пользователей, у которых `password_changed_at` уже был задан (на стенде — только временные пользователи проверки). diff --git a/docs/reviews/2026-09-26-changes-011-023-review.md b/docs/reviews/2026-09-26-changes-011-023-review.md new file mode 100644 index 0000000..988292e --- /dev/null +++ b/docs/reviews/2026-09-26-changes-011-023-review.md @@ -0,0 +1,107 @@ +# Ревью изменений 011–023 (доработки по ревью кодовой базы) — 2026-09-26 + +**Объём:** незакоммиченные изменения рабочего дерева поверх `cd09ef0`: 19 файлов, +514/−149, миграции `0005`–`0008`, скрипты `find_duplicate_addresses.py` и `find_unusable_addresses.py`, `requirements.lock`. +**Метод:** чтение diff. Подозрительные места проверены на стенде `ipam_control_006` на временных данных, после проверки данные удалены. Все экраны UI открыты в headless Chromium под новым CSP. Выполнены `alembic check` и `pytest`. + +Пометки: ✅ — воспроизведено на стенде, 🔎 — вывод по коду. + +## Итог + +Изменения в целом соответствуют планам и работают. Подтверждено на стенде: +- `alembic check` без расхождений, миграции 0005–0008 применены; +- контейнер работает от `uid=10001` и в статусе `healthy`; +- под CSP все экраны UI открываются без ошибок в консоли, шрифты загружаются; +- перенос префикса в другой VRF каскадно обновляет `addresses.vrf_id`; +- поиск по `%`, отзыв токенов, ограничение попыток входа и границы пагинации работают; +- тесты: 13 из 14 проходят. + +**Одна существенная находка:** лимит попыток входа обходится параллельными запросами (№ 1). Ещё одно расхождение между 011 и 015 и несколько мелких замечаний перечислены ниже. + +| # | Серьёзность | Изменение | Кратко | +|---|---|---|---| +| 1 | Средняя | 012 | Параллельные запросы обходят лимит входа: 16 одновременных попыток проверили 16 паролей при лимите 5 ✅ | +| 2 | Низкая | 011 × 015 | Перенос адресов в новый вложенный префикс может сделать адрес его адресом сети/broadcast ✅ | +| 3 | Низкая | 023 | `tests/test_users.py` падает: тест ожидает прежнее поведение токенов после смены пароля ✅ | +| 4 | Низкая | 012 | Журнал входов: первые неудачи по новым логинам с «шумного» IP не пишутся; `session.locked` не вытесняется первым при ротации 🔎 | +| 5 | Низкая | 023 | Сброс администратором **своего** пароля через `PATCH /users/{id}` завершает его сессию без выдачи нового токена 🔎 | +| 6 | Низкая (выявлено попутно) | — | `PATCH /devices/{id}` не проверяет `name` и `mac` (принимает `"bad name!"`, `"zz"`) ✅ | +| 7 | Инфо | 023 | Claim `iat` пишется, но не используется; версия пароля считается через `float` 🔎 | + +--- + +## Находки + +### 1. Параллельные запросы обходят лимит входа ✅ (012) +В `login` (`app/api/v1/auth.py`) шаги идут в таком порядке: +1. `_retry_after` проверяет блокировку; +2. проверяется пароль (argon2, около 50–100 мс); +3. попытка записывается; +4. блокировка проверяется повторно. + +Параллельные запросы проходят первую проверку до того, как любая из них запишет попытку, поэтому пароль проверяется в каждой. +**Воспроизведение:** 16 одновременных запросов с неверным паролем для одного логина. Ответы: 4 × 401 и 12 × 429, но в `login_attempts` 16 записей, то есть проверено 16 паролей. Если бы один из них был верным, запрос вернул бы токен: ветка успеха не смотрит на счётчик. Лимит «5 за 10 минут» фактически превращается в «5 + степень параллелизма». +**Исправление (любое из двух):** +- сериализовать попытки по логину: `SELECT pg_advisory_xact_lock(hashtext(:login))` в начале обработчика, до `_retry_after`; +- или записывать попытку **до** проверки пароля (пессимистично), а при успехе удалять её вместе с остальными. Тогда параллельные запросы видят растущий счётчик. + +Первый вариант проще и не меняет семантику журнала. + +### 2. Новый вложенный префикс может получить адрес сети или broadcast ✅ (011 × 015) +`rehome_addresses()` переносит адреса родителя в самый узкий префикс, не проверяя правило из 015. То же делает шаг переноса в миграции `0007`. +**Воспроизведение:** в `/24` назначен `198.51.100.128`, затем создан `198.51.100.128/25`. Адрес переехал в дочерний префикс и стал его адресом сети, хотя назначить его туда напрямую нельзя (422). +**Исправление:** в `create_prefix` и при переносе VRF (`_move_to_vrf`) отклонять операцию (422 или 409), если среди адресов, которые перейдут в новый префикс, есть его адрес сети или broadcast, и перечислять такие адреса. `allocate_subnet` это не затрагивает: он уже считает адреса родителя занятыми. В миграции 0007 такие случаи выводить в лог как предупреждение (`find_unusable_addresses.py` их найдёт). + +### 3. Устаревший тест токенов ✅ (023) +`tests/test_users.py::test_users_management`, строка 55: `other.get("/auth/me").status_code == 200 # выданный ранее токен продолжает работать`. После 023 ответ 401. Поведение изменено намеренно (отзыв токенов при смене пароля), тест нужно привести к новой семантике: +- старый токен даёт 401; +- токен из ответа `POST /users/me/password` работает. + +### 4. Полнота журнала входов 🔎 (012) +- В журнал пишется «первая неудача в окне». Счётчик `before` берётся как максимум по логину и по IP, поэтому если с этого IP уже были неудачи по другим логинам, первая неудача по **новому** логину не попадёт в журнал. При переборе логинов с одного IP в журнале останется одна запись `session.failed` и затем `session.locked` с областью `ip`. Число и список логинов из записей не восстановить. + **Исправление:** считать `before` отдельно по логину для решения о `session.failed`, а в `diff` записи `session.locked` по IP класть число различных логинов. +- При ротации первыми удаляются только `session.failed`. Записи `session.locked` тоже порождаются анонимными клиентами и вытесняют события данных. + **Исправление:** добавить `locked` в список удаляемых первыми. + +### 5. Сброс своего пароля через PATCH 🔎 (023) +`PATCH /users/{id}` с `password` для собственной учётной записи меняет `password_changed_at` и тем самым отзывает текущий токен администратора, но нового токена в ответе нет. UI такой сценарий не допускает (для своей записи поле пароля скрыто), а клиент API окажется разлогинен. +**Исправление:** для `id == admin.id` отклонять `password` с 422 «Используйте /users/me/password» (проверка текущего пароля там обязательна), либо возвращать новый токен. + +### 6. Нет валидации `PATCH /devices/{id}` ✅ (выявлено попутно) +`DeviceUpdate` не проверяет `name` (FQDN) и `mac` (формат, нормализация), хотя `DeviceIn` проверяет. PATCH с `name="bad name!"` и `mac="zz"` возвращает 200. Проблема существовала и до 011–023, но изменение 018 затронуло эту схему. +**Исправление:** переиспользовать валидаторы `DeviceIn._name` и `DeviceIn._mac` в `DeviceUpdate`, пропуская `None`. + +### 7. Мелочи 🔎 (023) +- `iat` добавлен в токен, но нигде не проверяется. Отзыв работает по `pv`, поэтому `iat` можно оставить как информационное поле или убрать. +- `_password_version` считает `int(changed.timestamp() * 1_000_000)` через `float`. Результат детерминирован, так как значение в сессии и в БД совпадает до микросекунды, но надёжнее считать целочисленно: `(changed - EPOCH) // timedelta(microseconds=1)`. + +--- + +## Соответствие планам + +| Изменение | Статус | Замечания | +|---|---|---| +| 011 уникальность IP в VRF | ✅ с замечанием | № 2; перенос в VRF и каскад `vrf_id` проверены | +| 012 лимит входа | ⚠️ | № 1 (обход параллелизмом), № 4 | +| 013 границы пагинации | ✅ | Вместо общей зависимости используется константа `MAX_OFFSET` (отражено в SUMMARY) | +| 014 экран адресов | ✅ | IPv6 `offset=9·10⁶` отвечает за 0,02 с | +| 015 адрес сети/broadcast | ✅ с замечанием | Обход через перенос адресов (№ 2) | +| 016 роль по умолчанию | ✅ | — | +| 017 проверка секретов | ✅ | — | +| 018 null в PATCH | ✅ | Попутно № 6 | +| 019 N+1 | ✅ частично | «Обзор» не переписан (отражено в SUMMARY) | +| 020 автоназначение адреса | ✅ | Параллельный сценарий не проверялся | +| 021 блокировки | ✅ | Параллельное отключение двух администраторов не проверялось | +| 022 эксплуатация | ✅ | `APP_BIND` по умолчанию `0.0.0.0` (сознательно, отражено в SUMMARY) | +| 023 мелкие улучшения | ✅ с замечаниями | № 3, 5, 7; CSP проверен в браузере | + +## Не проверено (для этапа тестов) +- Остановка миграции 0007 на реальном дубле и миграции 0008 на логинах, совпадающих без учёта регистра. +- Снятие блокировки входа через 10 минут; лимит по IP (20 попыток). +- Параллельные сценарии 020 (разные адреса) и 021 (двое администраторов отключают друг друга). +- Одновременный запуск двух `alembic upgrade head`. + +## Рекомендуемые действия до коммита +1. Исправить № 1: advisory-lock по логину в `login`. +2. Исправить № 2: проверка адреса сети и broadcast при переносе адресов в новый префикс. +3. Обновить тест из № 3 на этапе тестов. +4. № 4–7 — по желанию, вместе с этапом тестов. diff --git a/docs/reviews/2026-09-26-codebase-review.md b/docs/reviews/2026-09-26-codebase-review.md new file mode 100644 index 0000000..3de5f37 --- /dev/null +++ b/docs/reviews/2026-09-26-codebase-review.md @@ -0,0 +1,160 @@ +# Ревью кодовой базы IPAM Manager — 2026-09-26 + +**Объём:** `app/` (FastAPI, SQLAlchemy, ~1 700 строк), `web/app.js` (~1 000 строк), `alembic/`, `tests/`, Docker-окружение. +**Состояние:** коммит `cd09ef0` (задачи 001–010). +**Метод:** чтение кода. Подозрительные места проверены запросами к стенду `ipam_control_006` на временных данных, которые потом удалены. Выполнен `alembic check`. + +Пометки: ✅ — воспроизведено на стенде, 🔎 — вывод по коду. + +## Итог + +Кодовая база компактная и последовательная. Бизнес-правила собраны в API. Журнал аудита сделан добротно: ротация под advisory-lock, IP клиента с учётом доверенных прокси. В UI все данные сервера экранируются через `esc()`, XSS-векторов не найдено. Миграции совпадают с моделями (`alembic check`: «No new upgrade operations detected»). Все 14 тестов проходят. + +Основные риски: +- **Целостность адресного пространства.** Один IP можно завести дважды в одном VRF. Можно назначить адрес сети и broadcast. +- **Устойчивость API.** Отрицательные `limit`/`offset` дают 500. У `offset` на экране адресов нет верхней границы. +- **Журнал можно вытеснить без авторизации.** Неудачные входы пишутся без ограничений, ротация по количеству удаляет старые записи. + +| # | Серьёзность | Область | Кратко | +|---|---|---|---| +| 1 | Высокая | Данные | Один IP дважды в VRF (в родителе и в дочернем префиксе); занятость завышается ✅ | +| 2 | Высокая | Безопасность | Анонимный перебор паролей без ограничений и вытеснение журнала аудита 🔎 | +| 3 | Средняя | API | Отрицательные `limit`/`offset` дают 500 ✅ | +| 4 | Средняя | Производительность / DoS | Экран адресов: неограниченный `offset` для `status=free` и выборка без LIMIT 🔎 | +| 5 | Средняя | Данные | Можно назначить адрес сети и broadcast; `free` и загрузка считаются неверно ✅ | +| 6 | Средняя | Безопасность | Роль по умолчанию в `POST /users` — `admin` ✅ | +| 7 | Средняя | Безопасность | Встроенный `jwt_secret = "dev-only-secret"` без проверки при старте 🔎 | +| 8 | Низкая | API | `null` в PATCH адреса даёт 409 «Запись … уже существует» ✅ | +| 9 | Низкая | Производительность | N+1 запросов в списках устройств, операторов, VRF, типов и на «Обзоре» 🔎 | +| 10 | Низкая | Данные | `next_free` (автоназначение адреса) не учитывает вложенные префиксы 🔎 | +| 11 | Низкая | Конкурентность | Нет блокировок в проверке «последний администратор» и в `allocate_next` 🔎 | +| 12 | Низкая | Эксплуатация | Контейнер работает от root, нет healthcheck у `app`, миграции выполняются при старте каждой реплики 🔎 | +| 13 | Низкая | Мелочи | LIKE без экранирования, нет журналирования неудачных попыток очистки журнала, нет заголовков безопасности 🔎 | + +--- + +## Находки + +### 1. Один IP-адрес дважды в VRF ✅ +`create_address` (`app/api/v1/prefixes.py:314`) проверяет две вещи: адрес входит в префикс и уникальна пара `(prefix_id, address)` (`app/models.py:119`). Уникальности адреса в пределах VRF нет. Кроме того, адрес можно записать на родителя, даже если он попадает в диапазон дочернего префикса. +**Воспроизведение:** родитель `198.51.100.0/24`, дочерний `/25`. `198.51.100.5` создаётся и в родителе, и в дочернем (оба ответа 201). У родителя `used = 4` при трёх уникальных адресах. +**Последствия:** дубли в реестре. `_usage` суммирует адреса поддерева, поэтому загрузка и «Обзор» завышаются. +**Рекомендация:** +- Хранить адреса только в самом узком префиксе. При создании проверять, что адрес не попадает ни в один дочерний префикс. +- Добавить уникальность `(vrf_id, address)`: денормализовать `vrf_id` в `addresses` или поставить триггер. Иначе инвариант держится только в коде. +- При создании префикса (`attach_to_tree`) переносить в новый дочерний адреса родителя из его диапазона. + +### 2. Перебор паролей и вытеснение журнала без авторизации 🔎 +`POST /auth/login` (`app/api/v1/auth.py:14`) никак не ограничен. Каждая неудача пишет `session.failed` в `audit_log`. Ротация по количеству (`app/rotation.py:42`) удаляет самые старые записи сверх `max_entries` (по умолчанию 100 000). +**Последствия:** +- неограниченный перебор паролей; +- анонимный клиент может за несколько минут вытеснить из журнала историю реальных изменений. + +Попутно: при несуществующем логине argon2 не вызывается, поэтому по времени ответа можно понять, существует ли логин. У `LoginIn` нет ограничения длины пароля. +**Рекомендация:** +- ограничить частоту попыток по IP и логину (таблица попыток по образцу `ClearAttempt` или прокси); +- при серии неудач агрегировать их в одну запись журнала; +- выполнять фиктивную проверку argon2 для несуществующего логина; +- ограничить `password` до `max_length=128`; +- рассмотреть отдельную квоту ротации для событий `session.*`. + +### 3. Отрицательные `limit`/`offset` дают 500 ✅ +У всех списков объявлено `limit: int = Query(…, le=…)` и `offset: int = 0` без нижней границы. `GET /audit?limit=-1` и `GET /isps?offset=-1` возвращают **500**: PostgreSQL отвергает отрицательный LIMIT/OFFSET. +**Рекомендация:** `Query(…, ge=1, le=…)` и `Query(0, ge=0)` во всех списках (`refs.py`, `prefixes.py`, `journal.py`, `users.py`). Удобно завести общую зависимость пагинации. + +### 4. `GET /prefixes/{id}/addresses`: ресурсоёмкие ветки 🔎 +В `list_addresses` (`app/api/v1/prefixes.py:272`) две проблемы. +- При `status=free` цикл собирает `offset + limit` свободных адресов в список (`need = offset + limit`). У `offset` нет верхней границы. На IPv6-префиксе любой пользователь с ролью `viewer` одним запросом с `offset=10^8` занимает процессор и память воркера. +- Без фильтров основной запрос (строка 289) выполняется **без LIMIT/OFFSET**: все адреса префикса загружаются и нарезаются в Python. На крупных подсетях это десятки тысяч объектов на каждую страницу. + +**Рекомендация:** +- для `free` генерировать нужную страницу арифметически, пропуская занятые диапазоны, без материализации `offset` элементов, и ограничить `offset`; +- в ветке без свободных адресов перенести пагинацию в SQL; +- смешанный режим (занятые + свободные) оставить только для подсетей ≤ `FREE_LISTING_LIMIT`, как сейчас. + +### 5. Назначаются адрес сети и broadcast ✅ +`create_address` проверяет только `ip in network`. В `198.51.100.0/25` успешно назначаются `.0` и `.127`. При этом `capacity()` для IPv4 до `/30` вычитает эти два адреса. В итоге `free = cap - stored` занижается, а загрузка может превысить 100 %. +**Рекомендация:** для IPv4 с длиной ≤ 30 отклонять адрес сети и broadcast (422). Вариант — считать их занятыми только в `capacity`, но это хуже. + +### 6. Роль по умолчанию — администратор ✅ +`UserIn.role` по умолчанию `Role.admin` (`app/schemas.py:79`), и `POST /users` без `role` создаёт администратора. UI передаёт `viewer` явно, но у клиентов API и скриптов по умолчанию наибольшие права. +Та же картина в модели: `User.role` — `default=Role.admin`. +**Рекомендация:** по умолчанию `viewer`. + +### 7. Встроенный JWT-секрет 🔎 +`Settings.jwt_secret = "dev-only-secret"` (`app/config.py:8`). Если приложение запущено без `JWT_SECRET` (локально или другим способом развёртывания), токены подписываются общеизвестным ключом, и их можно подделать для любого логина. +В docker-compose пустое значение даёт не небезопасный режим, а 500 при входе: PyJWT отвергает пустой HMAC-ключ. +**Рекомендация:** убрать значение по умолчанию. При старте проверять длину секрета, например ≥ 32 байт, и не запускаться без него. То же для `ADMIN_PASSWORD` при пустой БД: сейчас без него админ молча не создаётся. + +### 8. `null` в `PATCH /addresses/{id}` даёт ложный 409 ✅ +`update_address` использует `model_dump(exclude_unset=True)` без `exclude_none`, поэтому `{"description": null}` пишет NULL в колонку NOT NULL. Приходит `IntegrityError`, и ответ — 409 «Запись с такими значениями уже существует». +Остальные PATCH-эндпоинты используют `exclude_none=True`. +**Рекомендация:** `exclude_none=True`. Если нужна очистка поля, явно приводить `None` к `""`. + +### 9. N+1 запросов 🔎 +Счётчики и связанные объекты догружаются отдельными запросами на каждую строку: +- `_device_out`: 2 запроса на устройство, до 1 000 на странице из 500; +- `_isp_out`: организация на каждого оператора; +- `_vrf_out`, `_type_out`, `_org_out`: счётчики на каждую строку; +- `_prefix_outs` на «Обзоре»: `_depths` и `_capacities` по каждой организации. + +При текущих объёмах это незаметно, но растёт линейно. +**Рекомендация:** агрегаты одним `GROUP BY` на страницу и `selectinload` для связей. + +### 10. Автоназначение адреса не учитывает вложенные префиксы 🔎 +`next_free` (`app/services.py:309`) исключает только адреса самого пула. Если внутри пула есть дочерний префикс, выданный адрес может попасть в его диапазон. Это частный случай п. 1. +Кроме того, функция перебирает хосты подряд и загружает в память все занятые адреса. Для крупных пулов лучше использовать тот же приём, что и в `next_free_subnet` (перескок за занятые диапазоны). + +### 11. Гонки без блокировок 🔎 +- **Последний администратор.** `update_user` и `delete_user` проверяют «последний активный администратор» через `count()` без блокировки. Два администратора, одновременно отключающие друг друга, могут оставить систему без админа. Нужен `SELECT … FOR UPDATE` по активным администраторам или advisory-lock. +- **Автоназначение адреса.** В `allocate_next` при параллельных запросах один из них получает 409 «повторите запрос». Корректно, но можно блокировать префикс так же, как в `allocate_subnet`. +- **Логин без учёта регистра.** Уникальность проверяется запросом, а в БД `username` уникален с учётом регистра. Нужен индекс `unique(lower(username))`, как у VRF. + +### 12. Эксплуатация 🔎 +- `Dockerfile`: процесс работает от root. Нет `USER`, нет `HEALTHCHECK`, у сервиса `app` в compose нет `healthcheck`, хотя `/healthz` есть. +- `alembic upgrade head` в `CMD` выполняется при старте каждой реплики. При масштабировании нужен отдельный job или advisory-lock в `env.py`. +- Приложение слушает `0.0.0.0:8088` по HTTP, JWT и пароли идут открытым текстом по LAN. Для эксплуатации нужен TLS-прокси (Caddy на хосте уже есть) и `TRUSTED_PROXIES`. +- Зависимости заданы диапазонами без lock-файла, поэтому сборки не воспроизводимы. + +### 13. Мелочи 🔎 +- **LIKE без экранирования.** В `list_prefixes`, `list_users` и `_like()` (`refs.py`) `%` и `_` в поиске не экранируются (в журнале это сделано, `_escape_like`). +- **Очистка журнала.** Неудачные попытки подтверждения пароля пишутся только в `clear_attempts`, в журнал они не попадают, хотя событие значимо для безопасности. +- **Ответы без заголовков безопасности.** Нет `Content-Security-Policy`, `X-Frame-Options`, `X-Content-Type-Options`, поэтому UI можно встроить во фрейм. +- **Удаление префикса с `force=true`.** Запись журнала не содержит числа удалённых вместе с ним адресов. +- **Хранение токена.** Токен лежит в `sessionStorage`: при XSS его можно украсть. XSS сейчас не найден, но CSP снизил бы риск. +- **Смена пароля.** Уже выданные токены не отзываются (задокументировано). Можно хранить `password_changed_at` и сверять с `iat` токена. + +--- + +## Что сделано хорошо +- **Целостность VRF и организации.** Обеспечена в БД: составной FK `fk_prefixes_vrf_org`, уникальный индекс `lower(name)`. +- **Ошибки API.** Единый формат `{"code", "message", "fields"}`. Нарушения ограничений БД переводятся в 409, а не в 500. +- **Журнал.** Ротация под `pg_try_advisory_xact_lock`. IP берётся из `X-Forwarded-For` только от доверенных прокси, разбор идёт справа налево. Мусор в заголовке игнорируется. +- **Пароли.** Хранятся в argon2. Очистка журнала подтверждается паролем с блокировкой после пяти неудач. Отключение учётной записи действует немедленно. +- **UI без сборки и зависимостей.** Всё, что приходит с сервера, экранируется, `innerHTML` используется только с экранированным содержимым. +- **Тесты интеграционные, по реальному стеку.** Они короткие и проверяют бизнес-правила, а не реализацию. + +## Планы доработок +Одна находка — один план в `docs/changes/`: + +| № | План | +|---|---| +| 1 | `011-address-unique-in-vrf` | +| 2 | `012-login-rate-limit` | +| 3 | `013-pagination-bounds` | +| 4 | `014-addresses-listing-performance` | +| 5 | `015-network-broadcast-addresses` | +| 6 | `016-default-role-viewer` | +| 7 | `017-jwt-secret-validation` | +| 8 | `018-patch-null-handling` | +| 9 | `019-n-plus-one-queries` | +| 10 | `020-next-free-address` (после 011) | +| 11 | `021-concurrency-locks` | +| 12 | `022-ops-hardening` | +| 13 | `023-minor-hardening` | + +## Предлагаемый порядок работ +1. **Быстрые правки, около часа, без миграций:** пп. 3, 5, 6, 8, экранирование LIKE из п. 13. +2. **Безопасность:** п. 2 (ограничение частоты входа, агрегирование неудач в журнале), п. 7 (проверка секрета при старте), п. 12 (non-root, healthcheck, TLS через прокси). +3. **Целостность данных:** пп. 1 и 10 — модель «адрес в самом узком префиксе» и уникальность в VRF. Нужны миграция и проверка существующих дублей. +4. **Производительность:** пп. 4 и 9, когда объёмы станут заметными. Ограничение `offset` из п. 4 стоит сделать сразу вместе с п. 3. diff --git a/requirements-dev.txt b/requirements-dev.txt index e3fcd49..ff09da1 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,3 +1,4 @@ -r requirements.txt pytest>=8 httpx>=0.27 +pip-tools>=7 diff --git a/requirements.lock b/requirements.lock new file mode 100644 index 0000000..04e8fe1 --- /dev/null +++ b/requirements.lock @@ -0,0 +1,96 @@ +# +# This file is autogenerated by pip-compile with Python 3.11 +# by the following command: +# +# pip-compile --no-index --output-file=requirements.lock --strip-extras requirements.txt +# +alembic==1.20.0 + # via -r requirements.txt +annotated-doc==0.0.5 + # via fastapi +annotated-types==0.8.0 + # via pydantic +anyio==4.15.1 + # via + # starlette + # watchfiles +argon2-cffi==25.1.0 + # via -r requirements.txt +argon2-cffi-bindings==26.1.0 + # via argon2-cffi +cffi==2.1.1 + # via argon2-cffi-bindings +click==8.5.0 + # via uvicorn +dnspython==2.8.0 + # via email-validator +email-validator==2.3.0 + # via -r requirements.txt +fastapi==0.141.1 + # via -r requirements.txt +greenlet==3.5.6 + # via sqlalchemy +h11==0.16.0 + # via uvicorn +httptools==0.8.0 + # via uvicorn +idna==3.20 + # via + # anyio + # email-validator +mako==1.4.3 + # via alembic +markupsafe==3.0.3 + # via mako +psycopg==3.3.6 + # via -r requirements.txt +psycopg-binary==3.3.6 + # via psycopg +pycparser==3.0 + # via cffi +pydantic==2.13.5 + # via + # fastapi + # pydantic-settings +pydantic-core==2.46.5 + # via pydantic +pydantic-settings==2.15.0 + # via -r requirements.txt +pyjwt==2.15.0 + # via -r requirements.txt +python-dotenv==1.2.3 + # via + # pydantic-settings + # uvicorn +pyyaml==6.0.3 + # via uvicorn +sqlalchemy==2.0.54 + # via + # -r requirements.txt + # alembic +starlette==1.7.0 + # via fastapi +typing-extensions==4.16.0 + # via + # alembic + # anyio + # fastapi + # psycopg + # pydantic + # pydantic-core + # sqlalchemy + # starlette + # typing-inspection +typing-inspection==0.4.4 + # via + # fastapi + # pydantic + # pydantic-settings +uvicorn==0.54.0 + # via -r requirements.txt +uvloop==0.22.1 + # via uvicorn +watchfiles==1.3.0 + # via uvicorn +websockets==17.1 + # via uvicorn diff --git a/scripts/find_duplicate_addresses.py b/scripts/find_duplicate_addresses.py new file mode 100644 index 0000000..d9ede35 --- /dev/null +++ b/scripts/find_duplicate_addresses.py @@ -0,0 +1,14 @@ +"""Только чтение: IP-адреса, записанные несколько раз в одном VRF (мешают миграции 0007). Запуск: venv/bin/python scripts/find_duplicate_addresses.py""" +import pathlib + +import psycopg + +env = dict(l.split("=", 1) for l in pathlib.Path(__file__).resolve().parent.parent.joinpath(".env").read_text().split() if "=" in l) +dsn = f"postgresql://{env['POSTGRES_USER']}:{env['POSTGRES_PASSWORD']}@127.0.0.1:{env['DB_HOST_PORT']}/{env['POSTGRES_DB']}" +with psycopg.connect(dsn) as conn: + rows = conn.execute( + "SELECT p.vrf_id, host(a.address), array_agg(p.prefix::text || ' (адрес id=' || a.id || ')' ORDER BY p.id) " + "FROM addresses a JOIN prefixes p ON p.id = a.prefix_id GROUP BY p.vrf_id, host(a.address) HAVING count(*) > 1 ORDER BY 1, 2").fetchall() +for vrf, ip, where in rows: + print(f"VRF {vrf}: {ip} — {'; '.join(where)}") +print(f"найдено: {len(rows)}") diff --git a/scripts/find_unusable_addresses.py b/scripts/find_unusable_addresses.py new file mode 100644 index 0000000..7b82731 --- /dev/null +++ b/scripts/find_unusable_addresses.py @@ -0,0 +1,17 @@ +"""Только чтение: адреса сети и broadcast, уже записанные в БД (IPv4, префикс ≤ /30). Запуск: venv/bin/python scripts/find_unusable_addresses.py""" +import ipaddress +import pathlib + +import psycopg + +env = dict(l.split("=", 1) for l in pathlib.Path(__file__).resolve().parent.parent.joinpath(".env").read_text().split() if "=" in l) +dsn = f"postgresql://{env['POSTGRES_USER']}:{env['POSTGRES_PASSWORD']}@127.0.0.1:{env['DB_HOST_PORT']}/{env['POSTGRES_DB']}" +with psycopg.connect(dsn) as conn: + rows = conn.execute("SELECT a.id, host(a.address), p.prefix::text, o.name FROM addresses a JOIN prefixes p ON p.id = a.prefix_id JOIN organizations o ON o.id = p.organization_id ORDER BY a.id").fetchall() +found = 0 +for aid, ip, cidr, org in rows: + net, addr = ipaddress.ip_network(cidr), ipaddress.ip_address(ip) + if net.version == 4 and net.prefixlen <= 30 and addr in (net.network_address, net.broadcast_address): + found += 1 + print(f"address id={aid} {ip} в {cidr} ({org}) — {'адрес сети' if addr == net.network_address else 'broadcast'}") +print(f"найдено: {found}") diff --git a/tests/test_users.py b/tests/test_users.py index 6092d5b..3de9ce1 100644 --- a/tests/test_users.py +++ b/tests/test_users.py @@ -50,9 +50,12 @@ def test_users_management(client): # смена своего пароля: нужен текущий, новый не должен совпадать с ним assert other.post("/users/me/password", json={"current_password": "wrong", "new_password": "third-pass-123"}).status_code == 403 assert other.post("/users/me/password", json={"current_password": "reset-pass-123", "new_password": "reset-pass-123"}).status_code == 422 - assert other.post("/users/me/password", json={"current_password": "reset-pass-123", "new_password": "third-pass-123"}).status_code == 200 + changed = other.post("/users/me/password", json={"current_password": "reset-pass-123", "new_password": "third-pass-123"}) + assert changed.status_code == 200 assert _client(name, "third-pass-123") is not None - assert other.get("/auth/me").status_code == 200 # выданный ранее токен продолжает работать + assert other.get("/auth/me").status_code == 401 # старый токен отозван сменой пароля (изменение 023) + other.headers["Authorization"] = "Bearer " + changed.json()["access_token"] + assert other.get("/auth/me").status_code == 200 # токен из ответа смены пароля рабочий # свою учётную запись удалить или отключить нельзя me = client.get("/auth/me").json() diff --git a/web/app.js b/web/app.js index 06708a6..3cd16ae 100644 --- a/web/app.js +++ b/web/app.js @@ -557,7 +557,7 @@ ${fArea("note", "Примечание", { value: a?.note })}`), const ENTITY_RU = { organization: "Организация", vrf: "VRF", prefix: "Префикс", address: "Адрес", device: "Устройство", device_type: "Тип устройства", isp: "Оператор", user: "Пользователь", session: "Сессия", journal: "Журнал" }; const eventBadge = (ev) => { const [entity, action] = ev.split("."); - const cls = { created: "blue", assigned: "green", updated: "amber", password_reset: "amber", deleted: "red", failed: "red", delete_blocked: "amber" }[action] || ""; + const cls = { created: "blue", assigned: "green", updated: "amber", password_reset: "amber", deleted: "red", failed: "red", locked: "red", clear_failed: "red", clear_locked: "red", delete_blocked: "amber" }[action] || ""; return badge(cls, ev); }; const fmtDateSec = (iso) => (iso ? iso.replace("T", " ").slice(0, 19) : "—"); @@ -716,7 +716,8 @@ ${self ? `
Свою учётную запись нельзя п function passwordDialog() { S.dialog = async (v) => { if (v.new_password !== v.confirm_password) throw new ApiError(422, { message: "Пароли не совпадают", fields: { confirm_password: "пароли не совпадают" } }); - await api("/users/me/password", { method: "POST", body: { current_password: v.current_password, new_password: v.new_password } }); + const r = await api("/users/me/password", { method: "POST", body: { current_password: v.current_password, new_password: v.new_password } }); + if (r?.access_token) store.token = r.access_token; // прежние токены отозваны сервером — продолжаем с новым toast("Пароль изменён"); }; openDialog({