AURORA-1936: запрет удаления чужих строк, подтверждение удаления #257

aep · 2026-10-30T17:07:41+03:00

aep commented

2026-10-30 17:07:41 +03:00

Closes #249.

Проблема

В формах 1, 2 и 4 можно было удалить строку, добавленную другим пользователем, без какого-либо подтверждения — строка пропадала сразу по клику и не восстанавливалась. Воспроизводилось под всеми ролями, включая удаление собственных строк (тоже без подтверждения).

Что сделано

api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql — v3.del_budget_line теперь проверяет владельца: не-ADMIN может удалить только budget_line, где created_by = current_user_id(). Проверка добавлена в обе ветки функции — обычные листы (budget_line.id = p_id) и сателлиты AHR_RENT/UTILITY/SECURITY (через created_by связанной budget_line). Используется уже существующий в проекте идиом v3.current_user_id() + v3.user_role_code() (как в can_delete_project) — значение app.user_id уже выставляется SET LOCAL на каждый запрос (REST и WebSocket), поэтому правка полностью на стороне SQL, без изменений Python/API-слоя. Нарушение -> role_not_allowed: ... -> 403 по уже существующей конвенции PG_403_ERRORS.
web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx — новая модалка подтверждения (построена на общем Modal + SecondaryButton/DangerOutlinedButton, по образцу ModalConfirmArchiveSsp).
RealtimeTable.jsx: handleDeleteRow больше не вызывает удаление напрямую — открывает модалку; реальное удаление (contextDeleteRow) происходит только по подтверждению. Отмена ничего не меняет (строка и так не трогалась до подтверждения).
Интеграционные тесты: новый пользователь isp2 в tests/sqls/fixture.sql (тот же org_unit, что у budget_form id=1, что и у isp1 — добавлена вторая user_org-строка для isp1), чтобы было два разных неадминских «владельца» для проверки.

Rulings (решения без обсуждения)

Классифицировал bounded — восстановление уже подразумевавшегося поведения (владелец строки), а не новая возможность.
Проверку владельца сделал в PL/pgSQL через v3.current_user_id()/v3.user_role_code(), а не передачей user_id/is_admin параметром из Python — это точно повторяет существующий идиом (can_delete_project в 0012_del_project.sql) и не меняет сигнатуру del_budget_line, сигнатуру репозитория/сервиса и REST-эндпоинт вообще не трогает. Расширение сигнатуры параметром было бы более explicit, но избыточно при уже работающем SET LOCAL app.user_id.
Ownership-проверка для сателлитов АХР (аренда/коммуналка/охрана) сделана через created_by связанной budget_line (сами таблицы rent_detail/utility_detail/security_detail created_by не хранят) — это единственный доступный источник владельца для этих строк.
Для теста добавил второго исполнителя isp2 и вторую user_org-запись для isp1 в общий tests/sqls/fixture.sql — аддитивно (только новые строки, ничего существующего не менял), проверил grep'ом, что никакой другой тест не считает точное число пользователей/user_org, так что риск повредить другие тесты минимален, но не проверено запуском (см. ниже).
Подтверждение в модалке единое для любой строки (не только чужой) — так требует тикет («Отсутствие подтверждения также воспроизводится при удалении собственной строки»); отдельного UI-признака «это не ваша строка» не добавлял, т.к. тикет этого не просит, а дальше всё равно решает бэкенд.

Тесты

Backend unit: pytest tests/unit — 58 passed, 2 failed, но оба failed не связаны с этой правкой (test_export_service_query_params.py, экспорт-сервис, я его не трогал — те же 2 падения на test без моих изменений).
Backend integration — НЕ ЗАПУСКАЛИСЬ: в этом окружении нет Postgres (pg_isready недоступен). Новые тесты (test_forms_delete_line_forbidden_for_non_owner_executor, test_forms_delete_line_allowed_for_admin_regardless_of_owner) собираются pytest --collect-only без ошибок, но ни разу не выполнялись. Нужно прогнать pytest tests/integration --init_db на окружении с живым Postgres перед мержем — если имя поля в ответе add_line не line_id/id (я ориентировался на getDataRowId с фронта: data?.id || data?.line_id), тест может потребовать правки под реальный формат ответа.
Frontend: node --test src/components/RealtimeTable/utils/*.test.js — 8/8. npm run build — без ошибок. Biome на изменённых файлах — новых ошибок не внесено (сверено с версией в test).

Ручная проверка (acceptance criteria, не автоматизировано)

Выделить чужую строку в форме 1/2/4, нажать «Удалить строку» — модалка с подтверждением, «Отмена» ничего не меняет.
Подтвердить удаление чужой строки под ролью исполнителя — сервер отклоняет (403), строка остаётся.
Удалить собственную строку — подтверждение, затем успешное удаление, итоги пересчитываются.
Администратор может удалить любую строку после подтверждения.
Прогнать pytest tests/integration --init_db -k aurora_1936_or_delete_line (или весь набор) на окружении с Postgres — см. риск в Rulings/Тестах выше.

🤖 Generated with Claude Code

Closes #249. ## Проблема В формах 1, 2 и 4 можно было удалить строку, добавленную другим пользователем, без какого-либо подтверждения — строка пропадала сразу по клику и не восстанавливалась. Воспроизводилось под всеми ролями, включая удаление собственных строк (тоже без подтверждения). ## Что сделано - `api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql` — `v3.del_budget_line` теперь проверяет владельца: не-ADMIN может удалить только `budget_line`, где `created_by = current_user_id()`. Проверка добавлена в обе ветки функции — обычные листы (budget_line.id = p_id) и сателлиты AHR_RENT/UTILITY/SECURITY (через created_by связанной budget_line). Используется уже существующий в проекте идиом `v3.current_user_id()` + `v3.user_role_code()` (как в `can_delete_project`) — значение `app.user_id` уже выставляется `SET LOCAL` на каждый запрос (REST и WebSocket), поэтому правка полностью на стороне SQL, без изменений Python/API-слоя. Нарушение -> `role_not_allowed: ...` -> 403 по уже существующей конвенции `PG_403_ERRORS`. - `web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx` — новая модалка подтверждения (построена на общем `Modal` + `SecondaryButton`/`DangerOutlinedButton`, по образцу `ModalConfirmArchiveSsp`). - `RealtimeTable.jsx`: `handleDeleteRow` больше не вызывает удаление напрямую — открывает модалку; реальное удаление (`contextDeleteRow`) происходит только по подтверждению. Отмена ничего не меняет (строка и так не трогалась до подтверждения). - Интеграционные тесты: новый пользователь `isp2` в `tests/sqls/fixture.sql` (тот же `org_unit`, что у `budget_form id=1`, что и у `isp1` — добавлена вторая `user_org`-строка для isp1), чтобы было два разных неадминских «владельца» для проверки. ## Rulings (решения без обсуждения) 1. Классифицировал bounded — восстановление уже подразумевавшегося поведения (владелец строки), а не новая возможность. 2. Проверку владельца сделал в PL/pgSQL через `v3.current_user_id()`/`v3.user_role_code()`, а не передачей `user_id`/`is_admin` параметром из Python — это точно повторяет существующий идиом (`can_delete_project` в `0012_del_project.sql`) и не меняет сигнатуру `del_budget_line`, сигнатуру репозитория/сервиса и REST-эндпоинт вообще не трогает. Расширение сигнатуры параметром было бы более explicit, но избыточно при уже работающем `SET LOCAL app.user_id`. 3. Ownership-проверка для сателлитов АХР (аренда/коммуналка/охрана) сделана через `created_by` связанной `budget_line` (сами таблицы rent_detail/utility_detail/security_detail created_by не хранят) — это единственный доступный источник владельца для этих строк. 4. Для теста добавил второго исполнителя `isp2` и вторую `user_org`-запись для `isp1` в общий `tests/sqls/fixture.sql` — аддитивно (только новые строки, ничего существующего не менял), проверил grep'ом, что никакой другой тест не считает точное число пользователей/user_org, так что риск повредить другие тесты минимален, но не проверено запуском (см. ниже). 5. Подтверждение в модалке единое для любой строки (не только чужой) — так требует тикет («Отсутствие подтверждения также воспроизводится при удалении собственной строки»); отдельного UI-признака «это не ваша строка» не добавлял, т.к. тикет этого не просит, а дальше всё равно решает бэкенд. ## Тесты - Backend unit: `pytest tests/unit` — 58 passed, 2 failed, но оба failed не связаны с этой правкой (`test_export_service_query_params.py`, экспорт-сервис, я его не трогал — те же 2 падения на `test` без моих изменений). - **Backend integration — НЕ ЗАПУСКАЛИСЬ: в этом окружении нет Postgres** (`pg_isready` недоступен). Новые тесты (`test_forms_delete_line_forbidden_for_non_owner_executor`, `test_forms_delete_line_allowed_for_admin_regardless_of_owner`) собираются `pytest --collect-only` без ошибок, но ни разу не выполнялись. Нужно прогнать `pytest tests/integration --init_db` на окружении с живым Postgres перед мержем — если имя поля в ответе `add_line` не `line_id`/`id` (я ориентировался на `getDataRowId` с фронта: `data?.id || data?.line_id`), тест может потребовать правки под реальный формат ответа. - Frontend: `node --test src/components/RealtimeTable/utils/*.test.js` — 8/8. `npm run build` — без ошибок. Biome на изменённых файлах — новых ошибок не внесено (сверено с версией в `test`). ## Ручная проверка (acceptance criteria, не автоматизировано) - [ ] Выделить чужую строку в форме 1/2/4, нажать «Удалить строку» — модалка с подтверждением, «Отмена» ничего не меняет. - [ ] Подтвердить удаление чужой строки под ролью исполнителя — сервер отклоняет (403), строка остаётся. - [ ] Удалить собственную строку — подтверждение, затем успешное удаление, итоги пересчитываются. - [ ] Администратор может удалить любую строку после подтверждения. - [ ] Прогнать `pytest tests/integration --init_db -k aurora_1936_or_delete_line` (или весь набор) на окружении с Postgres — см. риск в Rulings/Тестах выше. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

aep added 1 commit 2026-10-30 17:07:44 +03:00

fix(forms): запретить удаление чужих строк, добавить подтверждение b95029f70a

Удаление строки в формах 1/2/4 не проверяло владельца: любой исполнитель
мог удалить чужую строку мгновенно, без подтверждения.

- v3.del_budget_line (миграция 0050): не-admin может удалить только
  budget_line, где created_by совпадает с current_user_id() (проверка
  и для обычных листов, и для AHR_RENT/UTILITY/SECURITY — через
  created_by связанной budget_line). Нарушение -> role_not_allowed:
  (403), по существующей конвенции PG_403_ERRORS.
- RealtimeTable: кнопка «Удалить строку» теперь открывает
  ConfirmDeleteRowModal, реальное удаление — только после подтверждения.
- Интеграционные тесты: новый исполнитель isp2 (fixture.sql) в том же
  org_unit, что и budget_form id=1 — проверяют 403 при чужой строке,
  200 при своей и при удалении администратором.

Issue: #249 (AURORA-1936, эпик AURORA-1876, Высокий приоритет)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

aep referenced this pull request

2026-10-30 17:07:54 +03:00

AURORA-1936: формы 1/2/4 — можно удалить чужую строку без подтверждения #249

aep referenced this pull request

2026-10-30 23:26:16 +03:00

AURORA-1938: расчёт переноса в фонд экономии на листах АХР/КВ/Операц формы РФ #262

aep referenced this pull request

2026-10-30 23:26:19 +03:00

AURORA-1906: колонка «СПОД» в листах аренды, коммуналки и охраны АХР #263

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions.