ayurishchevandClaude Opus 5.5
13e17fbb47
Ревью кодовой базы (docs/reviews/2026-09-26-codebase-review.md) и планы по каждой находке:
011 IP уникален в VRF и хранится в самом узком префиксе (addresses.vrf_id, составной FK
с каскадом при переносе VRF, миграция 0007 с остановкой на дублях).
012 Ограничение попыток входа (login_attempts, 429 + Retry-After), выравнивание времени
ответа, журнал без вытеснения анонимными событиями (миграция 0006).
013 Границы пагинации: отрицательные/чрезмерные limit/offset дают 422 вместо 500.
014 Экран адресов: страница свободных адресов арифметикой, пагинация в SQL.
015 Запрет адреса сети/broadcast, загрузка не выше 100 %.
016 Роль по умолчанию — viewer.
017 Проверка JWT_SECRET/ADMIN_PASSWORD при старте.
018 null в PATCH очищает текстовые поля; нейтральный текст конфликта БД.
019 Пакетная загрузка в списках вместо N+1.
020 Автоназначение адреса вне вложенных префиксов, с блокировкой префикса.
021 Advisory-lock при снятии прав администратора, уникальный lower(username) (миграция 0008).
022 Контейнер не от root, healthcheck, блокировка миграций, requirements.lock.
023 Экранирование LIKE, журнал отказов очистки, заголовки безопасности, учёт force-удаления,
отзыв токенов при смене пароля (claim pv, миграция 0005).
024 Исправление находок ревью 011-023 (docs/reviews/2026-09-26-changes-011-023-review.md):
сериализация попыток входа, запрет переноса адресов в адрес сети/broadcast, журнал входов,
запрет смены своего пароля через PATCH, валидация PATCH устройства, обновлён тест токенов.
Тесты: 14 passed. Документация: README.md, docs/changes/011-024, docs/reviews.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
74 lines
7.6 KiB
Markdown
74 lines
7.6 KiB
Markdown
# Исправление находок ревью изменений 011–023 (изменение 024)
|
||
|
||
Источник: `docs/reviews/2026-09-26-changes-011-023-review.md`. Изменения 011–023 не закоммичены; правки вносятся поверх них в рабочем дереве.
|
||
Новые автотесты не пишутся (отдельный этап). Исключение — п. 3: существующий тест приводится к намеренно изменённому поведению.
|
||
|
||
## Находки и решения
|
||
|
||
### 1. Параллельные запросы обходят лимит входа (средняя, 012)
|
||
**Где:** `app/api/v1/auth.py`, `login`.
|
||
**Суть:** проверка блокировки (`_retry_after`) идёт до проверки пароля, запись попытки — после. Параллельные запросы проходят проверку одновременно.
|
||
Воспроизведено: 16 параллельных попыток → 16 проверок пароля при лимите 5.
|
||
**Решение:** в начале `login`, до `_retry_after`, выполнить `db.execute(select(func.pg_advisory_xact_lock(func.hashtext(name))))`, где `name` — логин в нижнем регистре.
|
||
Блокировка держится до конца транзакции, после `commit` снимается. Попытки одного логина сериализуются; лимит по IP при этом считается корректно для каждого логина.
|
||
Остальную логику не менять.
|
||
|
||
### 2. Перенос адресов нарушает запрет адреса сети/broadcast (низкая, 011 × 015)
|
||
**Где:** `app/api/v1/prefixes.py` — `create_prefix`, `_move_to_vrf`, `rehome_addresses`; `alembic/versions/0007_address_vrf_unique.py`.
|
||
**Суть:** при создании вложенного префикса адреса родителя из его диапазона переносятся в него, и адрес может стать адресом сети или broadcast нового префикса.
|
||
Воспроизведено: `.128` в `/24`, затем создание `.128/25` — адрес стал сетевым.
|
||
**Решение:**
|
||
- Функция `_unusable_after_rehome(db, p) -> list[str]`: адреса VRF префикса `p`, которые после переноса окажутся в префиксе, где они являются адресом сети или broadcast.
|
||
Проверять самый узкий целевой префикс; для IPv4 с длиной ≤ /30 использовать `network_role` из `app/services.py`.
|
||
- `create_prefix`: после `flush` и `attach_to_tree`, до `rehome_addresses`, при непустом списке — `db.rollback()` и 422:
|
||
«Адреса … станут адресом сети/broadcast префикса X: освободите их или выберите другой префикс».
|
||
- `_move_to_vrf`: та же проверка после смены VRF, до `rehome_addresses`, — 422 без частичных изменений (исключение внутри транзакции, `commit` не выполняется).
|
||
- `allocate_subnet` не трогать: адреса родителя уже считаются занятыми.
|
||
- Миграция 0007: после шага переноса адресов вывести предупреждение (`print` или `logging`) со списком адресов сети/broadcast в новых префиксах. Миграцию не останавливать.
|
||
Уже применённую на стенде миграцию не переписывать по смыслу — только добавить вывод.
|
||
|
||
### 3. Устаревший тест токенов (низкая, 023)
|
||
**Где:** `tests/test_users.py`, строка с комментарием «выданный ранее токен продолжает работать».
|
||
**Решение:** привести тест к новой семантике.
|
||
- Старый токен `other` после `POST /users/me/password` → 401.
|
||
- Токен из ответа (`access_token`) → `GET /auth/me` 200.
|
||
|
||
Больше тесты не менять.
|
||
|
||
### 4. Полнота журнала входов (низкая, 012)
|
||
**Где:** `app/api/v1/auth.py`, `app/rotation.py`.
|
||
**Решение:**
|
||
- Решение о записи `session.failed` принимать по числу неудач **этого логина** в окне (до вставки). Для этого `_retry_after` должна возвращать счётчики по областям отдельно,
|
||
например `{"login": n, "ip": m}`, а не их максимум.
|
||
- В `diff` записи `session.locked` с областью `ip` добавить `distinct_logins` — число различных логинов с этого IP в окне.
|
||
- `rotate()`: первыми удалять записи `entity_type == "session"` с `action` в (`failed`, `locked`).
|
||
|
||
### 5. Сброс своего пароля через PATCH (низкая, 023)
|
||
**Где:** `app/api/v1/users.py`, `update_user`.
|
||
**Решение:** если `u.id == admin.id` и в запросе есть `password` — 422 «Свой пароль меняется через /users/me/password (с подтверждением текущего)».
|
||
|
||
### 6. Нет валидации PATCH устройства (низкая, найдено попутно)
|
||
**Где:** `app/schemas.py`, `DeviceUpdate`.
|
||
**Решение:** вынести проверки `DeviceIn._name` (FQDN) и `DeviceIn._mac` (формат и нормализация `AA:BB:…`) в модульные функции и применить их в `DeviceIn` и `DeviceUpdate`.
|
||
В `DeviceUpdate` `None` пропускать; пустая строка для `mac` допустима, как в `DeviceIn`. Порядок с `_blank` (null → "") сохранить: сначала `_blank`, затем проверка.
|
||
|
||
### 7. Мелочи токена (инфо, 023)
|
||
**Где:** `app/security.py`.
|
||
**Решение:**
|
||
- `_password_version`: целочисленный расчёт `(changed - datetime(1970, 1, 1, tzinfo=timezone.utc)) // timedelta(microseconds=1)`.
|
||
- `iat` оставить как информационное поле: добавить комментарий, что отзыв работает по `pv`.
|
||
|
||
## Артефакты
|
||
- `docs/changes/024-review-fixes-011-023/SUMMARY.md` — что сделано по каждому пункту, отклонения от плана, как проверено.
|
||
- `README.md` — дополнить там, где меняется поведение:
|
||
- 422 при создании префикса или переносе VRF;
|
||
- запрет сброса своего пароля через PATCH;
|
||
- валидация PATCH устройства.
|
||
|
||
## Проверка
|
||
1. `venv/bin/python -c 'import app.main'` и `node --check web/app.js`.
|
||
2. `docker compose -p ipam_control_006 up -d --build app` — только проект `ipam_control_006`; контейнеры прежней поставки `ipam_control-*` не трогать.
|
||
3. `venv/bin/python -m pytest -q` — все тесты проходят.
|
||
4. Ручная проверка через API (скрипт в scratchpad, не в репозитории) по п. 1, 2, 5, 6. Временные данные удалить.
|
||
После проверки п. 1 очистить `login_attempts`, чтобы не блокировать вход.
|