From 2d00699831fdc79f4fa067e4818f2a2de0f8d5b6 Mon Sep 17 00:00:00 2001 From: lizerk Date: Tue, 14 Apr 2026 01:07:13 +0300 Subject: [PATCH] fix(ldap): nest department role groups into department group --- backend/api/v1/employees/views/departments.py | 39 ++-- backend/api/v1/employees/views/roles.py | 26 ++- .../ldap/services/department_service.py | 71 ++++++- backend/employees/signals/__init__.py | 1 + backend/employees/signals/ldap/__init__.py | 4 +- backend/employees/signals/ldap/role.py | 179 ++++++++++++++++++ backend/employees/tasks.py | 73 +++++++ .../api/v1/employees/test_department_roles.py | 25 +++ .../api/v1/employees/test_role_assignment.py | 30 ++- .../test_department_role_ldap_sync.py | 44 +++++ .../tests/employees/test_ldap_sync_queue.py | 82 ++++++++ 11 files changed, 535 insertions(+), 39 deletions(-) create mode 100644 backend/employees/signals/ldap/role.py create mode 100644 backend/tests/employees/test_department_role_ldap_sync.py diff --git a/backend/api/v1/employees/views/departments.py b/backend/api/v1/employees/views/departments.py index 42e90560..951f4c2b 100644 --- a/backend/api/v1/employees/views/departments.py +++ b/backend/api/v1/employees/views/departments.py @@ -381,37 +381,50 @@ class DepartmentViewSet(viewsets.ModelViewSet): via_assignment = False - if link: - # Сотрудник — член отдела: обновляем роль в линке - link.role = role - link.save(update_fields=["role"]) + active_assignments = list( + RoleAssignment.objects.filter( + employee_id=emp_id, + role__department=dept, + is_active=True, + ).select_related("role") + ) + + def deactivate_assignment(assignment: RoleAssignment) -> None: + assignment.is_active = False + assignment.save(update_fields=["is_active"]) + + if link: + # Сотрудник — член отдела: сперва приводим RoleAssignment + # к единственному актуальному значению, затем обновляем link.role. + for assignment in active_assignments: + if role is None or assignment.role_id != role.id: + deactivate_assignment(assignment) - # Также создаём/обновляем RoleAssignment для консистентности if role: RoleAssignment.objects.update_or_create( employee_id=emp_id, role=role, defaults={"is_active": True, "assigned_by": request.user}, ) - else: - RoleAssignment.objects.filter( - employee_id=emp_id, role__department=dept, is_active=True - ).update(is_active=False) + + link.role = role + link.save(update_fields=["role"]) else: # Сотрудник НЕ член отдела: используем только RoleAssignment via_assignment = True if role: + for assignment in active_assignments: + if assignment.role_id != role.id: + deactivate_assignment(assignment) RoleAssignment.objects.update_or_create( employee=employee, role=role, defaults={"is_active": True, "assigned_by": request.user}, ) else: - active_assignments = RoleAssignment.objects.filter( - employee_id=emp_id, role__department=dept, is_active=True - ) - active_assignments.update(is_active=False) + for assignment in active_assignments: + deactivate_assignment(assignment) return Response( { diff --git a/backend/api/v1/employees/views/roles.py b/backend/api/v1/employees/views/roles.py index ec30b71e..ef92b21c 100644 --- a/backend/api/v1/employees/views/roles.py +++ b/backend/api/v1/employees/views/roles.py @@ -75,7 +75,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): return qs def create(self, request, *args, **kwargs): - """Создание роли. Синхронизация LDAP требует реализации сигналов.""" + """Создание роли.""" ser = self.get_serializer(data=request.data) ser.is_valid(raise_exception=True) @@ -104,7 +104,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): ) def update(self, request, *args, **kwargs): - """Обновление роли. Синхронизация LDAP требует реализации сигналов.""" + """Обновление роли.""" partial = kwargs.pop("partial", False) instance = self.get_object() ser = self.get_serializer(instance, data=request.data, partial=partial) @@ -118,7 +118,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): return self.update(request, *args, **kwargs) def destroy(self, request, *args, **kwargs): - """Удаление роли. Синхронизация LDAP требует реализации сигналов.""" + """Удаление роли.""" instance = self.get_object() instance.delete() return Response(status=status.HTTP_204_NO_CONTENT) @@ -204,10 +204,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"]) def assign(self, request, pk=None): - """Назначает роль сотруднику. - - Синхронизация LDAP требует реализации сигналов. - """ + """Назначает роль сотруднику.""" role = self.get_object() employee_id = request.data.get("employee_id") @@ -244,10 +241,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"]) def revoke(self, request, pk=None): - """Отзывает роль у сотрудника. - - Синхронизация LDAP требует реализации сигналов. - """ + """Отзывает роль у сотрудника.""" role = self.get_object() employee_id = request.data.get("employee_id") @@ -259,7 +253,11 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet): except Employee.DoesNotExist: return Response({"detail": "Сотрудник не найден."}, status=404) - RoleAssignment.objects.filter(employee=employee, role=role).update( - is_active=False - ) + for assignment in RoleAssignment.objects.filter( + employee=employee, + role=role, + is_active=True, + ): + assignment.is_active = False + assignment.save(update_fields=["is_active"]) return Response(status=204) diff --git a/backend/employees/ldap/services/department_service.py b/backend/employees/ldap/services/department_service.py index 16456ffa..c02cf808 100644 --- a/backend/employees/ldap/services/department_service.py +++ b/backend/employees/ldap/services/department_service.py @@ -890,6 +890,46 @@ class DepartmentService(BaseService): except Exception as e: raise DirectoryDbError(str(e)) from e + def sync_role_state(self, role: DepartmentRole) -> None: + """Приводит LDAP-состояние роли к текущему состоянию БД.""" + self._ensure_runtime_services() + self._ensure_role_group(role) + + try: + dept_dn = self._get_department_dn(role.department) + except DirectoryServiceError: + dept_dn = None + self._reconcile_department_group(role.department, dept_dn) + + def sync_role_delete( + self, department: Department, role_group_dn: str | None = None + ) -> None: + """Удаляет LDAP-группу роли и пересобирает группу отдела.""" + self._ensure_runtime_services() + + if role_group_dn: + try: + self._load_ou_group(role_group_dn).delete() + except Exception: + pass + + try: + dept_dn = self._get_department_dn(department) + except DirectoryServiceError: + dept_dn = None + self._reconcile_department_group(department, dept_dn) + + def sync_role_assignment_state( + self, + employee: Employee, + role: DepartmentRole, + *, + is_active: bool, + ) -> None: + """Синхронизирует LDAP-членство сотрудника в группе роли.""" + self._ensure_runtime_services() + self._sync_role_membership(employee, role, add=is_active) + def set_member_role( self, dept: Department, employee: Employee, role ) -> None: @@ -1520,20 +1560,33 @@ class DepartmentService(BaseService): department_id=dept.id, is_active=True ).values_list("employee_id", flat=True) ) - if not active_emp_ids: + role_group_dns = [ + item + for item in DepartmentRole.objects.filter( + department_id=dept.id, + ) + .exclude(ldap_group_dn="") + .values_list("ldap_group_dn", flat=True) + if item + ] + if not active_emp_ids and not role_group_dns: group.member = [] group.save() return group_dn - dn_map = dict( - LdapSyncState.objects.filter( - model="employee", - object_pk__in=[str(i) for i in active_emp_ids], - ).values_list("object_pk", "ldap_dn") - ) - member_dns = [dn for dn in dn_map.values() if dn] + member_dns: List[str] = [] + if active_emp_ids: + dn_map = dict( + LdapSyncState.objects.filter( + model="employee", + object_pk__in=[str(i) for i in active_emp_ids], + ).values_list("object_pk", "ldap_dn") + ) + member_dns.extend(dn for dn in dn_map.values() if dn) + member_dns.extend(role_group_dns) - group.member = member_dns + deduped_member_dns = list(dict.fromkeys(member_dns)) + group.member = deduped_member_dns group.save() return group_dn diff --git a/backend/employees/signals/__init__.py b/backend/employees/signals/__init__.py index 96598968..5afa6294 100644 --- a/backend/employees/signals/__init__.py +++ b/backend/employees/signals/__init__.py @@ -20,6 +20,7 @@ if getattr(settings, 'LDAP_ENABLED', False): import_module('employees.signals.ldap.department') import_module('employees.signals.ldap.group') import_module('employees.signals.ldap.position') + import_module('employees.signals.ldap.role') __all__ = ['common', 'birthday'] else: __all__ = ['common', 'birthday'] diff --git a/backend/employees/signals/ldap/__init__.py b/backend/employees/signals/ldap/__init__.py index defc01ea..300f0229 100644 --- a/backend/employees/signals/ldap/__init__.py +++ b/backend/employees/signals/ldap/__init__.py @@ -1,5 +1,5 @@ """LDAP signals для автоматической синхронизации моделей с Active Directory.""" -from . import employee, department, group, position +from . import employee, department, group, position, role -__all__ = ['employee', 'department', 'group', 'position'] +__all__ = ['employee', 'department', 'group', 'position', 'role'] diff --git a/backend/employees/signals/ldap/role.py b/backend/employees/signals/ldap/role.py new file mode 100644 index 00000000..fb9274f6 --- /dev/null +++ b/backend/employees/signals/ldap/role.py @@ -0,0 +1,179 @@ +"""LDAP signals для DepartmentRole и RoleAssignment.""" + +import logging + +from django.conf import settings +from django.db.models.signals import post_delete, post_save +from django.dispatch import receiver + +from employees.ldap import DepartmentService +from employees.ldap.errors import ( + DirectoryDbError, + DirectoryLdapError, + DirectoryServiceError, +) +from employees.signals.ldap._queue import _enqueue + +logger = logging.getLogger(__name__) + + +def _is_ldap_enabled(): + return getattr(settings, "LDAP_ENABLED", False) + + +@receiver(post_save, sender="employees.DepartmentRole") +def sync_department_role_to_ldap_on_save(sender, instance, created, **kwargs): + del sender, created, kwargs + + if not _is_ldap_enabled(): + return + + if getattr(instance, "_skip_ldap_sync", False): + return + + try: + DepartmentService().sync_role_state(instance) + except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e: + logger.error( + "LDAP sync failed for DepartmentRole %s: %s", + instance.pk, + e, + exc_info=True, + ) + _enqueue( + "department_role_save", + "department_role", + instance.pk, + {"role_pk": str(instance.pk)}, + ) + except Exception as e: + logger.error( + "Unexpected LDAP sync error for DepartmentRole %s: %s", + instance.pk, + e, + exc_info=True, + ) + finally: + if hasattr(instance, "_skip_ldap_sync"): + delattr(instance, "_skip_ldap_sync") + + +@receiver(post_delete, sender="employees.DepartmentRole") +def sync_department_role_to_ldap_on_delete(sender, instance, **kwargs): + del sender, kwargs + + if not _is_ldap_enabled(): + return + + try: + DepartmentService().sync_role_delete( + instance.department, + role_group_dn=(instance.ldap_group_dn or "").strip() or None, + ) + except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e: + logger.error( + "LDAP delete failed for DepartmentRole %s: %s", + instance.pk, + e, + exc_info=True, + ) + _enqueue( + "department_role_delete", + "department_role", + instance.pk, + { + "department_pk": str(instance.department_id), + "role_group_dn": (instance.ldap_group_dn or "").strip(), + }, + ) + except Exception as e: + logger.error( + "Unexpected LDAP delete error for DepartmentRole %s: %s", + instance.pk, + e, + exc_info=True, + ) + + +@receiver(post_save, sender="employees.RoleAssignment") +def sync_role_assignment_to_ldap_on_save(sender, instance, created, **kwargs): + del sender, created, kwargs + + if not _is_ldap_enabled(): + return + + if getattr(instance, "_skip_ldap_sync", False): + return + + try: + DepartmentService().sync_role_assignment_state( + instance.employee, + instance.role, + is_active=instance.is_active, + ) + except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e: + logger.error( + "LDAP sync failed for RoleAssignment %s: %s", + instance.pk, + e, + exc_info=True, + ) + _enqueue( + "role_assignment", + "role_assignment", + instance.pk, + { + "employee_pk": str(instance.employee_id), + "role_pk": str(instance.role_id), + "is_active": bool(instance.is_active), + }, + ) + except Exception as e: + logger.error( + "Unexpected LDAP sync error for RoleAssignment %s: %s", + instance.pk, + e, + exc_info=True, + ) + finally: + if hasattr(instance, "_skip_ldap_sync"): + delattr(instance, "_skip_ldap_sync") + + +@receiver(post_delete, sender="employees.RoleAssignment") +def sync_role_assignment_to_ldap_on_delete(sender, instance, **kwargs): + del sender, kwargs + + if not _is_ldap_enabled(): + return + + try: + DepartmentService().sync_role_assignment_state( + instance.employee, + instance.role, + is_active=False, + ) + except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e: + logger.error( + "LDAP delete sync failed for RoleAssignment %s: %s", + instance.pk, + e, + exc_info=True, + ) + _enqueue( + "role_assignment", + "role_assignment", + instance.pk, + { + "employee_pk": str(instance.employee_id), + "role_pk": str(instance.role_id), + "is_active": False, + }, + ) + except Exception as e: + logger.error( + "Unexpected LDAP delete sync error for RoleAssignment %s: %s", + instance.pk, + e, + exc_info=True, + ) diff --git a/backend/employees/tasks.py b/backend/employees/tasks.py index 49d1635c..0df5d071 100644 --- a/backend/employees/tasks.py +++ b/backend/employees/tasks.py @@ -255,6 +255,76 @@ def _execute_position_delete(payload: dict) -> None: svc.delete_position_group(pos_stub) +def _execute_department_role_save(payload: dict) -> None: + """Повторяет синхронизацию роли отдела → LDAP.""" + from employees.ldap import DepartmentService + from employees.models import DepartmentRole + + role_pk = _require_payload_value(payload, "role_pk") + role = DepartmentRole.objects.filter(pk=role_pk).select_related( + "department" + ).first() + if role is None: + logger.info( + "DepartmentRole %s no longer exists, skipping queued role sync", + role_pk, + ) + return + + DepartmentService().sync_role_state(role) + + +def _execute_department_role_delete(payload: dict) -> None: + """Повторяет удаление LDAP-следов роли отдела.""" + from employees.ldap import DepartmentService + from employees.models import Department + + department_pk = _require_payload_value(payload, "department_pk") + dept = Department.objects.filter(pk=department_pk).first() + if dept is None: + logger.info( + "Department %s no longer exists, skipping queued role delete sync", + department_pk, + ) + return + + DepartmentService().sync_role_delete( + dept, + role_group_dn=payload.get("role_group_dn"), + ) + + +def _execute_role_assignment(payload: dict) -> None: + """Повторяет синхронизацию назначения роли → LDAP.""" + from employees.ldap import DepartmentService + from employees.models import DepartmentRole, Employee + + employee_pk = _require_payload_value(payload, "employee_pk") + role_pk = _require_payload_value(payload, "role_pk") + + employee = Employee.objects.filter(pk=employee_pk).first() + if employee is None: + logger.info( + "Employee %s no longer exists, skipping queued role assignment sync", + employee_pk, + ) + return + + role = DepartmentRole.objects.filter(pk=role_pk).first() + if role is None: + logger.info( + "DepartmentRole %s no longer exists, skipping queued role assignment sync", + role_pk, + ) + return + + DepartmentService().sync_role_assignment_state( + employee, + role, + is_active=bool(payload.get("is_active")), + ) + + # Реестр: operation → callable _EXECUTORS = { "employee_save": _execute_employee_save, @@ -267,6 +337,9 @@ _EXECUTORS = { "group_members": _execute_group_members, "position_save": _execute_position_save, "position_delete": _execute_position_delete, + "department_role_save": _execute_department_role_save, + "department_role_delete": _execute_department_role_delete, + "role_assignment": _execute_role_assignment, } diff --git a/backend/tests/api/v1/employees/test_department_roles.py b/backend/tests/api/v1/employees/test_department_roles.py index d1ffc6c2..5fc24e0c 100644 --- a/backend/tests/api/v1/employees/test_department_roles.py +++ b/backend/tests/api/v1/employees/test_department_roles.py @@ -2,8 +2,10 @@ import pytest from django.urls import reverse from django.contrib.auth import get_user_model +from django.test import override_settings from rest_framework.test import APIClient from rest_framework import status +from unittest.mock import patch from employees.models import ( Department, @@ -258,6 +260,29 @@ def test_create_with_scoped_permission_codes_and_update_name(api_client: APIClie format="json", ) assert resp.status_code == 201 + + +@pytest.mark.django_db +@override_settings(LDAP_ENABLED=True) +def test_create_role_triggers_ldap_sync(api_client: APIClient): + with override_settings(LDAP_ENABLED=False): + d = Department.objects.create(name="Dept LDAP") + staff = make_user("staff-ldap@example.com", staff=True) + + api_client.force_authenticate(user=staff) + url = reverse("api:v1:department-roles-list") + + with patch( + "employees.signals.ldap.role.DepartmentService.sync_role_state" + ) as mock_sync: + resp = api_client.post( + url, + {"department": d.id, "name": "Worker LDAP"}, + format="json", + ) + + assert resp.status_code == status.HTTP_201_CREATED + mock_sync.assert_called_once() rid = resp.json()["id"] # update имени (PATCH) diff --git a/backend/tests/api/v1/employees/test_role_assignment.py b/backend/tests/api/v1/employees/test_role_assignment.py index 9334d182..428c2ec1 100644 --- a/backend/tests/api/v1/employees/test_role_assignment.py +++ b/backend/tests/api/v1/employees/test_role_assignment.py @@ -2,8 +2,10 @@ Тесты для новых endpoints назначения ролей: assign, revoke, assignments. """ import pytest +from django.test import override_settings from rest_framework import status from rest_framework.test import APIClient +from unittest.mock import patch from django.contrib.auth import get_user_model from employees.models import ( @@ -165,9 +167,35 @@ class TestRoleAssignmentEndpoints: api_client.force_authenticate(user) url = f"/api/v1/department-roles/{role.id}/assign/" resp = api_client.post(url, {"employee_id": other_user.id}) - + assert resp.status_code == status.HTTP_201_CREATED + @override_settings(LDAP_ENABLED=True) + def test_assign_and_revoke_trigger_ldap_sync(self, api_client: APIClient): + with override_settings(LDAP_ENABLED=False): + admin = make_user("admin-ldap@test.com", is_staff=True) + dept = make_dept("IT LDAP") + role = make_role(dept, "Developer LDAP", []) + user = make_user("worker-ldap@test.com") + + api_client.force_authenticate(admin) + url_assign = f"/api/v1/department-roles/{role.id}/assign/" + url_revoke = f"/api/v1/department-roles/{role.id}/revoke/" + + with patch( + "employees.signals.ldap.role.DepartmentService.sync_role_assignment_state" + ) as mock_sync: + assign_resp = api_client.post(url_assign, {"employee_id": user.id}) + revoke_resp = api_client.post(url_revoke, {"employee_id": user.id}) + + assert assign_resp.status_code == status.HTTP_201_CREATED + assert revoke_resp.status_code == status.HTTP_204_NO_CONTENT + assert mock_sync.call_count >= 2 + first_call = mock_sync.call_args_list[0] + second_call = mock_sync.call_args_list[1] + assert first_call.kwargs["is_active"] is True + assert second_call.kwargs["is_active"] is False + def test_department_members_include_role_only_employees(self, api_client: APIClient): """members endpoint показывает сотрудников с ролью в отделе даже без membership.""" admin = make_user("admin-members@test.com", is_staff=True) diff --git a/backend/tests/employees/test_department_role_ldap_sync.py b/backend/tests/employees/test_department_role_ldap_sync.py new file mode 100644 index 00000000..194257cc --- /dev/null +++ b/backend/tests/employees/test_department_role_ldap_sync.py @@ -0,0 +1,44 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from employees.ldap.services.department_service import DepartmentService +from employees.models import Department, DepartmentRole + + +@pytest.mark.django_db +def test_sync_role_state_nests_role_group_into_department_group(): + dept = Department.objects.create(name="LDAP Dept") + role = DepartmentRole.objects.create(department=dept, name="Reviewer") + dept_dn = "OU=LDAP Dept,OU=Departments,DC=example,DC=local" + role_group_dn = "CN=ROLE_Reviewer,OU=LDAP Dept,OU=Departments,DC=example,DC=local" + fake_group = SimpleNamespace(member=[], save=MagicMock()) + + def ensure_role_group_side_effect(role_obj): + DepartmentRole.objects.filter(pk=role_obj.pk).update( + ldap_group_dn=role_group_dn + ) + role_obj.ldap_group_dn = role_group_dn + return role_group_dn + + with patch.object( + DepartmentService, + "_get_department_dn", + return_value=dept_dn, + ), patch.object( + DepartmentService, + "_ensure_role_group", + side_effect=ensure_role_group_side_effect, + ), patch.object( + DepartmentService, + "_ensure_department_group", + return_value=f"CN=DEP_{dept.name},{dept_dn}", + ), patch( + "employees.ldap.services.department_service.LdapOrganizationalUnitGroup.objects.get", + return_value=fake_group, + ): + DepartmentService().sync_role_state(role) + + assert fake_group.member == [role_group_dn] + fake_group.save.assert_called_once() diff --git a/backend/tests/employees/test_ldap_sync_queue.py b/backend/tests/employees/test_ldap_sync_queue.py index 260e019b..990206d1 100644 --- a/backend/tests/employees/test_ldap_sync_queue.py +++ b/backend/tests/employees/test_ldap_sync_queue.py @@ -377,6 +377,88 @@ class TestDepartmentQueueExecutors: ) +class TestDepartmentRoleQueueExecutors: + + @override_settings(LDAP_ENABLED=True) + def test_department_role_retry_reconciles_role_group( + self, db + ): + from employees.models import Department, DepartmentRole, LdapSyncQueue + from employees.tasks import process_ldap_queue_item + + with override_settings(LDAP_ENABLED=False): + department = Department.objects.create(name="Dept Role Queue") + role = DepartmentRole.objects.create( + department=department, + name="Reviewer", + ) + + item = LdapSyncQueue.objects.create( + operation="department_role_save", + model_name="department_role", + object_pk=str(role.pk), + payload={"role_pk": str(role.pk)}, + ) + + with patch( + "employees.ldap.services.department_service.DepartmentService.sync_role_state" + ) as mock_sync: + process_ldap_queue_item(item.pk) + + item.refresh_from_db() + assert item.status == LdapSyncQueue.Status.COMPLETED + mock_sync.assert_called_once_with(role) + + @override_settings(LDAP_ENABLED=True) + def test_role_assignment_retry_reconciles_ldap_membership( + self, db, user_factory + ): + from employees.models import ( + Department, + DepartmentRole, + LdapSyncQueue, + RoleAssignment, + ) + from employees.tasks import process_ldap_queue_item + + with override_settings(LDAP_ENABLED=False): + employee = user_factory(email="role-sync@example.com") + department = Department.objects.create(name="Dept Role Assignment") + role = DepartmentRole.objects.create( + department=department, + name="Operator", + ) + assignment = RoleAssignment.objects.create( + employee=employee, + role=role, + is_active=True, + ) + + item = LdapSyncQueue.objects.create( + operation="role_assignment", + model_name="role_assignment", + object_pk=str(assignment.pk), + payload={ + "employee_pk": str(employee.pk), + "role_pk": str(role.pk), + "is_active": True, + }, + ) + + with patch( + "employees.ldap.services.department_service.DepartmentService.sync_role_assignment_state" + ) as mock_sync: + process_ldap_queue_item(item.pk) + + item.refresh_from_db() + assert item.status == LdapSyncQueue.Status.COMPLETED + mock_sync.assert_called_once_with( + employee, + role, + is_active=True, + ) + + # --------------------------------------------------------------------------- # Сигналы: проверка enqueue при ошибке LDAP # ---------------------------------------------------------------------------