Задачи 025-030: ёмкость префиксов, политика входа, дерево префиксов
Повторный анализ кодовой базы (docs/reviews/2026-09-26-codebase-review-2.md) и доработки:
025 Ёмкость префикса — размер его подсети (а не сумма листьев); «Обзор» считает ёмкость
по корневым активным IPv4-префиксам и адреса внутри них.
026 Политика блокировки входа: 5 неудач на логин+IP, 20 на IP, 50 на логин со всех IP,
кроме известных IP (known_logins, миграция 0009) — владельца нельзя заблокировать анонимно.
027 Сериализация попыток входа по IP (advisory-lock после блокировки логина).
028 UI «Префиксы»: загрузка всех страниц (до 20 000), счётчики по total, предупреждение об усечении.
029 Advisory-lock по VRF для операций, меняющих дерево префиксов и раскладку адресов.
030 Исправление замечаний ревью 025-029: _lock_prefix (VRF блокируется до чтения префикса,
409 при одновременном переносе), константы политики входа перенесены в app/services.py.
Тесты: 14 passed (проверка ёмкости родителя приведена к семантике 025); сквозные сценарии
и гонки — docs/reviews/2026-09-26-changes-025-029-review.md, 2026-09-27-changes-030-review.md.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ayurishchevandClaude Opus 5.5 committed
1 parent
13e17fbb47
commit
03d727e496
25 files changed
+877
-78
No files matched your search
@@ -32,7 +32,11 @@ alembic/ scripts/{gen_env,seed_demo,find_duplicate_addresses,find_unusable_addr
|
||||
## Модель данных
|
||||
`organizations` → `vrfs` (по организации, «default» создаётся автоматически) → `prefixes` (дерево через `parent_id`,
|
||||
вложенность определяется автоматически) → `addresses`; `devices` + `device_types`; `isps` + `isp_networks`; `users`; `audit_log`.
|
||||
- Ёмкость листового префикса — размер подсети (IPv4 без сетевого/broadcast); родителя — сумма вложенных листьев.
|
||||
- Ёмкость префикса — размер его собственной подсети (IPv4 без сетевого/broadcast), у листа и у родителя одинаково: после частичного разбиения
|
||||
(автовыделение, изменение 010) ёмкость родителя не падает до суммы вложенных (изменение 025). `used` считается по адресам всего поддерева того же VRF.
|
||||
- «Обзор»: `capacity` — сумма ёмкостей «корневых» активных IPv4-префиксов (не вложенных ни в один другой активный IPv4-префикс того же VRF по CIDR,
|
||||
а не по `parent_id`, чтобы неактивные ветки не влияли на корни); `assigned`/`reserved` — IPv4-адреса этих статусов, лежащие внутри какого-либо корня
|
||||
(адреса в неактивных ветках не учитываются). «Высокая загрузка» — как раньше, по листовым IPv4-префиксам с ёмкостью > 1 (изменение 025).
|
||||
- «Свободные» адреса не хранятся, а вычисляются; в списке адресов они показываются для подсетей до /20. Страница `status=free` считается арифметически
|
||||
(без перебора адресов, `offset` ≤ 10 000 000), остальные фильтры и постраничная выдача — в SQL (изменение 014).
|
||||
- **IP уникален в пределах VRF и хранится в самом узком префиксе** (изменение 011): `addresses.vrf_id` + составной FK `(prefix_id, vrf_id)` (обновляется каскадом при переносе VRF),
|
||||
@@ -50,6 +54,8 @@ alembic/ scripts/{gen_env,seed_demo,find_duplicate_addresses,find_unusable_addr
|
||||
и адреса родителя); `GET` с тем же путём и `?length=` — предпросмотр. Нет места → 409, недопустимый размер → 422. В UI — пункт «Добавить вложенный (авто)» в меню «⋯» префикса.
|
||||
- VRF, тип устройства, организация с зависимыми объектами не удаляются (409). У организации без префиксов, устройств и операторов
|
||||
служебный VRF `default` удаляется вместе с ней (исправлено в изменении 007: раньше такое удаление давало 409).
|
||||
- Изменения дерева префиксов и раскладки адресов одного VRF (создание/удаление/перенос префикса, автовыделение, назначение адреса) выполняются по одному —
|
||||
`pg_advisory_xact_lock` по `vrf_id` берётся до чтения дерева; разные VRF друг друга не блокируют, чтения (`GET`) не блокируются (изменение 029, находка №5).
|
||||
|
||||
## API (`/api/v1`)
|
||||
`POST /auth/login` · `GET /auth/me` · `GET /overview`
|
||||
@@ -88,9 +94,15 @@ CRUD: `/organizations`, `/vrfs`, `/isps`, `/device-types`, `/devices`, `/prefixe
|
||||
- В журнал пишутся также входы (`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 заденет всех.
|
||||
- **Вход:** три области лимита за 10 минут (блокировка — окно после последней неудачи): **логин + IP** — 5 неудач блокирует эту пару; **IP** — 20 неудач блокирует
|
||||
любые логины с этого IP (как раньше); **логин** — 50 неудач со всех IP блокирует логин, но **не для «известных» IP** — тех, с которых этот пользователь уже успешно
|
||||
входил за последние 30 дней (`known_logins`, обновляется при каждом успешном входе; устаревшие записи удаляет ротация). Так анонимный клиент, знающий логин
|
||||
(например, `admin`), не может держать пользователя заблокированным с его обычного рабочего места — только с незнакомых IP (изменение 026, находка №2 ревью). 429 содержит
|
||||
`Retry-After`/`retry_after_seconds`; для несуществующего логина время ответа выравнивается, пароль ≤ 128 символов.
|
||||
Попытки одного логина сериализованы (`pg_advisory_xact_lock`, изменение 024), затем — попытки одного IP по всем логинам (изменение 027, находка №7: без этого перебор разных
|
||||
логинов с одного IP проходит проверку лимита по IP параллельно и превышает его на степень параллелизма); порядок всегда «логин, затем IP», взаимная блокировка исключена.
|
||||
Цена — попытки с одного IP (в том числе за NAT) обрабатываются по одной, время ответа при массовом переборе растёт на время проверки пароля.
|
||||
При ротации по количеству первыми удаляются `session.failed` и `session.locked`. За reverse-proxy без `TRUSTED_PROXIES` все клиенты делят один IP — лимит по IP заденет всех.
|
||||
|
||||
## Публикация и эксплуатация
|
||||
- Контейнер приложения работает от непривилегированного пользователя (uid 10001), у сервиса есть healthcheck (`/healthz`), сервисы перезапускаются (`restart: unless-stopped`).
|
||||
@@ -112,6 +124,8 @@ CRUD: `/organizations`, `/vrfs`, `/isps`, `/device-types`, `/devices`, `/prefixe
|
||||
«Операторы», «Устройства» и «Адреса» — окно редактирования (свободный адрес — «Назначить адрес» с этим IP). Ссылки, шеврон, меню «⋯» работают как раньше и не запускают действие строки;
|
||||
Ctrl/Shift+клик и выделение текста тоже игнорируются.
|
||||
Экран «Пользователи» доступен только администратору: создание, редактирование роли и доступа, удаление на месте, а свой пароль меняется кнопкой «Сменить пароль» в шапке.
|
||||
Экраны «Префиксы» и «Адреса» догружают все страницы `/prefixes` организации (не только первые 1000), но не больше `PREFIX_UI_CAP` (20 000) префиксов; заголовок,
|
||||
вкладка «Все» и подвал показывают `total`, а при срабатывании предела над таблицей появляется строка «Загружено X из N…» (изменение 028, находка №3).
|
||||
|
||||
## Тесты
|
||||
Идут против приложения в контейнерах, учётные данные берутся из `.env`:
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
"""known_logins: известные IP для смягчения общей блокировки по логину
|
||||
|
||||
Revision ID: 0009
|
||||
Revises: 0008
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
from sqlalchemy.dialects import postgresql
|
||||
|
||||
revision = "0009"
|
||||
down_revision = "0008"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"known_logins",
|
||||
sa.Column("username", sa.String(100), primary_key=True),
|
||||
sa.Column("client_ip", postgresql.INET(), primary_key=True),
|
||||
sa.Column("last_seen", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_table("known_logins")
|
||||
+61
-26
@@ -1,42 +1,61 @@
|
||||
"""Вход в UI. Перебор паролей ограничен: {MAX_PER_LOGIN} неудач на логин и {MAX_PER_IP} на IP за {WINDOW_MIN} минут → 429 (изменение 012)."""
|
||||
"""Вход в UI. Перебор паролей ограничен тремя областями (окно {WINDOW_MIN} минут → 429, изменения 012, 026, 027):
|
||||
{MAX_PER_LOGIN_IP} неудач на пару логин+IP, {MAX_PER_IP} на IP (любые логины), {MAX_PER_LOGIN} на логин со всех IP, кроме «известных»
|
||||
(IP, с которого этот пользователь успешно входил за последние {KNOWN_IP_DAYS} дн.) — иначе анонимный клиент, знающий логин,
|
||||
мог бы держать чужую учётную запись заблокированной с любого IP (находка №2 ревью 2026-09-26)."""
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException
|
||||
from sqlalchemy import delete, func, select
|
||||
from sqlalchemy import and_, delete, func, select
|
||||
from sqlalchemy.dialects.postgresql import insert
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.db import get_db
|
||||
from app.models import LoginAttempt, User
|
||||
from app.models import KnownLogin, LoginAttempt, User
|
||||
from app.request_context import request_meta
|
||||
from app.schemas import LoginIn, TokenOut, UserOut
|
||||
from app.security import create_token, current_user, hash_password, verify_password
|
||||
from app.services import ANONYMOUS, audit
|
||||
from app.services import ANONYMOUS, KNOWN_IP_DAYS, LOGIN_WINDOW, MAX_PER_IP, MAX_PER_LOGIN, MAX_PER_LOGIN_IP, 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))
|
||||
# пороги перебора (LOGIN_WINDOW, MAX_PER_*, KNOWN_IP_DAYS) — в app.services (изменение 030, ревью 025-029 находка №2)
|
||||
# пространства ключей двухаргументного advisory-lock (не пересекаются с одноаргументными: LOCK_KEY/USERS_ADMIN_LOCK)
|
||||
LOGIN_LOCK_NS = 7031
|
||||
IP_LOCK_NS = 7032
|
||||
__doc__ = __doc__.format(MAX_PER_LOGIN_IP=MAX_PER_LOGIN_IP, MAX_PER_IP=MAX_PER_IP, MAX_PER_LOGIN=MAX_PER_LOGIN,
|
||||
WINDOW_MIN=int(LOGIN_WINDOW.total_seconds() // 60), KNOWN_IP_DAYS=KNOWN_IP_DAYS)
|
||||
_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)
|
||||
if scope == "login_ip":
|
||||
return and_(LoginAttempt.username == login, LoginAttempt.client_ip == ip)
|
||||
if scope == "ip":
|
||||
return LoginAttempt.client_ip == ip
|
||||
return LoginAttempt.username == login # "login" — по всем 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."""
|
||||
def _is_known(db: Session, login: str, ip: str | None) -> bool:
|
||||
"""IP считается известным для логина, если вход с него был успешен не более KNOWN_IP_DAYS дней назад."""
|
||||
if ip is None:
|
||||
return False
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(days=KNOWN_IP_DAYS)
|
||||
return db.scalar(select(KnownLogin.username).where(KnownLogin.username == login, KnownLogin.client_ip == ip, KnownLogin.last_seen > cutoff)) is not None
|
||||
|
||||
|
||||
def _retry_after(db: Session, login: str, ip: str | None, known: bool) -> tuple[int, str | None, dict[str, int]]:
|
||||
"""(секунд до конца блокировки, область 'login_ip'|'ip'|'login', счётчики по каждой области, до лимита).
|
||||
Блокировка длится окно после последней неудачи. Область 'login' не блокирует, если IP запроса известен
|
||||
(изменение 026, находка №2); 'login_ip' и 'ip' действуют всегда. Счётчики считаются отдельно (изменение 024,
|
||||
находка №4): решение о записи в журнал принимается по конкретной области, а не по максимуму среди них."""
|
||||
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:
|
||||
for scope, limit in (("login_ip", MAX_PER_LOGIN_IP), ("ip", MAX_PER_IP), ("login", MAX_PER_LOGIN)):
|
||||
if scope in ("login_ip", "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()
|
||||
rows = db.scalars(select(LoginAttempt.ts).where(_condition(login, ip, scope), LoginAttempt.ts > now - LOGIN_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 len(rows) >= limit and not (scope == "login" and known):
|
||||
left = int((rows[0] + LOGIN_WINDOW - now).total_seconds()) + 1
|
||||
if left > worst:
|
||||
worst, why = left, scope
|
||||
return worst, why, counts
|
||||
@@ -45,7 +64,15 @@ def _retry_after(db: Session, login: str, ip: str | None) -> tuple[int, str | No
|
||||
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
|
||||
return db.scalar(select(func.count(func.distinct(LoginAttempt.username))).where(LoginAttempt.client_ip == ip, LoginAttempt.ts > now - LOGIN_WINDOW)) or 0
|
||||
|
||||
|
||||
def _remember_login(db: Session, login: str, ip: str | None) -> None:
|
||||
"""Отмечает IP как известный для логина (upsert last_seen); вызывается при успешном входе."""
|
||||
if ip is None:
|
||||
return
|
||||
stmt = insert(KnownLogin).values(username=login, client_ip=ip, last_seen=func.now())
|
||||
db.execute(stmt.on_conflict_do_update(index_elements=[KnownLogin.username, KnownLogin.client_ip], set_={"last_seen": func.now()}))
|
||||
|
||||
|
||||
def _locked(retry: int) -> HTTPException:
|
||||
@@ -58,27 +85,34 @@ 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)
|
||||
# сериализация попыток одного логина (изменение 024, находка №1), затем — этого IP (изменение 027, находка №7):
|
||||
# порядок всегда «логин, затем IP» исключает взаимную блокировку; без второй блокировки параллельный перебор
|
||||
# разных логинов с одного IP проходит проверку лимита по IP одновременно, и лимит превышается на степень
|
||||
# параллелизма — ценой служит то, что попытки с одного IP (в том числе за NAT) обрабатываются по одной,
|
||||
# и при массовом входе время ответа растёт на время проверки argon2.
|
||||
db.execute(select(func.pg_advisory_xact_lock(LOGIN_LOCK_NS, func.hashtext(name))))
|
||||
if ip is not None:
|
||||
db.execute(select(func.pg_advisory_xact_lock(IP_LOCK_NS, func.hashtext(ip))))
|
||||
known = _is_known(db, name, ip)
|
||||
retry, _, _ = _retry_after(db, name, ip, known)
|
||||
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):
|
||||
_, _, before = _retry_after(db, name, ip)
|
||||
_, _, before = _retry_after(db, name, ip, known)
|
||||
db.add(LoginAttempt(client_ip=ip, username=name))
|
||||
db.flush()
|
||||
retry, why, after = _retry_after(db, name, ip)
|
||||
retry, why, after = _retry_after(db, name, ip, known)
|
||||
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)
|
||||
scope_ru = {"login_ip": "по логину и IP", "ip": "с IP", "login": "по логину"}[why]
|
||||
audit(db, ANONYMOUS, "session", None, "locked", body.username[:100], diff,
|
||||
message=f"Вход заблокирован на {max(1, -(-retry // 60))} мин.: {attempts} неудачных попыток ({'по логину' if why == 'login' else 'с IP'})")
|
||||
message=f"Вход заблокирован на {max(1, -(-retry // 60))} мин.: {attempts} неудачных попыток ({scope_ru})")
|
||||
elif before["login"] == 0: # в журнал — только первая неудача по этому логину в окне (не по IP: иначе перебор логинов с одного IP её не оставит)
|
||||
audit(db, ANONYMOUS, "session", None, "failed", body.username[:100], message="Неудачная попытка входа в UI")
|
||||
db.commit()
|
||||
@@ -86,6 +120,7 @@ def login(body: LoginIn, db: Session = Depends(get_db)):
|
||||
raise _locked(retry)
|
||||
raise HTTPException(401, "Неверный логин или пароль")
|
||||
db.execute(delete(LoginAttempt).where(LoginAttempt.username == name))
|
||||
_remember_login(db, name, ip)
|
||||
audit(db, user, "session", None, "login", user.username, message=f"Вход в UI: {user.username}")
|
||||
db.commit()
|
||||
return TokenOut(access_token=create_token(user))
|
||||
|
||||
+31
-9
@@ -1,6 +1,6 @@
|
||||
from fastapi import APIRouter, Depends, Query
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy import and_, func, select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app import schemas as s
|
||||
@@ -24,18 +24,40 @@ class Overview(BaseModel):
|
||||
recent_changes: list[s.AuditOut]
|
||||
|
||||
|
||||
def _ipv4_roots(db: Session):
|
||||
"""Активные IPv4-префиксы, не вложенные ни в один другой активный IPv4-префикс того же VRF (по CIDR, не по
|
||||
parent_id — дерево может включать неактивные ветки). Основа ёмкости и загрузки «Обзора» (изменение 025, находка №1):
|
||||
раньше ёмкость считалась суммой листьев, а назначенные адреса — по всем IPv4-адресам, включая нелистовые префиксы."""
|
||||
q = Prefix.__table__.alias("q")
|
||||
contained = (
|
||||
select(1).select_from(q)
|
||||
.where(q.c.vrf_id == Prefix.vrf_id, q.c.id != Prefix.id, q.c.status == PrefixStatus.active,
|
||||
func.family(q.c.prefix) == 4, q.c.prefix.op(">>")(Prefix.prefix))
|
||||
.exists()
|
||||
)
|
||||
return select(Prefix.vrf_id, Prefix.prefix).where(Prefix.status == PrefixStatus.active, func.family(Prefix.prefix) == 4, ~contained)
|
||||
|
||||
|
||||
@router.get("/overview", response_model=Overview, tags=["overview"])
|
||||
def overview(db: Session = Depends(get_db)):
|
||||
prefixes = db.scalars(select(Prefix).where(Prefix.status == PrefixStatus.active)).all()
|
||||
outs = _prefix_outs(db, list(prefixes))
|
||||
active = db.scalars(select(Prefix).where(Prefix.status == PrefixStatus.active)).all()
|
||||
outs = _prefix_outs(db, list(active))
|
||||
parents = {p.parent_id for p in outs if p.parent_id}
|
||||
leaves = [p for p in outs if p.id not in parents] # ёмкость считаем по листьям, чтобы не удваивать
|
||||
leaves4 = [p for p in leaves if p.family == 4] # IPv6-пространство несопоставимо по размеру — в метрики использования не входит
|
||||
cap = sum(p.capacity for p in leaves4)
|
||||
v4 = func.family(Address.address) == 4
|
||||
assigned = db.scalar(select(func.count()).select_from(Address).where(Address.status == AddressStatus.assigned, v4)) or 0
|
||||
reserved = db.scalar(select(func.count()).select_from(Address).where(Address.status == AddressStatus.reserved, v4)) or 0
|
||||
leaves4 = [p for p in outs if p.id not in parents and p.family == 4] # top_prefixes — как раньше, по листьям
|
||||
top = sorted((p for p in leaves4 if p.capacity > 1), key=lambda p: p.utilization, reverse=True)[:4]
|
||||
|
||||
roots_stmt = _ipv4_roots(db)
|
||||
roots = db.execute(roots_stmt).all()
|
||||
cap = sum(capacity(str(r.prefix)) for r in roots)
|
||||
roots_cte = roots_stmt.cte("overview_roots")
|
||||
counts = dict(db.execute(
|
||||
select(Address.status, func.count(func.distinct(Address.id)))
|
||||
.select_from(Address.__table__.join(roots_cte, and_(roots_cte.c.vrf_id == Address.vrf_id, Address.address.op("<<=")(roots_cte.c.prefix))))
|
||||
.where(func.family(Address.address) == 4, Address.status.in_((AddressStatus.assigned, AddressStatus.reserved)))
|
||||
.group_by(Address.status)
|
||||
).all())
|
||||
assigned = counts.get(AddressStatus.assigned, 0)
|
||||
reserved = counts.get(AddressStatus.reserved, 0)
|
||||
recent = db.scalars(select(AuditLog).order_by(AuditLog.id.desc()).limit(4)).all()
|
||||
return Overview(
|
||||
prefixes=db.scalar(select(func.count()).select_from(Prefix)) or 0,
|
||||
|
||||
+45
-31
@@ -9,12 +9,43 @@ 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, 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,
|
||||
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,
|
||||
)
|
||||
|
||||
router = APIRouter(dependencies=[Depends(current_user)], tags=["prefixes"])
|
||||
FREE_LISTING_LIMIT = 4096 # «свободные» строки показываем, только если подсеть не больше /20
|
||||
VRF_LOCK_NS = 7033 # advisory-lock пространство: изменения дерева/раскладки адресов одного VRF выполняются по одному (изменение 029, находка №5)
|
||||
|
||||
|
||||
def _lock_vrf(db: Session, *vrf_ids: int) -> None:
|
||||
"""Сериализует операции над деревом префиксов и раскладкой адресов одного VRF: без этого параллельные запросы
|
||||
(например, ручное создание и автовыделение в одном родителе) читают дерево по разным снимкам транзакции и могут
|
||||
оставить неверный parent_id. Несколько VRF (перенос между ними) блокируются в порядке возрастания id — исключает
|
||||
взаимную блокировку. Вызывается до любых чтений дерева этого VRF; читатели (GET) не блокируются."""
|
||||
for vrf_id in sorted(set(vrf_ids)):
|
||||
db.execute(select(func.pg_advisory_xact_lock(VRF_LOCK_NS, vrf_id)))
|
||||
|
||||
|
||||
def _lock_prefix(db: Session, id: int, *extra_vrf_ids: int) -> Prefix:
|
||||
"""Читает и блокирует префикс вместе с деревом его VRF, устраняя гонку «vrf_id прочитан до блокировки»
|
||||
(изменение 030, ревью 025-029 находка №1): `vrf_id` читается отдельным SELECT, затем блокируется через
|
||||
`_lock_vrf` (плюс `extra_vrf_ids` — например, целевой VRF при переносе, одним вызовом), и только потом
|
||||
префикс перечитывается под `FOR UPDATE` с `populate_existing`, чтобы не взять устаревший объект из identity map.
|
||||
Если под блокировкой `vrf_id` оказался другим (префикс успели перенести в другой VRF) — 409, без повтора:
|
||||
повтор накопил бы в транзакции лишнюю блокировку устаревшего VRF, и порядок захвата между попытками перестал
|
||||
бы быть строго возрастающим (риск взаимной блокировки с зеркальной операцией). Транзакция откатится при
|
||||
закрытии сессии, и клиент повторяет запрос с нуля, без унаследованных блокировок."""
|
||||
vrf_id = db.scalar(select(Prefix.vrf_id).where(Prefix.id == id))
|
||||
if vrf_id is None:
|
||||
raise HTTPException(404, "Префикс не найден")
|
||||
_lock_vrf(db, vrf_id, *extra_vrf_ids)
|
||||
p = db.scalar(select(Prefix).where(Prefix.id == id).with_for_update().execution_options(populate_existing=True))
|
||||
if p is None:
|
||||
raise HTTPException(404, "Префикс не найден")
|
||||
if p.vrf_id != vrf_id:
|
||||
raise HTTPException(409, "Префикс одновременно изменяется, повторите запрос")
|
||||
return p
|
||||
|
||||
|
||||
# -------------------------------------------------------------------- prefixes
|
||||
@@ -50,36 +81,19 @@ def _depths(db: Session, organization_id: int) -> dict[int, int]:
|
||||
return {i: depth(i) for i in parents}
|
||||
|
||||
|
||||
def _capacities(db: Session, organization_id: int) -> dict[int, int]:
|
||||
"""Ёмкость листа — размер подсети; ёмкость родителя — сумма ёмкостей вложенных листьев."""
|
||||
rows = db.execute(select(Prefix.id, Prefix.parent_id, Prefix.prefix).where(Prefix.organization_id == organization_id)).all()
|
||||
kids: dict[int, list[int]] = {}
|
||||
for i, parent, _ in rows:
|
||||
if parent is not None:
|
||||
kids.setdefault(parent, []).append(i)
|
||||
own = {i: capacity(str(cidr)) for i, _, cidr in rows}
|
||||
memo: dict[int, int] = {}
|
||||
|
||||
def cap(i: int) -> int:
|
||||
if i not in memo:
|
||||
memo[i] = sum(cap(k) for k in kids[i]) if i in kids else own[i]
|
||||
return memo[i]
|
||||
|
||||
return {i: min(cap(i), MAX_CAPACITY) for i in own}
|
||||
|
||||
|
||||
def _prefix_outs(db: Session, rows: list[Prefix]) -> list[s.PrefixOut]:
|
||||
"""Ёмкость префикса — размер его собственной подсети (изменение 025, находка №1): раньше ёмкость родителя
|
||||
считалась суммой ёмкостей вложенных листьев, и после частичного разбиения (изменение 010) расходилась
|
||||
с экраном адресов того же префикса."""
|
||||
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))
|
||||
out = []
|
||||
for r in rows:
|
||||
used, stored = usage.get(r.id, (0, 0))
|
||||
cap = caps.get(r.id, capacity(str(r.prefix)))
|
||||
cap = capacity(str(r.prefix))
|
||||
out.append(s.PrefixOut(
|
||||
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,
|
||||
@@ -216,6 +230,7 @@ def get_prefix(id: int, db: Session = Depends(get_db)):
|
||||
|
||||
@router.post("/prefixes", response_model=s.PrefixOut, status_code=201)
|
||||
def create_prefix(body: s.PrefixIn, db: Session = Depends(get_db), user: User = Depends(admin_user)):
|
||||
_lock_vrf(db, body.vrf_id) # до любых чтений дерева VRF (изменение 029)
|
||||
vrf = get_or_404(db, Vrf, body.vrf_id, "VRF")
|
||||
if vrf.organization_id != body.organization_id:
|
||||
raise HTTPException(422, "VRF принадлежит другой организации")
|
||||
@@ -272,9 +287,7 @@ def preview_subnet(id: int, length: int = Query(ge=1, le=128), db: Session = Dep
|
||||
@router.post("/prefixes/{id}/subnets/next", response_model=s.PrefixOut, status_code=201)
|
||||
def allocate_subnet(id: int, body: s.SubnetNextIn, db: Session = Depends(get_db), user: User = Depends(admin_user)):
|
||||
"""Создаёт вложенный префикс заданного размера в первом свободном выровненном блоке родителя."""
|
||||
parent = db.scalar(select(Prefix).where(Prefix.id == id).with_for_update()) # сериализуем параллельные выделения из одного родителя
|
||||
if parent is None:
|
||||
raise HTTPException(404, "Префикс не найден")
|
||||
parent = _lock_prefix(db, id) # до любых чтений дерева (изменение 029/030) — сериализует параллельные выделения из одного родителя
|
||||
found, _, _ = _find_subnet(db, parent, body.length)
|
||||
if found is None:
|
||||
raise HTTPException(409, f"В префиксе {parent.prefix} нет свободного блока /{body.length}")
|
||||
@@ -291,9 +304,12 @@ def allocate_subnet(id: int, body: s.SubnetNextIn, db: Session = Depends(get_db)
|
||||
|
||||
@router.patch("/prefixes/{id}", response_model=s.PrefixOut)
|
||||
def update_prefix(id: int, body: s.PrefixUpdate, db: Session = Depends(get_db), user: User = Depends(admin_user)):
|
||||
p = get_or_404(db, Prefix, id, "Префикс")
|
||||
data = body.model_dump(exclude_unset=True, exclude_none=True)
|
||||
new_vrf = data.pop("vrf_id", None)
|
||||
# целевой VRF известен из тела запроса до чтения префикса — блокируем исходный и целевой VRF одним вызовом,
|
||||
# раньше чем прочитан текущий p.vrf_id (изменение 030, ревью 025-029 находка №1); если new_vrf совпадёт
|
||||
# с текущим VRF, лишняя блокировка того же VRF безвредна
|
||||
p = _lock_prefix(db, id, new_vrf) if new_vrf is not None else get_or_404(db, Prefix, id, "Префикс")
|
||||
changed = {k: str(v) for k, v in apply_update(p, data).items()}
|
||||
if new_vrf is not None and new_vrf != p.vrf_id:
|
||||
changed.update(_move_to_vrf(db, p, new_vrf)[0])
|
||||
@@ -304,7 +320,7 @@ def update_prefix(id: int, body: s.PrefixUpdate, db: Session = Depends(get_db),
|
||||
|
||||
@router.delete("/prefixes/{id}", status_code=204)
|
||||
def delete_prefix(id: int, force: bool = False, db: Session = Depends(get_db), user: User = Depends(admin_user)):
|
||||
p = get_or_404(db, Prefix, id, "Префикс")
|
||||
p = _lock_prefix(db, id) # до переподвешивания детей: устаревший p.parent_id иначе достанется всем детям (изменение 029/030)
|
||||
used = None if force else blockers(db, select(func.host(Address.address)).where(Address.prefix_id == id).order_by(Address.address))
|
||||
if used:
|
||||
refuse_delete(db, user, "prefix", p, str(p.prefix), "В префиксе есть адреса; удалите их или используйте force=true", {"addresses": used})
|
||||
@@ -377,7 +393,7 @@ def list_addresses(
|
||||
|
||||
@router.post("/prefixes/{id}/addresses", response_model=s.AddressOut, status_code=201)
|
||||
def create_address(id: int, body: s.AddressIn, db: Session = Depends(get_db), user: User = Depends(admin_user)):
|
||||
p = get_or_404(db, Prefix, id, "Префикс")
|
||||
p = _lock_prefix(db, id) # выбор самого узкого префикса должен видеть согласованное дерево VRF (изменение 029/030)
|
||||
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}")
|
||||
@@ -402,9 +418,7 @@ def allocate_next(
|
||||
id: int, body: s.AddressUpdate | None = None, db: Session = Depends(get_db), user: User = Depends(admin_user)
|
||||
):
|
||||
"""Автоназначение первого свободного адреса; только для префиксов с флагом is_pool."""
|
||||
p = db.scalar(select(Prefix).where(Prefix.id == id).with_for_update()) # параллельные запросы получают разные адреса
|
||||
if p is None:
|
||||
raise HTTPException(404, "Префикс не найден")
|
||||
p = _lock_prefix(db, id) # занятые диапазоны (вложенные префиксы) должны читаться по согласованному дереву (изменение 029/030)
|
||||
if not p.is_pool:
|
||||
raise HTTPException(422, "Префикс не является пулом для автоназначения")
|
||||
ip = next_free_address(ipaddress.ip_network(str(p.prefix)), _busy_ranges(db, p)) # вложенные префиксы и адрес сети/broadcast пропускаются
|
||||
|
||||
@@ -182,6 +182,16 @@ class LoginAttempt(Base):
|
||||
__table_args__ = (Index("ix_login_attempts_ip_ts", "client_ip", "ts"), Index("ix_login_attempts_user_ts", "username", "ts"))
|
||||
|
||||
|
||||
class KnownLogin(Base):
|
||||
"""IP, с которых логин уже успешно входил (last_seen обновляется при каждом входе); запись не старше
|
||||
KNOWN_IP_DAYS исключает логин из общей блокировки «по логину со всех IP» (изменение 026, находка №2).
|
||||
Записи старше срока удаляет ротация."""
|
||||
__tablename__ = "known_logins"
|
||||
username: Mapped[str] = mapped_column(String(100), primary_key=True) # в нижнем регистре, как в login_attempts
|
||||
client_ip: Mapped[str] = mapped_column(INET, primary_key=True)
|
||||
last_seen: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now())
|
||||
|
||||
|
||||
class ClearAttempt(Base):
|
||||
"""Неудачные попытки подтверждения пароля при очистке журнала (для блокировки)."""
|
||||
__tablename__ = "clear_attempts"
|
||||
|
||||
+3
-2
@@ -7,8 +7,8 @@ from sqlalchemy import delete, func, select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.db import SessionLocal
|
||||
from app.models import AppSetting, AuditLog, LoginAttempt
|
||||
from app.services import SYSTEM, audit
|
||||
from app.models import AppSetting, AuditLog, KnownLogin, LoginAttempt
|
||||
from app.services import KNOWN_IP_DAYS, SYSTEM, audit # KNOWN_IP_DAYS — из app.services, не из API-слоя (изменение 030, ревью 025-029 находка №2)
|
||||
|
||||
log = logging.getLogger("ipam.rotation")
|
||||
DEFAULTS = {"retention_days": 90, "max_entries": 100_000}
|
||||
@@ -50,6 +50,7 @@ def rotate(db: Session) -> dict | None:
|
||||
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)))
|
||||
db.execute(delete(KnownLogin).where(KnownLogin.last_seen < datetime.now(timezone.utc) - timedelta(days=KNOWN_IP_DAYS))) # изменение 026
|
||||
if by_age or by_count:
|
||||
parts = []
|
||||
if by_age:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import ipaddress
|
||||
from datetime import timedelta
|
||||
from types import SimpleNamespace
|
||||
|
||||
from fastapi import HTTPException
|
||||
@@ -12,6 +13,14 @@ from app.models import Address, AuditLog, Base
|
||||
MAX_CAPACITY = 2**53 - 1
|
||||
MAX_OFFSET = 10_000_000 # верхняя граница offset во всех списках
|
||||
|
||||
# Политика входа (изменение 026): пороги перебора и срок «известного» IP. Вынесены сюда из app.api.v1.auth
|
||||
# (изменение 030, ревью 025-029 находка №2) — app.rotation использует KNOWN_IP_DAYS и не должен зависеть от API-слоя.
|
||||
LOGIN_WINDOW = timedelta(minutes=10)
|
||||
MAX_PER_LOGIN_IP = 5 # логин + IP — блокирует эту пару
|
||||
MAX_PER_IP = 20 # IP — блокирует любые логины с этого IP
|
||||
MAX_PER_LOGIN = 50 # логин со всех IP, кроме известных — сигнал широкого перебора, не бьёт по легитимному пользователю
|
||||
KNOWN_IP_DAYS = 30 # IP считается известным, если вход с него был успешен не позднее этого срока
|
||||
|
||||
|
||||
SYSTEM = SimpleNamespace(username="system")
|
||||
ANONYMOUS = SimpleNamespace(username="anonymous")
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
# Ёмкость и загрузка частично разбитого префикса (изменение 025)
|
||||
|
||||
Находка № 1 из `docs/reviews/2026-09-26-codebase-review-2.md`, серьёзность — средняя. Выполняется первым.
|
||||
|
||||
## Context
|
||||
`_capacities` (`app/api/v1/prefixes.py`) считает ёмкость родителя как сумму ёмкостей его листьев, а `used` (`_usage`) — по всем адресам поддерева,
|
||||
включая адреса самого родителя вне дочерних префиксов. После автовыделения подсетей (изменение 010) частичное разбиение стало обычным сценарием.
|
||||
Воспроизведено: `172.20.0.0/24` с 40 адресами → `40/254, 16 %`; после выделения одного `/30` → `40/2, 100 %`. Экран адресов того же префикса показывает ёмкость 254.
|
||||
«Обзор» (`app/api/v1/overview.py`) искажён той же причиной: `assigned` — все IPv4-адреса, `capacity` — сумма только листьев.
|
||||
|
||||
## Решение
|
||||
1. **Ёмкость префикса — размер его собственной подсети.** `capacity(str(prefix))` из `app/services.py` (IPv4 без адреса сети/broadcast для ≤ /30, ограничение `MAX_CAPACITY`).
|
||||
`_capacities` удалить; в `_prefix_outs` — `cap = capacity(str(r.prefix))`. `used` (`_usage`, по поддереву CIDR того же VRF) не меняется: дублей нет благодаря изменению 011.
|
||||
Так числа в списке префиксов совпадут с `summary.capacity` экрана адресов.
|
||||
2. **«Обзор»:**
|
||||
- «корни» — активные IPv4-префиксы, не содержащиеся ни в одном другом активном IPv4-префиксе того же VRF (по CIDR, не по `parent_id`);
|
||||
- `capacity` = сумма `capacity()` корней;
|
||||
- `assigned` / `reserved` = IPv4-адреса со статусом assigned/reserved, лежащие внутри какого-либо корня того же VRF (адреса в неактивных ветках не учитываются) — один SQL-запрос с `EXISTS`/`JOIN` по `address <<= root.prefix`;
|
||||
- `utilization` — через `utilization()`;
|
||||
- `top_prefixes` («Высокая загрузка») — как сейчас: листья IPv4 с `capacity > 1`, по убыванию загрузки (формула загрузки листа не меняется);
|
||||
- `prefixes`, `vrfs`, `recent_changes` — без изменений.
|
||||
3. Формат ответов API не меняется. UI не меняется.
|
||||
4. `README.md`, раздел «Модель данных»: строку «Ёмкость листового префикса — размер подсети…; родителя — сумма вложенных листьев» заменить новым правилом; описать расчёт «Обзора».
|
||||
|
||||
## Файлы
|
||||
`app/api/v1/prefixes.py`, `app/api/v1/overview.py`, `README.md`.
|
||||
|
||||
## Проверка
|
||||
- Сценарий из Context: после выделения `/30` ёмкость родителя остаётся 254, загрузка — 16 %.
|
||||
- «Обзор» на демо-данных: `capacity` равна сумме корней (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 и т.п. — только активные, IPv4), загрузка не больше 100 %.
|
||||
@@ -0,0 +1,28 @@
|
||||
# Итог: ёмкость и загрузка частично разбитого префикса (изменение 025)
|
||||
|
||||
## Что сделано
|
||||
- `app/api/v1/prefixes.py`: функция `_capacities` (ёмкость родителя = сумма ёмкостей листьев) удалена; `_prefix_outs` считает ёмкость каждого префикса как `capacity(str(r.prefix))` —
|
||||
размер его собственной подсети, независимо от того, есть ли у него вложенные префиксы. `used` не менялся (по-прежнему `_usage`, по поддереву CIDR того же VRF). Неиспользуемый импорт `MAX_CAPACITY` убран.
|
||||
- `app/api/v1/overview.py` переписан:
|
||||
- «корни» (`_ipv4_roots`) — активные IPv4-префиксы, не вложенные ни в один другой активный IPv4-префикс того же VRF по CIDR (`NOT EXISTS` с оператором `>>`, не по `parent_id`);
|
||||
- `capacity` — сумма `capacity()` корней;
|
||||
- `assigned`/`reserved` — один SQL-запрос: `addresses` JOIN CTE корней по `vrf_id` и `address <<= root.prefix`, `COUNT(DISTINCT id)` по статусу, только IPv4;
|
||||
- `top_prefixes` («Высокая загрузка») не менялся — по-прежнему листовые IPv4-префиксы с ёмкостью > 1, по убыванию загрузки;
|
||||
- `prefixes`, `vrfs`, `recent_changes` не менялись.
|
||||
- `README.md`, раздел «Модель данных»: заменено правило ёмкости, описан расчёт «Обзора» по корням.
|
||||
|
||||
## Отклонения от плана
|
||||
Нет. План выполнен как описан.
|
||||
|
||||
## Как проверено
|
||||
1. `venv/bin/python -c 'import app.main'`, `node --check web/app.js` — без ошибок.
|
||||
2. Стенд `ipam_control_006` пересобран (`docker compose -p ipam_control_006 up -d --build app`), контейнер `healthy`.
|
||||
3. Сценарий из плана на временной организации (`rv3-cap-test`, скрипт в scratchpad, не в репозитории): `172.20.0.0/24` + 40 адресов → `used=40, capacity=254, utilization=16`;
|
||||
после `POST /prefixes/{id}/subnets/next {length:30}` — те же `capacity=254, utilization=16` у родителя (раньше падало до `capacity=2, utilization=100`). **PASS.**
|
||||
4. «Обзор»: результат API (`capacity=16843256`, `assigned=485` с учётом временных данных) сверен с прямым SQL-запросом той же логики (роли посчитаны вручную в psql) после удаления временной
|
||||
организации (`capacity=16843002`, `assigned=445`) — разница ровно 254/40, что соответствует временному корню `172.20.0.0/24`. **PASS.**
|
||||
5. Временная организация и все её объекты удалены по завершении проверки.
|
||||
|
||||
## Не проверено
|
||||
- Внешний вид в браузере (браузер недоступен в этой среде) — проверено только через прямые вызовы API и сверку с SQL.
|
||||
- Сценарий с несколькими VRF и IPv6-префиксами одновременно (на демо-данных стенда — только реальные данные, без специально подготовленного смешанного случая).
|
||||
@@ -0,0 +1,34 @@
|
||||
# Политика блокировки входа без блокировки администратора анонимом (изменение 026)
|
||||
|
||||
Находка № 2 из `docs/reviews/2026-09-26-codebase-review-2.md`, серьёзность — средняя.
|
||||
|
||||
## Context
|
||||
Лимит по логину (5 неудач за 10 минут, изменение 012) действует для всех IP, и во время блокировки отклоняется и верный пароль.
|
||||
Любой, кто знает логин (например, `admin`), может отправлять 5 неверных попыток раз в 10 минут и держать учётную запись заблокированной.
|
||||
|
||||
## Решение (`app/api/v1/auth.py`, `app/models.py`, миграция)
|
||||
1. **Три области лимита** (окно 10 минут, блокировка — окно после последней неудачи, как сейчас):
|
||||
| Область | Порог | Кого блокирует |
|
||||
|---|---|---|
|
||||
| логин + IP | 5 | этот логин с этого IP |
|
||||
| IP | 20 | любые логины с этого IP (как сейчас) |
|
||||
| логин (все IP) | 50 | этот логин со всех IP, **кроме «известных» IP** |
|
||||
Константы `MAX_PER_LOGIN_IP = 5`, `MAX_PER_IP = 20`, `MAX_PER_LOGIN = 50` в `auth.py`.
|
||||
2. **Известные IP:** таблица `known_logins (username, client_ip, last_seen)` с первичным ключом `(username, client_ip)` — новая миграция `0009` и модель `KnownLogin`.
|
||||
При успешном входе — upsert (`INSERT … ON CONFLICT (username, client_ip) DO UPDATE SET last_seen = now()`). IP считается известным, если `last_seen` не старше 30 дней (`KNOWN_IP_DAYS = 30`).
|
||||
Блокировка по области «логин» не применяется к запросам с известного IP. Области «логин + IP» и «IP» действуют всегда.
|
||||
Записи старше 30 дней удалять в часовом цикле ротации (`app/rotation.py`, рядом с очисткой `login_attempts`).
|
||||
3. `_retry_after` возвращает `(секунды, область, счётчики)`, где область — `login_ip` | `ip` | `login`, счётчики — по каждой области. Запись `session.failed` — по-прежнему только первая неудача
|
||||
этого логина в окне (счётчик по логину во всех IP). `session.locked` — один раз при первом пересечении порога любой областью, в `diff` — `scope` (`login_ip`/`ip`/`login`), `attempts`, `retry_after_seconds`,
|
||||
для `ip` — `distinct_logins` (как сейчас).
|
||||
4. Сериализацию попыток по логину (`pg_advisory_xact_lock(hashtext(name))`, изменение 024) и выравнивание времени ответа сохранить.
|
||||
5. Успешный вход удаляет неудачи этого логина (по всем IP), как сейчас.
|
||||
6. `README.md`, раздел «Журнал» (абзац «Вход»): новые пороги, известные IP, почему анонимный клиент не может заблокировать вход с рабочего места пользователя.
|
||||
|
||||
## Файлы
|
||||
`app/api/v1/auth.py`, `app/models.py`, `alembic/versions/0009_known_logins.py`, `app/rotation.py`, `README.md`.
|
||||
|
||||
## Проверка
|
||||
- 5 неверных попыток логина с IP A → 429 для A; верный пароль с IP B, с которого этот пользователь входил ранее, — 200.
|
||||
- 50 неверных попыток логина с разных IP (эмуляция через `TRUSTED_PROXIES` недоступна на стенде → проверка по коду или прямыми вставками в `login_attempts`) → вход с нового IP 429, с известного — 200.
|
||||
- `alembic check` без расхождений.
|
||||
@@ -0,0 +1,41 @@
|
||||
# Итог: политика блокировки входа без блокировки администратора анонимом (изменение 026)
|
||||
|
||||
Выполнено вместе с изменением 027 (один файл `app/api/v1/auth.py`); отдельный SUMMARY для 027 — там же, с разницей только в advisory-lock по IP.
|
||||
|
||||
## Что сделано
|
||||
- `app/api/v1/auth.py`:
|
||||
- три области лимита за окно 10 минут: `login_ip` (5, эта пара логин+IP), `ip` (20, любые логины с этого IP — как раньше), `login` (50, этот логин со всех IP, **кроме известных**);
|
||||
константы `MAX_PER_LOGIN_IP`, `MAX_PER_IP`, `MAX_PER_LOGIN`;
|
||||
- `_is_known(db, login, ip)` — IP считается известным для логина, если есть строка в `known_logins` с `last_seen` не старше `KNOWN_IP_DAYS = 30`;
|
||||
- `_retry_after` переписана: считает счётчики по всем трём областям, блокировка возвращается для первой пересечённой, но `scope == "login"` пропускается (не блокирует), если `known=True`;
|
||||
`login_ip`/`ip` блокируют всегда;
|
||||
- `_remember_login(db, login, ip)` — upsert в `known_logins` (`INSERT … ON CONFLICT (username, client_ip) DO UPDATE SET last_seen = now()`, через `sqlalchemy.dialects.postgresql.insert`),
|
||||
вызывается при успешном входе;
|
||||
- `session.locked`: `diff.scope` теперь одно из `login_ip`/`ip`/`login` (а не только `login`/`ip`, как в изменении 024), сообщение в журнале — по-русски описывает область.
|
||||
- `app/models.py`: модель `KnownLogin` (`username`, `client_ip` — составной первичный ключ; `last_seen`).
|
||||
- `alembic/versions/0009_known_logins.py`: таблица `known_logins`.
|
||||
- `app/rotation.py`: `known_logins` с `last_seen` старше `KNOWN_IP_DAYS` удаляются вместе с остальной ротацией (импорт `KNOWN_IP_DAYS` из `auth.py`).
|
||||
- `README.md`, раздел «Журнал» → абзац «Вход»: описаны три области, известные IP и защита администратора от анонимной блокировки.
|
||||
|
||||
## Отклонения от плана
|
||||
Нет. Пороги и пространства имён — как в плане (значения из примера плана: `MAX_PER_LOGIN_IP=5`, `MAX_PER_IP=20`, `MAX_PER_LOGIN=50`, `KNOWN_IP_DAYS=30`).
|
||||
|
||||
## Как проверено
|
||||
1. `venv/bin/python -c 'import app.main'` — без ошибок (нет циклического импорта `rotation.py` → `auth.py`, так как `auth.py` не импортирует `rotation`).
|
||||
2. Стенд пересобран, `healthy`; миграция `0009` применена (`alembic_version = 0009`); `alembic check` — без расхождений.
|
||||
3. Сценарий из плана (временный пользователь `rv3-locka`, скрипт в scratchpad):
|
||||
- 5 неверных попыток логина с одного IP (докер-шлюз, виден серверу как `172.31.0.1`) → 4×401, 5-я попытка сама пересекает порог `login_ip` и получает 429 (как и раньше для одиночного порога);
|
||||
верный пароль с того же IP сразу после — тоже 429 (область `login_ip` активна);
|
||||
- верный пароль с **другого** реального IP (127.0.0.1, изнутри контейнера приложения) для того же логина — **200**: анонимный клиент с одного IP не блокирует пользователя с другого. **PASS.**
|
||||
4. Сценарий «перебор с разных IP» (план: «эмуляция через `TRUSTED_PROXIES` недоступна на стенде → проверка … прямыми вставками в `login_attempts`»), временный пользователь `rv3-lockb`:
|
||||
- успешный вход с IP A (докер-шлюз) регистрирует его как известный;
|
||||
- 55 неудачных попыток вставлены напрямую в `login_attempts` с 55 разными фиктивными IP (`203.0.113.1..55`) для этого логина;
|
||||
- вход с верным паролем с нового, никогда не виденного IP (127.0.0.1 изнутри контейнера) → **429** (область `login`, IP не известен);
|
||||
- вход с верным паролем с известного IP A → **200** (область `login` не применяется к известному IP; счётчик пересобирается заново после того, как предыдущая проверка его не сбросила). **PASS.**
|
||||
5. `login_attempts` очищена после проверок (`delete from login_attempts`). Временные пользователи `rv3-locka`/`rv3-lockb` удалены. Осталась одна легитимная строка `known_logins`
|
||||
(`admin`, IP докер-шлюза) — образовалась от обычных входов администратора при тестировании, не «rv3-» тестовые данные, не удалялась.
|
||||
|
||||
## Не проверено
|
||||
- Реальный перебор с физически разных IP-адресов (сеть стенда не даёт больше двух различимых реальных адресов) — область `login` проверена вставкой записей напрямую в `login_attempts`,
|
||||
как и предусмотрено планом.
|
||||
- Поведение при `TRUSTED_PROXIES`, настроенном на реальный прокси (не задан на стенде).
|
||||
@@ -0,0 +1,21 @@
|
||||
# Сериализация попыток входа по IP (изменение 027)
|
||||
|
||||
Находка № 7 из `docs/reviews/2026-09-26-codebase-review-2.md`, серьёзность — низкая. Выполняется вместе с 026 (тот же файл).
|
||||
|
||||
## Context
|
||||
Изменение 024 сериализует попытки одного логина (`pg_advisory_xact_lock(hashtext(логин))`). Параллельные попытки **разных** логинов с одного IP проходят проверку лимита по IP
|
||||
одновременно, и порог 20 можно превысить на степень параллелизма.
|
||||
|
||||
## Решение (`app/api/v1/auth.py`, `login`)
|
||||
1. После блокировки по логину и до `_retry_after` — вторая блокировка `pg_advisory_xact_lock(hashtext('ip:' || ip))`, если `ip` известен.
|
||||
Порядок всегда «логин, затем IP»: IP-блокировка берётся последней, поэтому цикл ожидания невозможен.
|
||||
2. Использовать двухаргументную форму advisory-lock с отдельными пространствами ключей, чтобы исключить коллизии между логинами и IP:
|
||||
`pg_advisory_xact_lock(<LOGIN_NS>, hashtext(name))` и `pg_advisory_xact_lock(<IP_NS>, hashtext(ip))`; константы пространств — в `auth.py` (например, 7031 и 7032).
|
||||
3. Комментарий в коде: цена — попытки с одного IP (в том числе за NAT) обрабатываются по одной, время ответа при массовом входе растёт на время argon2.
|
||||
|
||||
## Файлы
|
||||
`app/api/v1/auth.py`, `README.md` (одна фраза в абзаце «Вход»).
|
||||
|
||||
## Проверка
|
||||
16 параллельных неверных входов на 16 разных несуществующих логинов с одного IP при заранее вставленных 15 неудачах этого IP → проверено не больше 5 паролей
|
||||
(в `login_attempts` добавилось ≤ 5 записей), остальные — 429.
|
||||
@@ -0,0 +1,28 @@
|
||||
# Итог: сериализация попыток входа по IP (изменение 027)
|
||||
|
||||
Выполнено вместе с изменением 026 (один файл `app/api/v1/auth.py`, один проход); детали трёх областей лимита — в SUMMARY 026.
|
||||
|
||||
## Что сделано
|
||||
- `app/api/v1/auth.py`, `login`: после advisory-lock по логину (`pg_advisory_xact_lock(LOGIN_LOCK_NS, hashtext(name))`, изменение 024) и до `_retry_after` — вторая блокировка по IP,
|
||||
если `ip` известен: `pg_advisory_xact_lock(IP_LOCK_NS, hashtext(ip))`. Порядок всегда «логин, затем IP» — исключает взаимную блокировку.
|
||||
- Использована двухаргументная форма advisory-lock с раздельными пространствами ключей (`LOGIN_LOCK_NS = 7031`, `IP_LOCK_NS = 7032`), чтобы `hashtext(логин)` и `hashtext(ip)`
|
||||
гарантированно не пересекались (не зависит от совпадения хэшей, как было бы при одноаргументной форме с общим пространством).
|
||||
- Комментарий в коде — плата за сериализацию: попытки с одного IP (в том числе за NAT) обрабатываются по одной, время ответа при массовом переборе растёт на время проверки argon2.
|
||||
- `README.md`, абзац «Вход»: добавлено предложение о второй сериализации и её цене.
|
||||
|
||||
## Отклонения от плана
|
||||
Нет.
|
||||
|
||||
## Как проверено
|
||||
1. `venv/bin/python -c 'import app.main'` — без ошибок.
|
||||
2. Стенд пересобран и здоров (см. SUMMARY 026 — общая пересборка для 026+027).
|
||||
3. Сценарий из плана: 15 неудачных попыток по IP (докер-шлюз) вставлены напрямую в `login_attempts` (порог `MAX_PER_IP = 20`, то есть до блокировки осталось 5); затем 16 параллельных
|
||||
(через `ThreadPoolExecutor`, 16 потоков) неверных входов на 16 разных **несуществующих** логинов с этого же IP:
|
||||
- ответы: 4×401, 12×429;
|
||||
- новых строк в `login_attempts` для этого IP (кроме затравки) — **5** (запрос, доведший счётчик до 20, тоже проверяет пароль и добавляет строку, но получает 429, а не 401 —
|
||||
поэтому 401 на один меньше числа добавленных строк, само число строк не превышает лимит). Соответствует условию плана «добавилось ≤ 5 записей». **PASS.**
|
||||
4. `login_attempts` очищена после проверки.
|
||||
|
||||
## Не проверено
|
||||
- Поведение при коллизии `hashtext` между конкретным логином и конкретным IP в старой (одноаргументной) схеме — не воспроизводилось намеренно, замена на двухаргументную форму
|
||||
устраняет саму возможность, отдельно не тестировалось.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Экран «Префиксы»: без молчаливого усечения (изменение 028)
|
||||
|
||||
Находка № 3 из `docs/reviews/2026-09-26-codebase-review-2.md`, серьёзность — низкая.
|
||||
|
||||
## Context
|
||||
`screens.prefixes` и `screens.address` (`web/app.js`) запрашивают `/prefixes?organization_id=…&limit=1000` одной страницей. Заголовок («N префиксов»), вкладка «Все» и подвал
|
||||
показывают `all.length` (число загруженных), а не `total`. Родительский список в «Новом префиксе», хлебные крошки экрана адресов и размер по умолчанию в «Добавить вложенный (авто)»
|
||||
строятся по усечённому набору. Организация, в которой `/22` разбит на `/30`, быстро превышает 1 000 префиксов.
|
||||
|
||||
## Решение (`web/app.js`, только UI)
|
||||
1. Функция `loadPrefixes(orgId)`: загружает все страницы `/prefixes` (`limit=1000`, `offset += 1000`) до `total`, но не больше `PREFIX_UI_CAP = 20000` префиксов;
|
||||
возвращает `{ items, total }`. Страницы запрашиваются последовательно (порядок `ORDER BY vrf_id, prefix` стабилен).
|
||||
2. `screens.prefixes` и `screens.address` используют `loadPrefixes` вместо одиночного запроса.
|
||||
3. Заголовок, вкладка «Все» и подвал используют `total`. Если `items.length < total` (сработал `PREFIX_UI_CAP`), над таблицей — информационная строка
|
||||
«Загружено X из N префиксов — уточните поиск или выберите VRF» (стиль `.info`).
|
||||
4. Бэкенд не меняется.
|
||||
5. `README.md`, раздел «Поведение таблиц UI»: одна фраза о загрузке всех страниц и пределе 20 000.
|
||||
|
||||
## Файлы
|
||||
`web/app.js`, `README.md`.
|
||||
|
||||
## Проверка
|
||||
`node --check web/app.js`; на стенде временная организация с 1 100 префиксами (`/30` через `POST …/subnets/next`) — в UI заголовок «1 101 префикс», все строки доступны.
|
||||
Данные после проверки удалить.
|
||||
@@ -0,0 +1,27 @@
|
||||
# Итог: экран «Префиксы» без молчаливого усечения (изменение 028)
|
||||
|
||||
## Что сделано
|
||||
- `web/app.js`: добавлена функция `loadPrefixes(orgId)` — последовательно догружает все страницы `/prefixes` (`limit=1000`, `offset` увеличивается на длину полученной страницы) до `total`,
|
||||
но не больше `PREFIX_UI_CAP = 20000` префиксов; возвращает `{ items, total }`.
|
||||
- `screens.prefixes` и `screens.address` используют `loadPrefixes` вместо одиночного запроса с `limit=1000`.
|
||||
- Заголовок («N префиксов»), вкладка «Все» и подвал таблицы теперь используют `pl.total` (реальное общее число), а не `all.length` (число загруженных).
|
||||
- Если сработал предел `PREFIX_UI_CAP` (`items.length < total`), над таблицей выводится информационная строка `.info`: «Загружено X из N префиксов — уточните поиск или выберите VRF».
|
||||
- Бэкенд не менялся.
|
||||
- `README.md`, раздел «Поведение таблиц UI»: добавлено предложение о постраничной догрузке и пределе 20 000.
|
||||
|
||||
## Отклонения от плана
|
||||
Нет.
|
||||
|
||||
## Как проверено
|
||||
1. `node --check web/app.js` — без ошибок.
|
||||
2. Сценарий из плана воспроизведён на API-уровне (браузер недоступен в этой среде, поэтому визуальный рендер заголовка не смотрели глазами, но проверили именно тот запрос/расчёт,
|
||||
который его формирует): временная организация `rv3-page-test` (scratchpad-скрипт), родитель `10.0.0.0/12`, 1100 вложенных `/30` через `POST …/subnets/next` (по одному, как в плане) —
|
||||
всего **1101 префикс**. Затем эмуляция ровно алгоритма `loadPrefixes` (`limit=1000`, `offset += len(page.items)`, до `total`) через прямые вызовы API:
|
||||
- `total=1101`, загружено `1101` элементов за **2 страницы** (1000 + 101) — соответствует «в UI заголовок «1 101 префикс», все строки доступны» из плана. **PASS.**
|
||||
3. Удаление 1100 вложенных префиксов заняло ~32 с, само создание — ~100 с (растёт с числом уже выделенных блоков в одном родителе — ожидаемо, это не предмет изменения 028).
|
||||
Временная организация и все её префиксы удалены по завершении (`org delete: 204`).
|
||||
|
||||
## Не проверено
|
||||
- Собственно визуальный рендер (браузер недоступен в этой среде): заголовок, строка-предупреждение `.info` при срабатывании `PREFIX_UI_CAP`, поведение вкладок и подвала на живой странице.
|
||||
Логика, формирующая эти значения (`pl.total`, `truncated`), проверена чтением кода и совпадает с тем, что подтверждено на уровне API.
|
||||
- Собственно предел `PREFIX_UI_CAP = 20000` (сценарий с >20 000 префиксов) — не воспроизводился (потребовал бы кратно больше времени на создание тестовых данных).
|
||||
@@ -0,0 +1,25 @@
|
||||
# Целостность дерева префиксов при параллельных изменениях (изменение 029)
|
||||
|
||||
Находка № 5 из `docs/reviews/2026-09-26-codebase-review-2.md`, серьёзность — низкая.
|
||||
|
||||
## Context
|
||||
`attach_to_tree` определяет родителя и забирает вложенные префиксы по снимку данных транзакции. Два параллельных запроса могут создать в одном VRF пересекающиеся префиксы разной длины
|
||||
(ручное создание `.0/29` и автовыделение `.4/30`). Уникальность `(vrf_id, prefix)` это не ловит, `parent_id` останется неверным. `FOR UPDATE` в `allocate_subnet` блокирует только строку родителя.
|
||||
|
||||
## Решение (`app/api/v1/prefixes.py`)
|
||||
1. Хелпер `_lock_vrf(db, *vrf_ids)`: `pg_advisory_xact_lock(<VRF_NS>, vrf_id)` для каждого VRF **в порядке возрастания id** (исключает взаимную блокировку); пространство ключей — константа (например, 7033).
|
||||
2. Вызов в начале каждой операции, меняющей дерево или раскладку адресов, до любых чтений дерева:
|
||||
- `create_prefix` — VRF из тела запроса;
|
||||
- `allocate_subnet` — VRF родителя. Сначала прочитать `vrf_id` родителя обычным `SELECT`, затем `_lock_vrf`, затем уже существующий `SELECT … FOR UPDATE`;
|
||||
- `update_prefix` при смене VRF (`_move_to_vrf`) — исходный и целевой VRF;
|
||||
- `delete_prefix` — VRF префикса (переподвешивание детей);
|
||||
- `create_address` и `allocate_next` — VRF префикса (выбор самого узкого префикса и занятых диапазонов должен видеть согласованное дерево).
|
||||
3. Операции над разными VRF не блокируют друг друга. Чтения (`GET`) не блокируются.
|
||||
4. `README.md`: одна фраза в разделе «Модель данных» — изменения дерева одного VRF выполняются по одному.
|
||||
|
||||
## Файлы
|
||||
`app/api/v1/prefixes.py`, `README.md`.
|
||||
|
||||
## Проверка
|
||||
Параллельно (потоки) в одном VRF: создание `.0/29` и автовыделение `/30` из `/24` → у выделенного `/30`, если он внутри `/29`, родитель — `/29`.
|
||||
Цикл из 20 повторов без ошибок и без неверных родителей. Данные после проверки удалить.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Итог: целостность дерева префиксов при параллельных изменениях (изменение 029)
|
||||
|
||||
## Что сделано
|
||||
- `app/api/v1/prefixes.py`: константа `VRF_LOCK_NS = 7033` и хелпер `_lock_vrf(db, *vrf_ids)` — `pg_advisory_xact_lock(VRF_LOCK_NS, vrf_id)` для каждого VRF, отсортированных
|
||||
по возрастанию id (исключает взаимную блокировку при операциях над несколькими VRF).
|
||||
- Вызов добавлен в начале каждой операции, меняющей дерево или раскладку адресов, до любых чтений дерева:
|
||||
- `create_prefix` — по `body.vrf_id`, первой строкой обработчика;
|
||||
- `allocate_subnet` — сначала обычный `SELECT vrf_id` префикса-родителя (без блокировки строки), затем `_lock_vrf`, затем уже существующий `SELECT … FOR UPDATE`;
|
||||
- `update_prefix` при смене VRF — по исходному и целевому VRF, до вызова `_move_to_vrf`;
|
||||
- `delete_prefix` — по VRF префикса, до переподвешивания детей;
|
||||
- `create_address` — по VRF префикса, до проверки «самый узкий префикс»;
|
||||
- `allocate_next` — обычный `SELECT vrf_id`, затем `_lock_vrf`, затем существующий `SELECT … FOR UPDATE` по префиксу.
|
||||
- Чтения (`GET`) не блокируются; операции над разными VRF друг друга не блокируют (независимые ключи advisory-lock).
|
||||
- `README.md`, раздел «Модель данных»: одно предложение о сериализации изменений дерева одного VRF.
|
||||
|
||||
## Отклонения от плана
|
||||
Нет.
|
||||
|
||||
## Как проверено
|
||||
1. `venv/bin/python -c 'import app.main'` — без ошибок.
|
||||
2. Стенд пересобран, `healthy`.
|
||||
3. Сценарий из плана (20 повторов, временная организация `rv3-tree-test`, скрипт в scratchpad): в каждой итерации — свой `/24`-родитель, и **параллельно** (два потока) —
|
||||
явное создание `.0/29` и автовыделение `/30` из родителя (`POST …/subnets/next {length:30}`). Оба запроса каждый раз завершились без ошибок (0 ошибок из 40 запросов).
|
||||
Итоговое состояние дерева (отдельный `GET` **после** завершения обоих запросов пары — не тело ответа гонки, оно отражает лишь момент собственного commit) проверено на всех
|
||||
20 итерациях: во всех случаях, когда выделенный `/30` оказался внутри `.0/29`, его `parent_id` равен id `.0/29`, а не `.0/24`. **0 из 20 неверных `parent_id`. PASS.**
|
||||
- Первый прогон теста дал 5 ложных срабатываний из-за ошибки самого теста (сверка велась по телу HTTP-ответа каждого из двух конкурентных запросов, которое отражает
|
||||
состояние дерева на момент commit именно этого запроса, а не финальное состояние после обоих); после исправления теста (повторный `GET` уже обоих префиксов после
|
||||
завершения гонки) все 20 итераций прошли успешно, а прямая проверка в БД (`SELECT … FROM prefixes`) подтвердила, что для всех 5 «проблемных» по первому прогону
|
||||
итераций конечное состояние в базе уже было верным (гонка не оставляла ошибочных данных, ошибался только клиентский тест).
|
||||
4. Временная организация и все её префиксы удалены по завершении.
|
||||
|
||||
## Не проверено
|
||||
- Гонка при `update_prefix` (смена VRF, две блокировки) и при одновременном `create_address`/`allocate_next` в одном VRF — проверялась только комбинация
|
||||
`create_prefix` + `allocate_subnet`, явно описанная в плане; остальные точки вызова `_lock_vrf` проверены только чтением кода.
|
||||
- Поведение под большей степенью параллелизма (более двух одновременных запросов) и при конкурентных операциях над разными VRF (что они не блокируют друг друга) — не измерялось.
|
||||
@@ -0,0 +1,44 @@
|
||||
# Исправление замечаний 1–2 ревью изменений 025–029 (изменение 030)
|
||||
|
||||
Источник: `docs/reviews/2026-09-26-changes-025-029-review.md`, замечания № 1 и № 2. Только код. Тесты исполнитель не пишет и не запускает (тестирование — отдельный шаг).
|
||||
|
||||
## 1. Чтение префикса до блокировки VRF (низкая, 029)
|
||||
**Где:** `app/api/v1/prefixes.py` — `delete_prefix`, `update_prefix` (при смене VRF), `create_address`.
|
||||
**Суть:** префикс читается через `get_or_404` **до** `_lock_vrf`. Возникают две проблемы:
|
||||
- в `delete_prefix` устаревший `p.parent_id` используется для переподвешивания детей;
|
||||
- если префикс параллельно переносят в другой VRF, блокируется не тот VRF (`p.vrf_id` прочитан до блокировки).
|
||||
|
||||
**Решение:**
|
||||
1. Хелпер `_lock_prefix(db, id: int, *extra_vrf_ids: int) -> Prefix` рядом с `_lock_vrf`:
|
||||
- прочитать `vrf_id` префикса отдельным `SELECT Prefix.vrf_id`; нет строки — 404 «Префикс не найден»;
|
||||
- `_lock_vrf(db, vrf_id, *extra_vrf_ids)` — все VRF одним вызовом, порядок по возрастанию id уже обеспечен `_lock_vrf`;
|
||||
- перечитать префикс под блокировкой: `select(Prefix).where(Prefix.id == id).with_for_update().execution_options(populate_existing=True)`,
|
||||
чтобы не взять устаревший объект из identity map сессии;
|
||||
- если перечитанный `vrf_id` отличается от заблокированного (префикс успели перенести), повторить цикл, не больше 3 попыток, затем 409 «Префикс одновременно изменяется, повторите запрос».
|
||||
Блокировки транзакции при повторе не снимаются, это допустимо: порядок захвата внутри одной попытки по-прежнему возрастающий.
|
||||
Если «лишняя» блокировка ухудшает порядок между попытками, допустимо вместо повтора сразу возвращать 409.
|
||||
Выбрать вариант и указать его в SUMMARY.
|
||||
2. Применение:
|
||||
- `delete_prefix`: `p = _lock_prefix(db, id)` вместо `get_or_404` + `_lock_vrf`;
|
||||
- `create_address`: то же;
|
||||
- `update_prefix`: целевой VRF (`body.vrf_id`) известен из тела запроса до чтения префикса, поэтому `p = _lock_prefix(db, id, new_vrf)` при заданном `vrf_id`, иначе обычный `get_or_404` (без смены VRF дерево не меняется).
|
||||
Если `new_vrf` совпал с текущим `p.vrf_id`, лишняя блокировка того же VRF безвредна.
|
||||
- `allocate_subnet` и `allocate_next` уже читают `vrf_id` до блокировки; перевести их на `_lock_prefix` для единообразия, сохранив поведение (`FOR UPDATE` строки префикса есть в хелпере).
|
||||
3. Комментарии — кратко, со ссылкой на изменение 030.
|
||||
|
||||
## 2. Зависимость ротации от API-слоя (низкая, 026)
|
||||
**Где:** `app/rotation.py` импортирует `KNOWN_IP_DAYS` из `app.api.v1.auth`.
|
||||
**Решение:**
|
||||
1. Перенести константы политики входа — `LOGIN_WINDOW` (сейчас `WINDOW`), `MAX_PER_LOGIN_IP`, `MAX_PER_IP`, `MAX_PER_LOGIN`, `KNOWN_IP_DAYS` — в `app/services.py` отдельным блоком с комментарием.
|
||||
2. `app/api/v1/auth.py` импортирует их из `app.services`; внутренние имена в `auth.py` не должны расходиться с перенесёнными. `WINDOW` заменить на `LOGIN_WINDOW` по месту, модульная docstring с `.format(...)` продолжает работать.
|
||||
3. `app/rotation.py` импортирует `KNOWN_IP_DAYS` из `app.services`; импорт `app.api.v1.auth` удалить.
|
||||
4. Пространства ключей advisory-lock (`LOGIN_LOCK_NS`, `IP_LOCK_NS`) остаются в `auth.py`.
|
||||
|
||||
## Артефакты
|
||||
- `docs/changes/030-review-fixes-025-029/SUMMARY.md`: что сделано, выбранный вариант повтора из п. 1, отклонения.
|
||||
- `README.md` — только если меняется описанное поведение (ожидается, что не меняется).
|
||||
|
||||
## Проверка (выполняет ревьюер, не исполнитель)
|
||||
- Импорт приложения; `rotation.py` не импортирует API-модули.
|
||||
- Параллельные сценарии по п. 1: удаление префикса ∥ вставка промежуточного префикса; перенос префикса в другой VRF ∥ создание адреса / удаление.
|
||||
- Регрессия: `pytest -q`.
|
||||
@@ -0,0 +1,81 @@
|
||||
# Итог: исправление замечаний 1–2 ревью изменений 025–029 (изменение 030)
|
||||
|
||||
Источник: `docs/reviews/2026-09-26-changes-025-029-review.md`, замечания № 1 и № 2. Только код, без тестов.
|
||||
|
||||
## 1. Чтение префикса до блокировки VRF (`app/api/v1/prefixes.py`)
|
||||
|
||||
- Добавлен хелпер `_lock_prefix(db, id, *extra_vrf_ids) -> Prefix` рядом с `_lock_vrf`:
|
||||
1. `vrf_id` читается отдельным `SELECT Prefix.vrf_id` (без блокировки строки); нет строки — 404 «Префикс не найден».
|
||||
2. `_lock_vrf(db, vrf_id, *extra_vrf_ids)` — все нужные VRF одним вызовом, в возрастающем порядке (как раньше).
|
||||
3. Префикс перечитывается под блокировкой: `select(Prefix).where(Prefix.id == id).with_for_update().execution_options(populate_existing=True)` —
|
||||
`populate_existing`, чтобы не получить устаревший объект из identity map сессии.
|
||||
4. Если перечитанный `vrf_id` не совпал с заблокированным (префикс успели перенести в другой VRF между шагами 1 и 3) — `409 «Префикс одновременно изменяется, повторите запрос»`.
|
||||
|
||||
- Применение:
|
||||
- `delete_prefix`: `p = _lock_prefix(db, id)` вместо `get_or_404` + `_lock_vrf(db, p.vrf_id)`.
|
||||
- `create_address`: то же самое.
|
||||
- `update_prefix`: `new_vrf` вычисляется из тела запроса **до** чтения префикса (`data.pop("vrf_id", None)`); дальше
|
||||
`p = _lock_prefix(db, id, new_vrf) if new_vrf is not None else get_or_404(db, Prefix, id, "Префикс")`. Если целевой VRF совпадает
|
||||
с текущим, лишняя (повторная) блокировка того же VRF безвредна — как и предполагал план.
|
||||
- `allocate_subnet`, `allocate_next`: переведены на `_lock_prefix` для единообразия. Прежний код читал `vrf_id` отдельным `SELECT`,
|
||||
затем `_lock_vrf`, затем `SELECT … FOR UPDATE` без сверки `vrf_id` после блокировки — сейчас эта сверка выполняется хелпером,
|
||||
т.е. поведение не просто перенесено, а дополнительно защищено той же гарантией, что и остальные точки.
|
||||
|
||||
### Выбранный вариант обработки гонки — без повтора, сразу 409
|
||||
|
||||
В плане предлагалось два варианта: (а) до 3 попыток внутри `_lock_prefix`, не снимая блокировки между попытками, либо
|
||||
(б) сразу отдавать 409 без повтора. Выбран вариант **(б)**.
|
||||
|
||||
Причина: между двумя попытками цикла блокировка устаревшего (первого прочитанного) `vrf_id` из предыдущей попытки не снимается
|
||||
(advisory-lock держится до конца транзакции), а на следующей попытке блокируется уже новый `vrf_id`. `_lock_vrf` сортирует по
|
||||
возрастанию только ids **внутри одного своего вызова** — порядок между накопленными за разные попытки блокировками этой
|
||||
гарантии не имеет. Если два конкурентных запроса одновременно переносят префиксы «навстречу» друг другу (транзакция A: сначала
|
||||
видит VRF 5, после гонки — VRF 3; транзакция B — в обратном порядке), возможна ситуация, когда A держит блокировку VRF 5 и
|
||||
запрашивает VRF 3, а B держит VRF 3 и запрашивает VRF 5 — классический deadlock из-за несогласованного порядка захвата между
|
||||
попытками. Однократная попытка с немедленным 409 гарантированно не накапливает блокировки разных VRF за пределами одного
|
||||
согласованного вызова `_lock_vrf`, поэтому этот риск не возникает. Транзакция откатывается при закрытии сессии (as-is для всех
|
||||
остальных `HTTPException` в этом модуле — см., например, комментарий у `_move_to_vrf`), и клиент просто повторяет весь HTTP-запрос
|
||||
с чистого листа, без унаследованных блокировок.
|
||||
|
||||
Это разумная цена: окно гонки узкое (перенос префикса в другой VRF — редкая административная операция), а 3 внутренние попытки
|
||||
не устраняют риск deadlock — они его создают.
|
||||
|
||||
## 2. Зависимость ротации от API-слоя
|
||||
|
||||
- `app/services.py`: добавлен блок констант политики входа (после `MAX_CAPACITY`/`MAX_OFFSET`, с комментарием):
|
||||
`LOGIN_WINDOW` (бывший `WINDOW`), `MAX_PER_LOGIN_IP`, `MAX_PER_IP`, `MAX_PER_LOGIN`, `KNOWN_IP_DAYS`. Добавлен импорт `from datetime import timedelta`.
|
||||
- `app/api/v1/auth.py`: собственные определения констант удалены, всё импортируется из `app.services`. `WINDOW` заменён на
|
||||
`LOGIN_WINDOW` по всем местам использования (`_retry_after`, `_distinct_logins`, форматирование модульной docstring). Внутренние
|
||||
имена не разошлись с перенесёнными. `LOGIN_LOCK_NS`/`IP_LOCK_NS` (пространства advisory-lock) остались в `auth.py`, как и планировалось.
|
||||
- `app/rotation.py`: `from app.api.v1.auth import KNOWN_IP_DAYS` заменён на `from app.services import KNOWN_IP_DAYS, SYSTEM, audit`.
|
||||
Модуль больше не импортирует ничего из `app.api.*`.
|
||||
|
||||
## Отклонения от плана
|
||||
|
||||
- П. 1: выбран вариант «сразу 409» вместо цикла до 3 попыток — обоснование выше (план явно допускал оба варианта и просил
|
||||
зафиксировать выбор в SUMMARY).
|
||||
- В остальном реализация соответствует плану без отклонений.
|
||||
|
||||
## Изменённые файлы
|
||||
|
||||
- `app/api/v1/prefixes.py` — хелпер `_lock_prefix`, применение в `delete_prefix`, `create_address`, `update_prefix`, `allocate_subnet`, `allocate_next`.
|
||||
- `app/services.py` — константы политики входа.
|
||||
- `app/api/v1/auth.py` — импорт констант из `app.services` вместо локальных определений.
|
||||
- `app/rotation.py` — импорт `KNOWN_IP_DAYS` из `app.services` вместо `app.api.v1.auth`.
|
||||
|
||||
`README.md` не менялся: наблюдаемое поведение API не меняется (кроме нового 409 при воспроизведении узкой гонки переноса VRF,
|
||||
который относится к тому же классу конкурентных ошибок, что уже описан в README для изменения 029).
|
||||
|
||||
## Не проверено
|
||||
|
||||
Всё поведение — по правилам задачи, автор изменения тесты не пишет и не запускает. Не проверялось (проверяет ревьюер):
|
||||
- Импорт приложения проверен только `import app.main`; `pytest -q` не запускался.
|
||||
- Параллельные сценарии из плана: удаление префикса ∥ вставка промежуточного префикса; перенос префикса в другой VRF ∥
|
||||
создание адреса / удаление — не воспроизводились ни через API, ни вручную.
|
||||
- Что `_lock_prefix` действительно возвращает 409 при воспроизведённой гонке (реальный `vrf_id` mismatch под нагрузкой).
|
||||
- Поведение `populate_existing=True` в связке с уже загруженными объектами `Prefix` в сессии (в текущих точках вызова такой
|
||||
объект до `_lock_prefix` не загружается, поэтому эффект защитный, но не критичен для текущих путей вызова).
|
||||
- Корректность обновлённой модульной docstring `auth.py` после форматирования (`__doc__.format(...)`) — визуально не отличается
|
||||
от прежней, но не выводилась и не сравнивалась построчно.
|
||||
- `rotation.py`: что при импорте больше не вычисляется фиктивный argon2-хэш из `auth.py` (косвенный эффект удаления импорта,
|
||||
отдельно не измерялся).
|
||||
@@ -0,0 +1,71 @@
|
||||
# Ревью и тестирование изменений 025–029 — 2026-09-26
|
||||
|
||||
**Объём:** незакоммиченные изменения поверх `13e17fb`, выполненные агентом (Sonnet 5) по планам `docs/changes/025…029`.
|
||||
Изменены `app/api/v1/{auth,overview,prefixes}.py`, `app/models.py`, `app/rotation.py`, `web/app.js`, `README.md`; добавлена миграция `0009_known_logins`.
|
||||
|
||||
**Метод:**
|
||||
- чтение diff;
|
||||
- автотесты проекта;
|
||||
- собственные сквозные сценарии на стенде `ipam_control_006`: API, прямые вставки в БД для эмуляции других IP, headless Chromium для UI. Скрипты лежат во временной папке, в репозиторий не добавлены. Временные данные удалены.
|
||||
|
||||
## Итог
|
||||
|
||||
Все пять изменений реализованы по планам и работают.
|
||||
|
||||
| Проверка | Результат |
|
||||
|---|---|
|
||||
| `import app.main`, `node --check web/app.js` | ✅ |
|
||||
| Стенд: `app` и `db` healthy, миграция `0009`, `alembic check` | ✅ расхождений нет |
|
||||
| `pytest -q` | ✅ **14 passed**, после приведения одной проверки к семантике 025 (см. ниже) |
|
||||
| Собственные сценарии 025–029 | ✅ **14/14** |
|
||||
|
||||
**Правка тестов.** `test_tree_utilization_and_next_free` проверял прежнее правило «ёмкость родителя = сумма листьев» (`capacity == 6`), которое изменение 025 намеренно отменило. Проверка заменена на `capacity == 65534` (размер `/16`), `used == 2`. Агенту трогать тесты было запрещено, поэтому до этой правки тест падал ожидаемо.
|
||||
|
||||
## Результаты сценариев
|
||||
|
||||
| Изм. | Сценарий | Результат |
|
||||
|---|---|---|
|
||||
| 025 | `/24` с 40 адресами, затем автовыделение `/30`: у родителя `40 / 254 / 16 %` (раньше было `40 / 2 / 100 %`) | ✅ |
|
||||
| 025 | Ёмкость родителя совпадает с `summary.capacity` экрана адресов | ✅ |
|
||||
| 025 | «Обзор» совпадает с независимым SQL-расчётом: ёмкость — сумма корневых активных IPv4-префиксов, «назначено» — адреса внутри корней | ✅ |
|
||||
| 026 | Успешный вход добавляет IP в `known_logins` | ✅ |
|
||||
| 026 | 5 неудач с чужого IP не блокируют вход владельца с его IP | ✅ |
|
||||
| 026 | 55 неудач по логину с разных IP: вход с известного IP — 200, с неизвестного — 429 | ✅ |
|
||||
| 026 | 5 неудач с одного IP: 401×4, затем 429; верный пароль с этого IP тоже 429; `session.locked` с `scope=login_ip` | ✅ |
|
||||
| 027 | 15 накопленных неудач IP, затем 16 параллельных входов на разные логины: проверено ≤ 5 паролей, остальные 429 | ✅ |
|
||||
| 028 | Организация с 1 101 префиксом: UI показывает «1101 префикс» (total), предупреждения об усечении нет, ошибок в консоли нет | ✅ |
|
||||
| 029 | 20 гонок «ручной `/29` ∥ автовыделение `/30`» в одном `/24` (15 раз первым выполнился ручной, 5 — автоматический): у всех префиксов родитель — самый узкий охватывающий, 0 расхождений | ✅ |
|
||||
|
||||
Замечание о методике. В первом прогоне сценарий 029 давал ложные «FAIL»: эталон ожидал `.4/30`, а при ручном `/29` первым автовыделение законно выбирает `.8/30`. Сценарий переписан на проверку инварианта дерева (родитель — самый узкий охватывающий префикс). Той же причины касается замечание агента о «5 ложных срабатываниях».
|
||||
|
||||
## Замечания ревью
|
||||
|
||||
| # | Серьёзность | Изм. | Суть |
|
||||
|---|---|---|---|
|
||||
| 1 | Низкая | 029 | `delete_prefix`, `update_prefix` и `create_address` читают префикс **до** блокировки VRF 🔎 |
|
||||
| 2 | Низкая | 026 | `app/rotation.py` импортирует константу из API-модуля (`app.api.v1.auth.KNOWN_IP_DAYS`) 🔎 |
|
||||
| 3 | Инфо | 028 | Экран адресов загружает все префиксы организации, до 20 000, ради крошек и диалогов 🔎 |
|
||||
| 4 | Инфо | 026 | Компромисс политики: распределённый перебор получает до 5 попыток на IP 🔎 |
|
||||
|
||||
### 1. Чтение префикса до блокировки VRF (029)
|
||||
- В `delete_prefix` `p.parent_id` читается до `_lock_vrf`. Если параллельно между `p` и его родителем создан промежуточный префикс, дочерние элементы `p` при удалении будут переподвешены к устаревшему родителю.
|
||||
- В `update_prefix` и `create_address` блокировка берётся по `vrf_id`, прочитанному до неё. Если в это время префикс переносят в другой VRF, будет заблокирован не тот VRF.
|
||||
|
||||
Окно гонки узкое. **Исправление:** как в `allocate_subnet`, сначала `SELECT vrf_id`, затем `_lock_vrf`, затем чтение префикса. Другой вариант — `db.refresh(p)` сразу после блокировки.
|
||||
|
||||
### 2. Зависимость ротации от API-слоя (026)
|
||||
`rotation.py` → `app.api.v1.auth`: служебный модуль зависит от роутера, а при импорте ротации вычисляется фиктивный argon2-хэш. Циклического импорта сейчас нет, но связь хрупкая.
|
||||
**Исправление:** перенести `KNOWN_IP_DAYS` и пороги входа в `app/services.py` или `app/config.py`.
|
||||
|
||||
### 3. Объём загрузки на экране адресов (028)
|
||||
`screens.address` вызывает `loadPrefixes`, то есть загружает до 20 страниц по 1 000 префиксов, причём каждая страница на сервере пересчитывает глубины (`_depths`) по всей организации. Для крупной организации открытие каждого экрана адресов станет заметно медленнее.
|
||||
**Рекомендация (отдельной задачей):**
|
||||
- эндпоинт цепочки предков для хлебных крошек;
|
||||
- загрузка префиксов VRF для диалогов по требованию, при открытии окна.
|
||||
|
||||
### 4. Компромисс политики блокировки (026)
|
||||
После 026 перебор с N разных IP даёт до 5 попыток на каждый IP и до 50 в сумме по логину для неизвестных IP. До 026 общий лимит был 5 на логин. Это осознанная цена за то, что владельца больше нельзя заблокировать анонимно.
|
||||
Лимит по IP (20) по-прежнему позволяет заблокировать вход всем пользователям за общим NAT. Если это существенно, можно исключать из лимита по IP известные пары «логин + IP».
|
||||
|
||||
## Вывод
|
||||
Замечания 1–2 можно закрыть небольшой правкой, 3–4 — по желанию, отдельными задачами. Изменения 025–029 вместе с правкой теста готовы к коммиту.
|
||||
@@ -0,0 +1,116 @@
|
||||
# Повторный анализ кодовой базы IPAM Manager — 2026-09-26
|
||||
|
||||
**Состояние:** коммит `13e17fb` (задачи 001–024). Объём: `app/` и `web/app.js` — около 3 400 строк, миграции 0001–0008.
|
||||
**Метод:** чтение кода с упором на участки, которые изменения 010–024 затронули или усилили. Подозрительные места проверены на стенде `ipam_control_006` на временных данных, после проверки данные удалены.
|
||||
**Предыдущие отчёты:** `2026-09-26-codebase-review.md` (13 находок) и `2026-09-26-changes-011-023-review.md` (7 находок).
|
||||
|
||||
Пометки: ✅ — воспроизведено на стенде, 🔎 — вывод по коду.
|
||||
|
||||
## Итог
|
||||
|
||||
Все 13 находок первого ревью закрыты, остаточные замечания указаны ниже. Все 7 находок ревью изменений 011–023 закрыты изменением 024.
|
||||
Состояние системы:
|
||||
- `alembic check` без расхождений;
|
||||
- 14 тестов проходят;
|
||||
- контейнер работает от непривилегированного пользователя и в статусе `healthy`;
|
||||
- UI работает под CSP без ошибок в консоли.
|
||||
|
||||
Новый анализ выявил одну заметную ошибку: неверная ёмкость и загрузка частично разбитого префикса (№ 1). Её усиливает сценарий автовыделения подсетей из изменения 010. Кроме неё — один риск доступности (№ 2) и несколько низких замечаний.
|
||||
|
||||
| # | Серьёзность | Область | Кратко |
|
||||
|---|---|---|---|
|
||||
| 1 | Средняя | Данные / отображение | Ёмкость родителя равна сумме ёмкостей листьев: после выделения одного `/30` у `/24` ёмкость 254 → 2, загрузка 16 % → 100 % ✅ |
|
||||
| 2 | Средняя | Доступность | Анонимный клиент может держать администратора заблокированным: 5 запросов каждые 10 минут, верный пароль тоже отклоняется ✅ |
|
||||
| 3 | Низкая | UI | Экран «Префиксы» загружает не больше 1 000 префиксов и показывает число загруженных, а не `total`; дерево молча усекается 🔎 |
|
||||
| 4 | Низкая | Тесты / эксплуатация | Тесты работают с БД рабочего стенда: создают данные и сбрасывают настройки журнала на значения по умолчанию, а не на исходные 🔎 |
|
||||
| 5 | Низкая | Конкурентность | Дерево префиксов (`parent_id`) поддерживается только кодом: параллельное создание пересекающихся префиксов может оставить неверного родителя 🔎 |
|
||||
| 6 | Низкая | API | Явный `parent_id` при создании префикса может указать не самого узкого родителя, и дерево разойдётся с вложенностью CIDR 🔎 |
|
||||
| 7 | Низкая | Безопасность | Лимит в 20 попыток по IP не сериализуется между разными логинами (остаточное замечание 024) 🔎 |
|
||||
| 8 | Инфо | API | `PATCH /organizations/{id}` требует полное тело (`OrgIn`), частичное обновление даёт 422 🔎 |
|
||||
| 9 | Инфо | Эксплуатация | Быстрый старт в README (`docker compose up`) поднимает проект `ipam_control`, а рабочий стенд — `ipam_control_006`; легко перепутать поставки 🔎 |
|
||||
|
||||
---
|
||||
|
||||
## Находки
|
||||
|
||||
### 1. Ёмкость и загрузка частично разбитого префикса ✅
|
||||
`_capacities` (`app/api/v1/prefixes.py`) считает ёмкость родителя как сумму ёмкостей листьев, а `used` (`_usage`) — по всем адресам поддерева, включая адреса самого родителя вне дочерних префиксов. После изменения 010 («Добавить вложенный (авто)») частичное разбиение стало обычным сценарием, и расхождение проявляется сразу.
|
||||
**Воспроизведение:** `172.20.0.0/24` с 40 адресами — `used 40 / capacity 254 / 16 %`. После автовыделения одного `/30` — `used 40 / capacity 2 / 100 %`. Экран адресов того же префикса по-прежнему показывает ёмкость 254.
|
||||
Та же причина искажает «Обзор»: `assigned` считается по всем адресам IPv4, а `capacity` — только по листьям. Адреса в нелистовых префиксах завышают загрузку.
|
||||
**Рекомендация:**
|
||||
- ёмкость любого префикса — размер его собственной подсети (`capacity(prefix)`);
|
||||
- на «Обзоре» ёмкость — сумма по корневым префиксам (без родителя) каждого VRF;
|
||||
- `used` оставить как есть: считается по поддереву, дублей нет благодаря изменению 011.
|
||||
|
||||
Так числа станут согласованы с экраном адресов.
|
||||
|
||||
### 2. Блокировка входа администратора анонимным клиентом ✅
|
||||
Лимит по логину (5 неудач за 10 минут, изменение 012) действует для всех IP. Верный пароль во время блокировки тоже отклоняется. Проверено при сверке 012: 429 и `Retry-After` на верный пароль.
|
||||
Любой, кто знает логин (например, `admin` из README), может отправлять 5 неверных попыток раз в 10 минут и держать администратора заблокированным. Риск был отмечен в плане 012; ниже вариант смягчения.
|
||||
**Рекомендация:**
|
||||
- блокировать по паре «логин + IP» (5 попыток), а общий по логину порог сделать выше (например, 50) и рассматривать его как сигнал в журнале;
|
||||
- альтернатива: не блокировать вход с IP, с которого этот пользователь успешно входил за последние N дней.
|
||||
|
||||
### 3. Усечение списка префиксов в UI 🔎
|
||||
`screens.prefixes` и экран адресов запрашивают `/prefixes?limit=1000`, а заголовок, вкладки и подвал показывают `all.length`, то есть число загруженных префиксов, а не `total`. Родительский список в «Новом префиксе» и умолчание размера в «Добавить вложенный» тоже строятся по усечённому набору.
|
||||
Организация, у которой `/22` разбит на `/30` (256 префиксов), быстро выходит за 1 000.
|
||||
**Рекомендация:**
|
||||
- минимум: показывать `total` и предупреждение «загружено 1 000 из N»;
|
||||
- лучше: догружать страницы (`offset`) или получать дерево для конкретного VRF и ветки.
|
||||
|
||||
### 4. Тесты работают с данными стенда 🔎
|
||||
`tests/conftest.py` работает с API и БД из `.env`, то есть со стендом `ipam_control_006`, где лежат демо-данные.
|
||||
- `test_rotation_by_age_and_count` в `finally` выставляет `retention_days=90, max_entries=100000` вместо значений, которые были до теста. Пользовательские настройки ротации теряются.
|
||||
- Остатки неудачных прогонов (организации `test-*`) видны в UI.
|
||||
|
||||
**Рекомендация:**
|
||||
- отдельный compose-проект или БД для тестов (например, `docker compose -p ipam_test` с другим `.env`);
|
||||
- в тесте ротации сохранять и восстанавливать исходные настройки.
|
||||
|
||||
### 5. Дерево префиксов при параллельных изменениях 🔎
|
||||
`attach_to_tree` определяет родителя и забирает вложенные префиксы по снимку данных в транзакции. Два параллельных запроса могут создать в одном VRF пересекающиеся префиксы разной длины (ручное создание `.0/29` и автовыделение `.4/30`). Уникальность `(vrf_id, prefix)` это не ловит, и `parent_id` останется неверным.
|
||||
`FOR UPDATE` в `allocate_subnet` блокирует только строку родителя.
|
||||
**Рекомендация:** в `create_prefix`, `allocate_subnet` и `_move_to_vrf` брать `pg_advisory_xact_lock` по `vrf_id`: изменения дерева одного VRF выполняются по одному. Дешёвый и полный вариант.
|
||||
|
||||
### 6. Явный `parent_id` против вложенности CIDR 🔎
|
||||
`create_prefix` с `parent_id` (`keep_parent=True`) проверяет только, что новый префикс входит в указанный родитель. Если существует более узкий префикс, который тоже содержит новый, дерево будет указывать на более широкого родителя. `_capacities` и `_depths` работают по `parent_id`, `_usage` — по вложенности CIDR, и эти расчёты разойдутся.
|
||||
**Рекомендация:** если указан не самый узкий родитель, возвращать 422 или игнорировать `parent_id` и вычислять родителя автоматически, как без параметра.
|
||||
|
||||
### 7. Лимит по IP между разными логинами 🔎
|
||||
Сериализация из изменения 024 работает по логину. Параллельный перебор многих разных логинов с одного IP проходит проверку лимита по IP одновременно и может превысить 20 попыток на степень параллелизма.
|
||||
**Рекомендация:** вторая advisory-блокировка по `hashtext(ip)`, брать её после блокировки логина, всегда в одном порядке.
|
||||
|
||||
### 8. PATCH организации требует полное тело 🔎
|
||||
`update_org` принимает `OrgIn` (обязательные `name` и `inn`), поэтому `PATCH` с одним полем возвращает 422. UI отправляет полную форму, так что пользователи этого не замечают, но семантика PATCH нарушена.
|
||||
**Рекомендация:** схема `OrgUpdate` с необязательными полями и `exclude_unset`, как у остальных PATCH.
|
||||
|
||||
### 9. Две поставки стенда 🔎
|
||||
В README быстрый старт использует `docker compose up -d --build`, то есть проект `ipam_control`: он собирает старые контейнеры `ipam_control-*` на тех же портах. Актуальный стенд поднят как `docker compose -p ipam_control_006`.
|
||||
**Рекомендация:** задать имя проекта в `docker-compose.yml` (`name: ipam_control`) и перенести стенд на него, либо описать в README, какой проект рабочий.
|
||||
|
||||
---
|
||||
|
||||
## Статус находок первого ревью
|
||||
|
||||
| № | Находка | Статус |
|
||||
|---|---|---|
|
||||
| 1 | Один IP дважды в VRF | ✅ закрыта (011); остаток: старые адреса сети/broadcast блокируют охватывающий префикс (на стенде 0) |
|
||||
| 2 | Перебор паролей и вытеснение журнала | ✅ закрыта (012, 024); остаток — № 2 и № 7 этого отчёта |
|
||||
| 3 | 500 на отрицательных `limit`/`offset` | ✅ закрыта (013) |
|
||||
| 4 | Ресурсоёмкий экран адресов | ✅ закрыта (014) |
|
||||
| 5 | Адрес сети/broadcast | ✅ закрыта (015, 024) |
|
||||
| 6 | Роль по умолчанию admin | ✅ закрыта (016) |
|
||||
| 7 | Встроенный JWT-секрет | ✅ закрыта (017) |
|
||||
| 8 | null в PATCH → ложный 409 | ✅ закрыта (018) |
|
||||
| 9 | N+1 | ✅ в основном закрыта (019); «Обзор» и `_prefix_outs` по-прежнему читают всё дерево организации |
|
||||
| 10 | Автоназначение и вложенные префиксы | ✅ закрыта (020) |
|
||||
| 11 | Гонки | ✅ закрыта (021); новая гонка по дереву префиксов — № 5 |
|
||||
| 12 | Эксплуатация | ✅ закрыта (022); `APP_BIND` по умолчанию `0.0.0.0` (сознательно) |
|
||||
| 13 | Мелочи | ✅ закрыта (023) |
|
||||
|
||||
## Предлагаемый порядок
|
||||
1. № 1 — ёмкость префиксов: небольшая правка в `_capacities` и `overview`, заметна пользователю сразу.
|
||||
2. № 2 и № 7 — политика блокировки входа.
|
||||
3. № 3 и № 5 — масштаб и целостность дерева префиксов.
|
||||
4. № 4 и № 9 — изоляция тестового окружения и порядок в поставках.
|
||||
5. № 6 и № 8 — по желанию.
|
||||
@@ -0,0 +1,47 @@
|
||||
# Ревью и тестирование изменения 030 — 2026-09-27
|
||||
|
||||
**Объём:** правки агента (Sonnet 5) по плану `docs/changes/030-review-fixes-025-029/PLAN.md`, которые закрывают замечания 1 и 2 ревью `2026-09-26-changes-025-029-review.md`.
|
||||
Изменены `app/api/v1/prefixes.py`, `app/api/v1/auth.py`, `app/services.py`, `app/rotation.py`.
|
||||
**Тестировались только доработки 1 и 2.** Помимо этого выполнен регрессионный прогон `pytest`.
|
||||
|
||||
## Ревью кода
|
||||
|
||||
### Замечание 1 — `_lock_prefix`
|
||||
Хелпер работает в таком порядке:
|
||||
1. `SELECT vrf_id` префикса;
|
||||
2. `_lock_vrf` для этого VRF и дополнительных (целевого VRF при переносе), одним вызовом, по возрастанию id;
|
||||
3. повторное чтение префикса под `FOR UPDATE` с `populate_existing`;
|
||||
4. если `vrf_id` к этому моменту изменился — 409 без повтора.
|
||||
|
||||
Хелпер применён в `delete_prefix`, `update_prefix` (только при смене VRF), `create_address`, `allocate_subnet`, `allocate_next`.
|
||||
|
||||
- **Порядок блокировок единый:** везде сначала advisory-блокировка VRF, затем строка префикса. `update_prefix` без смены VRF блокирует только строку (при flush) и не запрашивает блокировку VRF, поэтому цикла ожидания нет.
|
||||
- **Отказ от повтора обоснован.** Повтор оставил бы в транзакции блокировку устаревшего VRF и нарушил бы возрастающий порядок захвата между попытками. Сценарий 1d показал, что ветка 409 реально срабатывает.
|
||||
- **Встречные переносы** (A: 5→3, B: 3→5) блокируют пару VRF одним вызовом в одинаковом порядке, взаимной блокировки нет. Подтверждено сценарием 1c.
|
||||
- **Побочный эффект:** `create_address` теперь берёт `FOR UPDATE` строки префикса, и назначения адресов в одном префиксе выполняются по одному. На нагрузку это не влияет, операции короткие.
|
||||
|
||||
### Замечание 2 — константы входа
|
||||
- `LOGIN_WINDOW`, `MAX_PER_LOGIN_IP`, `MAX_PER_IP`, `MAX_PER_LOGIN`, `KNOWN_IP_DAYS` перенесены в `app/services.py`.
|
||||
- `auth.py` и `rotation.py` импортируют их оттуда. Пространства ключей advisory-блокировок остались в `auth.py`.
|
||||
- Импорт `app.rotation` больше не загружает модули `app.api.*`: проверено по `sys.modules`, список пуст. Докстринг `auth.py` форматируется корректно.
|
||||
|
||||
Новых замечаний нет.
|
||||
|
||||
## Тесты
|
||||
|
||||
| Проверка | Результат |
|
||||
|---|---|
|
||||
| Стенд `ipam_control_006` пересобран, `app` healthy | ✅ |
|
||||
| Регрессия `pytest -q` | ✅ 14 passed |
|
||||
| 2: `app.rotation` не импортирует API-слой | ✅ |
|
||||
| 2: политика входа не изменилась: 5 неудач → 401×4 + 429, на верный пароль тоже 429, `Retry-After` ≈ 600 с | ✅ |
|
||||
| 1a: удаление префикса ∥ вставка промежуточного префикса, ×20 | ✅ все пары ответов 204/201; дерево согласовано: у каждого префикса родитель — самый узкий охватывающий |
|
||||
| 1b: перенос в другой VRF ∥ создание адреса в переносимом префиксе, ×20 | ✅ без 500 и взаимных блокировок; у всех адресов `vrf_id` совпадает с VRF их префикса; все 20 префиксов перенесены |
|
||||
| 1c: встречные переносы двух префиксов между двумя VRF, ×15 | ✅ всё 200/200, взаимных блокировок нет |
|
||||
| 1d: перенос ∥ удаление того же префикса, ×10 | ✅ только ожидаемые исходы: 5× (перенос 200, удаление 409 «одновременно изменяется»), 5× (перенос 404, удаление 204) |
|
||||
| Лог приложения: `deadlock` / `Traceback` | ✅ 0 |
|
||||
|
||||
Сценарии выполнялись скриптом во временной папке, в репозиторий он не добавлен. Временные данные (`vt30-*`), попытки входа и записи `known_logins` удалены.
|
||||
|
||||
## Вывод
|
||||
Замечания 1 и 2 закрыты. Изменения 025–030 готовы к коммиту.
|
||||
+1
-1
@@ -35,7 +35,7 @@ def test_tree_utilization_and_next_free(client, org):
|
||||
assert page["summary"] == {"assigned": 1, "reserved": 1, "deprecated": 0, "free": 4, "capacity": 6}
|
||||
assert client.post(f"/prefixes/{leaf['id']}/addresses/next").json()["address"] == "10.203.1.3"
|
||||
parent = client.get(f"/prefixes/{parent['id']}").json()
|
||||
assert parent["capacity"] == 6 and parent["used"] == 2 # ёмкость родителя — по вложенным листьям
|
||||
assert parent["capacity"] == 65534 and parent["used"] == 2 # ёмкость родителя — размер его подсети, used — по поддереву (изменение 025)
|
||||
|
||||
|
||||
def test_in_use_objects_cannot_be_deleted(client, org):
|
||||
|
||||
+21
-5
@@ -358,11 +358,25 @@ async function typesDialog() {
|
||||
}
|
||||
|
||||
// ---- prefixes
|
||||
const PREFIX_UI_CAP = 20000; // защита от неограниченной подгрузки: организация с частичным разбиением может дать сотни тысяч префиксов
|
||||
async function loadPrefixes(orgId) {
|
||||
// грузит все страницы /prefixes (порядок ORDER BY vrf_id, prefix стабилен) до total, но не больше PREFIX_UI_CAP —
|
||||
// иначе заголовок, вкладка «Все» и родительский список в диалогах молча ограничивались первой страницей (изменение 028, находка №3)
|
||||
let items = [], total = 0, offset = 0;
|
||||
for (;;) {
|
||||
const page = await api("/prefixes", { params: { organization_id: orgId, limit: 1000, offset } });
|
||||
total = page.total;
|
||||
items = items.concat(page.items);
|
||||
offset += page.items.length;
|
||||
if (page.items.length === 0 || offset >= total || items.length >= PREFIX_UI_CAP) break;
|
||||
}
|
||||
return { items, total };
|
||||
}
|
||||
screens.prefixes = async () => {
|
||||
const org = currentOrg();
|
||||
if (!org) return shell("prefixes", header("Префиксы", "Сначала добавьте организацию") + `<div class="card"><div class="empty">Нет организаций. ${btn("Добавить организацию", "org-new", { cls: "primary" })}</div></div>`);
|
||||
const [pl, vl] = await Promise.all([
|
||||
api("/prefixes", { params: { organization_id: org.id, limit: 1000 } }),
|
||||
loadPrefixes(org.id),
|
||||
api("/vrfs", { params: { organization_id: org.id } }),
|
||||
]);
|
||||
S.prefixes = pl.items;
|
||||
@@ -393,11 +407,13 @@ screens.prefixes = async () => {
|
||||
}).join("");
|
||||
const statusItems = [{ label: "любой", value: "" }, { label: "Активен", value: "active" }, { label: "Резерв", value: "reserved" }, { label: "Устарел", value: "deprecated" }];
|
||||
const famItems = [{ label: "любое", value: "" }, { label: "IPv4", value: "4" }, { label: "IPv6", value: "6" }];
|
||||
return shell("prefixes", `${crumbsOrg()}${header("Префиксы", `${all.length} ${plural(all.length, "префикс", "префикса", "префиксов")} · ${vl.total} VRF · организация: ${esc(org.name)}`, btn("Добавить префикс", "pfx-new", { cls: "primary", icon: I.plus() }))}
|
||||
<div class="tabs">${tab(0, "Все", all.length)}${vl.items.map((v) => tab(v.id, v.name, v.prefixes_count)).join("")}<div class="grow"></div><button type="button" class="link-muted" data-action="vrfs">${I.gear()}Управление VRF</button></div>
|
||||
const truncated = pl.total > all.length ? `<div class="info">Загружено ${fmtNum(all.length)} из ${fmtNum(pl.total)} префиксов — уточните поиск или выберите VRF</div>` : "";
|
||||
return shell("prefixes", `${crumbsOrg()}${header("Префиксы", `${pl.total} ${plural(pl.total, "префикс", "префикса", "префиксов")} · ${vl.total} VRF · организация: ${esc(org.name)}`, btn("Добавить префикс", "pfx-new", { cls: "primary", icon: I.plus() }))}
|
||||
<div class="tabs">${tab(0, "Все", pl.total)}${vl.items.map((v) => tab(v.id, v.name, v.prefixes_count)).join("")}<div class="grow"></div><button type="button" class="link-muted" data-action="vrfs">${I.gear()}Управление VRF</button></div>
|
||||
${truncated}
|
||||
<div class="card"><div class="filters">${orgSwitcher()}${searchBox(S.q || "", "Поиск: префикс, описание", 280)}${filterBtn("status", "Статус", statusItems, S.status ?? "")}${filterBtn("family", "Семейство", famItems, S.family ?? "")}</div>
|
||||
${bulkBar(bulkMenu("bulk-status", "Статус", Object.entries(PREFIX_STATUS).map(([value, [, label]]) => ({ label, value }))))}<div class="tr th" style="--cols:${cols};--h:40px">${selAllCell()}<span></span><span>Префикс</span><span>Описание</span><span>VRF</span><span>Статус</span><span>Использование</span><span>Адресов</span><span></span></div>
|
||||
${rows || '<div class="empty">Префиксов нет</div>'}<div class="foot">Показано ${visible.length} из ${all.length} ${plural(all.length, "префикса", "префиксов", "префиксов")}</div></div>`);
|
||||
${rows || '<div class="empty">Префиксов нет</div>'}<div class="foot">Показано ${visible.length} из ${pl.total} ${plural(pl.total, "префикса", "префиксов", "префиксов")}</div></div>`);
|
||||
};
|
||||
function prefixDialog(p) {
|
||||
const edit = !!p;
|
||||
@@ -495,7 +511,7 @@ screens.address = async (id) => {
|
||||
const limit = S.limit || 100;
|
||||
const [page, plist, devs, vrfs] = await Promise.all([
|
||||
api(`/prefixes/${id}/addresses`, { params: { status: S.astatus, q: S.q, limit } }),
|
||||
api("/prefixes", { params: { organization_id: p.organization_id, limit: 1000 } }),
|
||||
loadPrefixes(p.organization_id),
|
||||
api("/devices", { params: { organization_id: p.organization_id, limit: 500 } }),
|
||||
api("/vrfs", { params: { organization_id: p.organization_id } }), // нужен окну «Редактировать префикс»
|
||||
]);
|
||||
|
||||
Reference in new issue
Block a user