From b95029f70aac3d59b52c73620ad2ee5d51d95fe6 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Fri, 30 Oct 2026 18:06:52 +0400 Subject: [PATCH] =?UTF-8?q?fix(forms):=20=D0=B7=D0=B0=D0=BF=D1=80=D0=B5?= =?UTF-8?q?=D1=82=D0=B8=D1=82=D1=8C=20=D1=83=D0=B4=D0=B0=D0=BB=D0=B5=D0=BD?= =?UTF-8?q?=D0=B8=D0=B5=20=D1=87=D1=83=D0=B6=D0=B8=D1=85=20=D1=81=D1=82?= =?UTF-8?q?=D1=80=D0=BE=D0=BA,=20=D0=B4=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D1=82=D1=8C=20=D0=BF=D0=BE=D0=B4=D1=82=D0=B2=D0=B5=D1=80=D0=B6?= =?UTF-8?q?=D0=B4=D0=B5=D0=BD=D0=B8=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Удаление строки в формах 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 --- .../0050_budget_line_delete_owner_guard.py | 15 ++ .../0050_budget_line_delete_owner_guard.sql | 158 ++++++++++++++++++ api/tests/integration/conftest.py | 12 ++ api/tests/integration/test_forms_api_smoke.py | 49 ++++++ api/tests/sqls/fixture.sql | 11 +- .../Modals/ConfirmDeleteRowModal.jsx | 19 +++ .../RealtimeTable/RealtimeTable.jsx | 15 +- 7 files changed, 273 insertions(+), 6 deletions(-) create mode 100644 api/alembic/versions/0050_budget_line_delete_owner_guard.py create mode 100644 api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql create mode 100644 web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx diff --git a/api/alembic/versions/0050_budget_line_delete_owner_guard.py b/api/alembic/versions/0050_budget_line_delete_owner_guard.py new file mode 100644 index 0000000..6f49e35 --- /dev/null +++ b/api/alembic/versions/0050_budget_line_delete_owner_guard.py @@ -0,0 +1,15 @@ +from sql_migration import run_sql_file_migration + + +revision = "0050" +down_revision = "0049" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + run_sql_file_migration("0050_budget_line_delete_owner_guard.sql") + + +def downgrade() -> None: + pass diff --git a/api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql b/api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql new file mode 100644 index 0000000..07aeb26 --- /dev/null +++ b/api/alembic/versions/sql/0050_budget_line_delete_owner_guard.sql @@ -0,0 +1,158 @@ +CREATE OR REPLACE FUNCTION v3.del_budget_line(p_id integer, p_direction character varying DEFAULT NULL::character varying, p_sheet character varying DEFAULT NULL::character varying) + RETURNS TABLE(row_type character varying, depth integer, sort_order bigint, data jsonb) + LANGUAGE plpgsql +AS $function$ +DECLARE + v_form_id INT; + v_org_id INT; + v_sheet VARCHAR; + v_sat_table TEXT; + v_eid INT; + v_line_id INT; + v_ei_path INT[]; + v_actor_id INT; + v_role VARCHAR; + v_created_by INT; +BEGIN + v_actor_id := v3.current_user_id(); + v_role := v3.user_role_code(v_actor_id); + + IF p_sheet IN ('AHR_RENT','AHR_UTILITY','AHR_SECURITY') THEN + v_sat_table := CASE p_sheet + WHEN 'AHR_RENT' THEN 'rent_detail' + WHEN 'AHR_UTILITY' THEN 'utility_detail' + WHEN 'AHR_SECURITY' THEN 'security_detail' + END; + + EXECUTE format( + 'SELECT bl.budget_form_id, bl.created_by FROM v3.%I s + JOIN v3.budget_line bl ON bl.id = s.line_id + WHERE s.id = $1', v_sat_table + ) USING p_id INTO v_form_id, v_created_by; + + IF v_form_id IS NULL THEN + RAISE EXCEPTION '%.id=% не существует', v_sat_table, p_id; + END IF; + + IF v_role != 'ADMIN' AND v_created_by IS DISTINCT FROM v_actor_id THEN + RAISE EXCEPTION 'role_not_allowed: можно удалить только добавленную вами строку'; + END IF; + + SELECT m.target_line_id + INTO v_line_id + FROM v3.form2_ahr_satellite_line_source m + WHERE m.source_sheet = p_sheet + AND m.source_id = p_id; + + EXECUTE format('DELETE FROM v3.%I WHERE id = $1', v_sat_table) USING p_id; + + IF v_line_id IS NOT NULL THEN + SELECT bl.expense_item_id + INTO v_eid + FROM v3.budget_line bl + WHERE bl.id = v_line_id; + + DELETE FROM v3.plan WHERE line_id = v_line_id; + DELETE FROM v3.budget_line_quarter WHERE line_id = v_line_id; + DELETE FROM v3.budget_line WHERE id = v_line_id; + + SELECT org_unit_id INTO v_org_id FROM v3.budget_form WHERE id = v_form_id; + + PERFORM v3.log_event( + 'ROW_DELETE', 'ROW', + jsonb_build_object( + 'sheet', 'AHR', + 'line_id', v_line_id, + 'entity_id', v_line_id, + 'expense_item_id', v_eid, + 'direction', 'Support', + 'source_sheet', p_sheet, + 'source_id', p_id + ), + v_form_id, NULL, v_org_id + ); + END IF; + RETURN; + END IF; + + SELECT bl.budget_form_id, ei.sheet, bl.expense_item_id, bl.created_by + INTO v_form_id, v_sheet, v_eid, v_created_by + FROM v3.budget_line bl + JOIN v3.expense_item ei ON ei.id = bl.expense_item_id + WHERE bl.id = p_id; + + IF v_form_id IS NULL THEN + RAISE EXCEPTION 'budget_line id=% не существует', p_id; + END IF; + + IF v_role != 'ADMIN' AND v_created_by IS DISTINCT FROM v_actor_id THEN + RAISE EXCEPTION 'role_not_allowed: можно удалить только добавленную вами строку'; + END IF; + + IF v_sheet = 'AHR' AND ( + EXISTS ( + SELECT 1 + FROM v3.form2_ahr_satellite_line_source m + WHERE m.target_line_id = p_id + ) + OR EXISTS ( + SELECT 1 + FROM v3.budget_line bl + JOIN v3.expense_item ei ON ei.id = bl.expense_item_id + JOIN v3.limit_template lt + ON lt.row_type = 'LEAF' + AND ( + (ei.depth = 3 AND ei.item_id = lt.expense_item_code) + OR ei.num_group_id = lt.expense_item_code + ) + WHERE bl.id = p_id + AND bl.budget_form_id = v_form_id + AND ei.sheet = 'AHR' + AND ei.direction = 'Support' + ) + ) THEN + RAISE EXCEPTION 'Строку АХР, созданную из листа-сателлита или АХР Лимиты, нельзя удалить напрямую' + USING ERRCODE = '23503', + HINT = 'Удалите строку на исходном листе АХР Аренда / АХР Коммунальные услуги / АХР Безопасность либо измените строку на листе АХР Лимиты'; + END IF; + + v_ei_path := v3._expense_item_path_set(ARRAY[v_eid]); + + DELETE FROM v3.plan WHERE line_id = p_id; + DELETE FROM v3.sequestration WHERE line_id = p_id; + DELETE FROM v3.reserve WHERE line_id = p_id; + DELETE FROM v3.allocation WHERE line_id = p_id; + DELETE FROM v3.contract_summary WHERE line_id = p_id; + DELETE FROM v3.collegial_approval WHERE line_id = p_id; + DELETE FROM v3.ckk WHERE line_id = p_id; + DELETE FROM v3.contract_detail WHERE line_id = p_id; + DELETE FROM v3.booking WHERE line_id = p_id; + DELETE FROM v3.budget_line_quarter WHERE line_id = p_id; + DELETE FROM v3.rent_detail WHERE line_id = p_id; + DELETE FROM v3.utility_detail WHERE line_id = p_id; + DELETE FROM v3.security_detail WHERE line_id = p_id; + + DELETE FROM v3.budget_line WHERE id = p_id; + + SELECT org_unit_id INTO v_org_id FROM v3.budget_form WHERE id = v_form_id; + + PERFORM v3.log_event( + 'ROW_DELETE', 'ROW', + jsonb_build_object( + 'sheet', v_sheet, + 'line_id', p_id, + 'entity_id', p_id, + 'expense_item_id', v_eid, + 'direction', p_direction + ), + v_form_id, NULL, v_org_id + ); + + RETURN QUERY + SELECT v.row_type, v.depth, v.sort_order, v.data + FROM v3.v_form_view(v_form_id, v_sheet, NULL, p_direction) v + WHERE v.row_type IN ('ROOT','GROUP','ITEM','SUB_ITEM') + AND (v.data->'header'->>'expense_item_id')::INT = ANY(v_ei_path) + ORDER BY v.sort_order; +END; +$function$; diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index 81cb26d..1ed78dd 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -79,3 +79,15 @@ def isp_tokens(client, isp_password: str): ) assert response.status_code == 200 return response.json() + + +@pytest.fixture +def isp2_tokens(client, isp_password: str): + # Второй исполнитель в том же org_unit, что и budget_form id=1 — нужен, + # чтобы проверять, что исполнитель не может удалить чужую строку. + response = client.post( + "/api/v1/auth/login", + json={"username": "isp2", "password": isp_password}, + ) + assert response.status_code == 200 + return response.json() diff --git a/api/tests/integration/test_forms_api_smoke.py b/api/tests/integration/test_forms_api_smoke.py index 4fb199a..49bf30d 100644 --- a/api/tests/integration/test_forms_api_smoke.py +++ b/api/tests/integration/test_forms_api_smoke.py @@ -167,6 +167,55 @@ def test_forms_delete_line_not_found_error_shape(client, admin_tokens, auth_head assert "message" in payload +def _added_line_id(payload: dict, marker: str) -> int: + for row in payload["result"]: + data = row.get("data") or {} + if data.get("internal_order") == marker: + return data.get("line_id") or data.get("id") + raise AssertionError(f"строка с internal_order={marker!r} не найдена в ответе add_line") + + +def test_forms_delete_line_forbidden_for_non_owner_executor(client, isp_tokens, isp2_tokens, auth_headers): + # AURORA-1936: исполнитель может удалить только им же созданную строку. + marker = "aurora-1936-owner-guard" + add_response = client.post( + "/api/v1/form/1/sheet/AHR/line?direction=Support§ions=plan,contract_summary", + json={"expense_item_id": 120, "direction": "Support", "internal_order": marker}, + headers=auth_headers(isp_tokens), + ) + assert add_response.status_code == 200 + line_id = _added_line_id(add_response.json(), marker) + + forbidden_response = client.delete( + f"/api/v1/form/1/sheet/AHR/line/{line_id}?direction=Support", + headers=auth_headers(isp2_tokens), + ) + assert forbidden_response.status_code == 403 + + own_delete_response = client.delete( + f"/api/v1/form/1/sheet/AHR/line/{line_id}?direction=Support", + headers=auth_headers(isp_tokens), + ) + assert own_delete_response.status_code == 200 + + +def test_forms_delete_line_allowed_for_admin_regardless_of_owner(client, isp2_tokens, admin_tokens, auth_headers): + marker = "aurora-1936-admin-bypass" + add_response = client.post( + "/api/v1/form/1/sheet/AHR/line?direction=Support§ions=plan,contract_summary", + json={"expense_item_id": 120, "direction": "Support", "internal_order": marker}, + headers=auth_headers(isp2_tokens), + ) + assert add_response.status_code == 200 + line_id = _added_line_id(add_response.json(), marker) + + response = client.delete( + f"/api/v1/form/1/sheet/AHR/line/{line_id}?direction=Support", + headers=auth_headers(admin_tokens), + ) + assert response.status_code == 200 + + def test_forms_add_form_success(client, admin_tokens, auth_headers): response = client.post( "/api/v1/form/", diff --git a/api/tests/sqls/fixture.sql b/api/tests/sqls/fixture.sql index e9652c7..2c51a63 100644 --- a/api/tests/sqls/fixture.sql +++ b/api/tests/sqls/fixture.sql @@ -1,7 +1,8 @@ INSERT INTO v3.app_user (id,email,username,hashed_password,full_name,role_id,is_active,created_at,updated_at) VALUES (1,'admin@rshb.ru','admin','{admin_password}','admin',1,true,'2026-05-20 11:09:16.987331+03',NULL), - (2,'ispolnitel@rshb.ru','isp1','{isp_password}','Роль Исполнитель Первый',2,true,'2026-05-20 11:00:51.811967+03','2026-06-17 11:51:01.123054+03'); -SELECT setval('v3.app_user_id_seq', 2); + (2,'ispolnitel@rshb.ru','isp1','{isp_password}','Роль Исполнитель Первый',2,true,'2026-05-20 11:00:51.811967+03','2026-06-17 11:51:01.123054+03'), + (3,'ispolnitel2@rshb.ru','isp2','{isp_password}','Роль Исполнитель Второй',2,true,'2026-05-20 11:00:51.811967+03','2026-06-17 11:51:01.123054+03'); +SELECT setval('v3.app_user_id_seq', 3); INSERT INTO v3.org_unit (id,title,is_active,is_ssp,utc_offset) VALUES (1,'Test_SSP',true,true,'+03'), @@ -11,8 +12,10 @@ SELECT setval('v3.org_unit_id_seq', 2); INSERT INTO v3.user_org (id,user_id,org_unit_id) VALUES (1,1,2), (2,1,1), - (3,2,2); -SELECT setval('v3.user_org_id_seq', 3); + (3,2,2), + (4,3,1), + (5,2,1); +SELECT setval('v3.user_org_id_seq', 5); INSERT INTO v3.budget_form (id,form_type_code,"year",created_by,created_at,updated_by,updated_at,org_unit_id) VALUES (1,'FORM_1',2027,NULL,'2026-05-06 16:57:38.066371',NULL,'2026-06-11 14:35:53.783017',1), diff --git a/web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx b/web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx new file mode 100644 index 0000000..6d712db --- /dev/null +++ b/web/src/components/RealtimeTable/Modals/ConfirmDeleteRowModal.jsx @@ -0,0 +1,19 @@ +import { Alert } from '@mui/material'; +import { DangerOutlinedButton, SecondaryButton } from '../../common/Buttons/Buttons'; +import Modal from '../../common/Modal/Modal'; + +export const ConfirmDeleteRowModal = ({ isOpen, onClose, onConfirm }) => ( + + Отмена + Удалить + + }> + Удалить выбранную строку? Данные строки будут потеряны. + +); diff --git a/web/src/components/RealtimeTable/RealtimeTable.jsx b/web/src/components/RealtimeTable/RealtimeTable.jsx index 539e4b3..c641185 100644 --- a/web/src/components/RealtimeTable/RealtimeTable.jsx +++ b/web/src/components/RealtimeTable/RealtimeTable.jsx @@ -12,6 +12,7 @@ import { getTableColumns } from './tableColumns'; import { CircularProgress } from '@mui/material'; import { debounce } from '@mui/material'; import { toast } from 'react-toastify'; +import { ConfirmDeleteRowModal } from './Modals/ConfirmDeleteRowModal'; import { CreateProgramModal } from './Modals/CreateProgramProjectModal'; import { SelectExpenseItemModal } from './Modals/SelectExpenseItemModal'; import { SelectVspModal } from './Modals/SelectVspModal'; @@ -105,6 +106,7 @@ const RealtimeTable = ({ formType, formId, sheetName, direction, year }) => { const [isOpenModalSelectVsp, setIsOpenModalSelectVsp] = useState(false); const [isOpenModalSelectExpenseItem, setIsOpenModalSelectExpenseItem] = useState(false); + const [rowIdPendingDelete, setRowIdPendingDelete] = useState(null); const [createModalType, setCreateModalType] = useState(null); const isFormNoCanAddRow = useMemo(() => FormsNoCanAddRow[formType]?.includes(sheetName) ?? false, [formType, sheetName]); @@ -676,9 +678,17 @@ const RealtimeTable = ({ formType, formId, sheetName, direction, year }) => { toast.error('Выделите строку для удаления'); return; } - contextDeleteRow(getRowId(row)); + setRowIdPendingDelete(getRowId(row)); + }, [canModifyRows, getSelectedRow]); + + const closeConfirmDeleteRow = useCallback(() => setRowIdPendingDelete(null), []); + + const confirmDeleteRow = useCallback(() => { + if (rowIdPendingDelete == null) return; + contextDeleteRow(rowIdPendingDelete); + setRowIdPendingDelete(null); setRowSelection({}); - }, [canModifyRows, contextDeleteRow, getSelectedRow]); + }, [contextDeleteRow, rowIdPendingDelete]); const addRow = useCallback(() => { if (!canModifyRows) { @@ -848,6 +858,7 @@ const RealtimeTable = ({ formType, formId, sheetName, direction, year }) => { +