73 lines
7.6 KiB
Markdown
73 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`, чтобы не блокировать вход.
|