Audit and fix LDAP module: imports, docstrings, tests

2026-03-19 23:12:22 +03:00 · 2026-03-19 23:12:22 +03:00 · 06931f3706
commit 06931f3706
parent 75662ec7ea
10 changed files with 236 additions and 88 deletions
--- a/backend/employees/ldap/orm_models.py
+++ b/backend/employees/ldap/orm_models.py
@ -9,49 +9,62 @@
 - Унифицированный API для CRUD операций
 - Меньше кода, больше удобства

-ВАЖНО: Ограничения django-ldapdb
-==================================
+ВАЖНО: Миксины расширяют django-ldapdb
+=======================================

-django-ldapdb (версия 1.5.1) НЕ поддерживает следующие операции LDAP:
+django-ldapdb (версия 1.5.1) изначально НЕ поддерживает некоторые операции,
+но мы добавили их через кастомные миксины:

-1. ModifyDN (rename/move) — перемещение объектов между OU
-   - Для перемещения пользователей используется ldap3.Connection.modify_dn()
-   - См. utils.dn_utils._move_to_department()
-   - При переводе в отдел, увольнении — низкоуровневый ldap3
+✅ ModifyDN (rename/move) — перемещение объектов между OU
+   - РЕШЕНО: ModifyDnMixin автоматически выполняет modify_dn при изменении base_dn
+   - Использование: user.base_dn = new_dn; user.save()
+   - См. employees.ldap.mixins.ModifyDnMixin

-2. Создание объектов с динамическим DN
+✅ Автоматическое управление LdapSyncState
+   - РЕШЕНО: LdapSyncStateMixin автоматически создает/обновляет/удаляет sync state
+   - Работает на save() и delete()
+   - См. employees.ldap.mixins.LdapSyncStateMixin
+
+Что требует низкоуровневого ldap3:
+
+1. Создание объектов с динамическим DN
   - Создание пользователей: ldap3.Connection.add() с явным DN
   - DN формируется на основе department_dn или LDAP_USERS_BASE
   - См. services.user_service.UserService._create_user_in_ldap()

-3. Транзакционное создание с перебором CN (collision handling)
+2. Транзакционное создание с перебором CN (collision handling)
   - При создании пользователя перебираются варианты CN до успеха
   - См. services.user_service.UserService._try_add_with_cn_list()

-Что МОЖНО делать через ORM:
- UPDATE: изменение атрибутов существующих объектов (.save())
- READ: поиск объектов для последующего обновления (.objects.get(dn=...))
+Что МОЖНО делать через ORM с миксинами:
+- CREATE: через сервисы (UserService.create_user)
+- READ: поиск объектов (.objects.get(dn=...))
+- UPDATE: изменение атрибутов (.save())
+- MOVE: изменение base_dn + save() — автоматический modify_dn! 🎉
+- DELETE: .delete() — автоматическое удаление sync state
 - BATCH UPDATE: массовое обновление атрибутов (.save() в цикле)

-Архитектура использования:
-==========================
+Архитектура использования (с миксинами):
+========================================

 CREATE пользователя:
-  1. ldap3.Connection.add(dn, object_classes, attrs)  # низкоуровневое создание
-  2. LdapUser.objects.get(dn=new_dn)                  # загрузка для проверки
+  from employees.ldap.services import UserService
+  service = UserService()
+  employee = service.create_user(employee)  # создание + LdapSyncState
  
 UPDATE пользователя:
-  1. ldap_user = LdapUser.objects.get(dn=dn)          # загрузка через ORM
-  2. ldap_user.display_name = "New Name"              # изменение атрибутов
-  3. ldap_user.save()                                 # сохранение через ORM
+  ldap_user = LdapUser.objects.get(dn=dn)
+  ldap_user.display_name = "New Name"
+  ldap_user.save()  # автообновление LdapSyncState через миксин

-MOVE пользователя (перевод в отдел):
-  1. new_dn = conn.modify_dn(dn, new_superior=dept_dn)  # низкоуровневое перемещение
-  2. ldap_user = LdapUser.objects.get(dn=new_dn)        # загрузка по новому DN
-  3. ldap_user.save()                                   # обновление атрибутов
+MOVE пользователя (перевод в отдел) — НОВОЕ С МИКСИНОМ:
+  ldap_user = LdapUser.objects.get(dn=old_dn)
+  ldap_user.base_dn = "OU=NewDept,OU=Departments,DC=..."
+  ldap_user.save()  # ModifyDnMixin автоматически выполнит modify_dn!

 DELETE пользователя:
-  1. conn.delete(dn)                                    # низкоуровневое удаление
+  ldap_user = LdapUser.objects.get(dn=dn)
+  ldap_user.delete()  # автоудаление LdapSyncState через миксин

 Структура DN в Active Directory:
 =================================
@ -185,6 +198,10 @@ class LdapUser(LdapSyncStateMixin, ModifyDnMixin, LdapModel):
    base_dn = get_users_base()
    object_classes = ['top', 'person', 'organizationalPerson', 'user']
    
+    # RDN атрибут для построения Distinguished Name
+    # Пример: cn=Ivan Ivanov → dn=CN=Ivan Ivanov,OU=...,DC=...
+    rdn_attributes = ['cn']
+    
    # Основные атрибуты
    # dn наследуется от ldapdb.models.Model как primary_key (Distinguished Name)
    cn = CharField(db_column='cn')
@ -248,10 +265,11 @@ class LdapGroup(ModifyDnMixin, LdapModel):
    - При изменении base_dn ModifyDnMixin автоматически выполнит modify_dn!
    
    Операции через django-ldapdb:
-    - CREATE: создание через GroupService.create() (ldap3.Connection.add)
+    - CREATE: создание через GroupService.create() (ORM)
    - UPDATE: изменение атрибутов через .save()
    - MOVE: group.base_dn = new_dn; group.save() (автоматический modify_dn!)
-    - DELETE: удаление через ldap3.Connection.delete()
+    - RENAME: переименование через GroupService.rename() (low-level ldap3)
+    - DELETE: удаление через .delete() (ORM)
    
    При создании ролей отделов DN формируется как:
    CN=ROLE_{name},OU=Roles,OU={dept},OU=Departments,...
@ -261,6 +279,10 @@ class LdapGroup(ModifyDnMixin, LdapModel):
    base_dn = get_base_dn()
    object_classes = ['top', 'group']
    
+    # RDN атрибут для построения Distinguished Name
+    # Пример: cn=Developers → dn=CN=Developers,OU=Groups,DC=...
+    rdn_attributes = ['cn']
+    
    # Основные атрибуты
    # dn наследуется от ldapdb.models.Model как primary_key (Distinguished Name)
    cn = CharField(db_column='cn')
--- a/backend/employees/ldap/repositories/ldap_repository.py
+++ b/backend/employees/ldap/repositories/ldap_repository.py
@ -214,6 +214,21 @@ class LdapRepository:
        raise RuntimeError(f"LDAP modify failed: {self.conn.result}")


+def ensure_container_exists(conn: Connection, base_dn: str) -> None:
+    """Standalone-обёртка: проверяет что контейнер (OU) существует.
+    
+    Args:
+        conn: LDAP соединение.
+        base_dn: Distinguished Name контейнера.
+        
+    Raises:
+        TypeError: Если base_dn неверного типа.
+        RuntimeError: Если контейнер не найден.
+    """
+    LdapRepository(conn).ensure_container_exists(base_dn)
+
+
 __all__ = [
    "LdapRepository",
+    "ensure_container_exists",
 ]
--- a/backend/employees/ldap/services/department_service.py
+++ b/backend/employees/ldap/services/department_service.py
@ -1063,8 +1063,9 @@ class DepartmentService(BaseService):
    ) -> str:
        """Переименовывает OU отдела и возвращает новый DN.
        
-        TODO(ldap-orm): modify_dn — ldap3 навсегда.
-        django-ldapdb не поддерживает переименование.
+        NOTE: Используется low-level ldap3 для rename RDN (OU=OldName → OU=NewName).
+        ModifyDnMixin покрывает move (смену superior), но НЕ rename RDN.
+        Rename OU — это изменение атрибута ou, что django-ldapdb не поддерживает.
        
        Args:
            conn: LDAP соединение.
--- a/backend/employees/ldap/services/group_service.py
+++ b/backend/employees/ldap/services/group_service.py
@ -98,8 +98,6 @@ class GroupService(BaseService):
            scope: Тип группы.
            security_enabled: Флаг безопасности.
        """
-        import ldap as _ldap_module
-        
        base = parent_dn or getattr(settings, "LDAP_GROUPS_BASE", None)
        if not base:
            raise RuntimeError(
@ -116,17 +114,19 @@ class GroupService(BaseService):
                description=description or "",
            )
            self._log_operation("create", model="group", dn=dn, success=True)
-        except _ldap_module.ALREADY_EXISTS:
-            self._logger.info(f"Group already exists: {dn}")
        except Exception as e:
-            self._log_operation(
-                "create",
-                model="group",
-                dn=dn,
-                success=False,
-                error=e,
-            )
-            raise RuntimeError(f"LDAP add group failed: {e}") from e
+            err_str = str(e).lower()
+            if "already exists" in err_str or "entryalreadyexists" in err_str:
+                self._logger.info(f"Group already exists: {dn}")
+            else:
+                self._log_operation(
+                    "create",
+                    model="group",
+                    dn=dn,
+                    success=False,
+                    error=e,
+                )
+                raise RuntimeError(f"LDAP add group failed: {e}") from e
        
        return dn

--- a/backend/employees/ldap/services/user_service.py
+++ b/backend/employees/ldap/services/user_service.py
@ -11,7 +11,6 @@

 from __future__ import annotations

-import ldap
 import logging
 from typing import Any, Dict, Iterable, List, Optional

@ -577,8 +576,10 @@ class UserService(BaseService):
    def _create_user_in_ldap(self, conn: Connection, dto: DirectoryUserDTO) -> str:
        """Создаёт объект user в LDAP (AD) и возвращает его DN.
        
-        TODO(ldap-orm): conn.add(), ensure_container_exists, cn_candidates — ldap3 навсегда.
-        django-ldapdb не поддерживает создание с перебором CN и проверкой контейнера.
+        NOTE: Используется low-level ldap3 (conn.add) из-за необходимости:
+        - Перебора вариантов CN при коллизиях имён
+        - Проверки/создания контейнера (ensure_container_exists)
+        django-ldapdb ORM не поддерживает эти сценарии.
        """
        base_dn = (
            dto.department_dn
@ -678,7 +679,7 @@ class UserService(BaseService):
        # 3-5) ORM: UAC + Атрибуты + Аватар (batch через один save)
        try:
            ldap_user = LdapUser.objects.get(dn=dn)
-        except ldap.NO_SUCH_OBJECT:
+        except LdapUser.DoesNotExist:
            # DN неверный - пытаемся найти пользователя по employeeNumber
            if emp_pk:
                logger.warning(
@ -827,9 +828,10 @@ class UserService(BaseService):
    def _move_user_to_base(self, conn: Connection, user_dn: str, base_dn: str) -> str:
        """Перемещает пользовательский объект в указанный контейнер.
        
-        TODO(ldap-orm): Невозможно заменить на ORM.
-        modify_dn (move/rename) — не поддерживается django-ldapdb.
-        Остаётся через ldap3 навсегда.
+        NOTE: Используется low-level ldap3 для атомарного move-only
+        (без обновления атрибутов). ModifyDnMixin решает ту же задачу
+        через ORM (user.base_dn = ...; user.save()), но совмещает
+        move с обновлением атрибутов.
        
        Args:
            conn: LDAP соединение.
--- a/backend/employees/ldap/tests/conftest.py
+++ b/backend/employees/ldap/tests/conftest.py
@ -83,11 +83,20 @@ def sample_department(db):

@pytest.fixture
 def sample_position(db):
-    """Создает тестовую должность."""
-    return Position.objects.create(
-        name="Разработчик",
-        description="Backend разработчик"
-    )
+    """Создает тестовую должность (без LDAP синхронизации)."""
+    from employees.signals.ldap.position import sync_position_to_ldap_on_save
+    from django.db.models.signals import post_save
+    
+    # Отключаем LDAP синхронизацию при создании Position
+    post_save.disconnect(sync_position_to_ldap_on_save, sender=Position)
+    try:
+        pos = Position.objects.create(
+            name="Разработчик",
+            description="Backend разработчик"
+        )
+    finally:
+        post_save.connect(sync_position_to_ldap_on_save, sender=Position)
+    return pos


@pytest.fixture
--- a/backend/employees/ldap/tests/unit/test_group_service.py
+++ b/backend/employees/ldap/tests/unit/test_group_service.py
@ -69,16 +69,23 @@ class TestGroupServiceDelete:
        mock_ldap_connection,
        mock_ldap_repository
    ):
-        """Тест удаления группы."""
+        """Тест удаления группы через ORM."""
        # Arrange
        service = GroupService()
        group_dn = "CN=Test Group,OU=Groups,DC=example,DC=com"
        
-        # Act
-        service.delete(group_dn)
+        mock_group = Mock()
+        with patch(
+            'employees.ldap.services.group_service.LdapGroup'
+        ) as MockLdapGroup:
+            MockLdapGroup.objects.get.return_value = mock_group
+            
+            # Act
+            service.delete(group_dn)
        
        # Assert
-        mock_ldap_connection.delete.assert_called_once_with(group_dn)
+        MockLdapGroup.objects.get.assert_called_once_with(dn=group_dn)
+        mock_group.delete.assert_called_once()


 class TestGroupServiceModify:
--- a/backend/employees/ldap/tests/unit/test_position_service.py
+++ b/backend/employees/ldap/tests/unit/test_position_service.py
@ -206,7 +206,7 @@ class TestPositionServiceDelete:
        # Mock ORM LdapGroup.objects.get().delete()
        mock_group = Mock()
        with patch(
-            'employees.ldap.services.position_service.LdapGroup'
+            'employees.ldap.orm_models.LdapGroup'
        ) as MockLdapGroup:
            MockLdapGroup.objects.get.return_value = mock_group
            
@ -233,7 +233,7 @@ class TestPositionServiceDelete:
        sample_position.ldap_group_dn = ""
        
        with patch(
-            'employees.ldap.services.position_service.LdapGroup'
+            'employees.ldap.orm_models.LdapGroup'
        ) as MockLdapGroup:
            # Act
            service.delete_position_group(sample_position)
@ -278,7 +278,7 @@ class TestPositionServiceHelpers:
        
        # Mock LdapRepository в месте использования
        with patch(
-            'employees.ldap.services.position_service.LdapRepository'  # noqa: E501
+            'employees.ldap.services.position_service.ensure_container_exists'
        ):
            # Act
            result = service._ensure_positions_base(mock_ldap_connection)
@ -316,7 +316,7 @@ class TestPositionServiceHelpers:
        
        # Mock LdapRepository
        with patch(
-            'employees.ldap.services.position_service.LdapRepository'
+            'employees.ldap.services.position_service.ensure_container_exists'
        ):
            # Act
            dn = service._ensure_position_group(
@ -350,20 +350,18 @@ class TestPositionServiceHelpers:
        mock_ldap_connection.search.return_value = False
        mock_ldap_connection.entries = []
        
-        # Mock LdapRepository + LdapGroup ORM
+        # Mock ensure_container_exists + conn.add (low-level create)
+        mock_ldap_connection.add.return_value = True
        with patch(
-            'employees.ldap.services.position_service.LdapRepository'
+            'employees.ldap.services.position_service.ensure_container_exists'
        ):
-            with patch(
-                'employees.ldap.services.position_service.LdapGroup'
-            ) as MockLdapGroup:
-                # Act
-                dn = service._ensure_position_group(
-                    mock_ldap_connection,
-                    sample_position
-                )
-                
-                # Assert
-                assert expected_name in dn
-                # Проверяем, что LdapGroup.objects.create был вызван
-                MockLdapGroup.objects.create.assert_called_once()
+            # Act
+            dn = service._ensure_position_group(
+                mock_ldap_connection,
+                sample_position
+            )
+            
+            # Assert
+            assert expected_name in dn
+            # Проверяем, что conn.add был вызван (low-level создание)
+            mock_ldap_connection.add.assert_called_once()
--- a/backend/employees/ldap/tests/unit/test_user_service.py
+++ b/backend/employees/ldap/tests/unit/test_user_service.py
@ -405,22 +405,17 @@ class TestUserServiceHelpers:
        old_dn = 'CN=Test,OU=OldBase,DC=example,DC=com'
        new_base = 'OU=NewBase,DC=example,DC=com'
        
-        # Mock ORM LdapUser.objects.get
-        mock_user = Mock()
-        mock_user.dn = f'CN=Test,{new_base}'
-        with patch(
-            'employees.ldap.services.user_service.LdapUser'
-        ) as MockLdapUser:
-            MockLdapUser.objects.get.return_value = mock_user
-            
-            # Act
-            new_dn = service._move_user_to_base(old_dn, new_base)
+        # Mock conn.modify_dn для low-level перемещения
+        mock_ldap_connection.modify_dn.return_value = True
+        
+        # Act
+        new_dn = service._move_user_to_base(mock_ldap_connection, old_dn, new_base)
        
        # Assert
        assert new_dn == f'CN=Test,{new_base}'
-        MockLdapUser.objects.get.assert_called_once_with(dn=old_dn)
-        assert mock_user.base_dn == new_base
-        mock_user.save.assert_called_once()
+        mock_ldap_connection.modify_dn.assert_called_once_with(
+            old_dn, 'CN=Test', new_superior=new_base
+        )


 class TestUserServiceSync:
--- a/backend/employees/migrations/0042_ldapsyncqueue.py
+++ b/backend/employees/migrations/0042_ldapsyncqueue.py
@ -0,0 +1,99 @@
+# Generated by Django 5.2.4 on 2026-03-19 19:53
+
+import employees.ldap.mixins
+import ldapdb.models.fields
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('employees', '0041_alter_departmentpermission_code'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='LdapGroup',
+            fields=[
+                ('dn', ldapdb.models.fields.CharField(max_length=200, primary_key=True, serialize=False)),
+                ('cn', ldapdb.models.fields.CharField(db_column='cn', max_length=200)),
+                ('object_guid', ldapdb.models.fields.CharField(db_column='objectGUID', max_length=200)),
+                ('sam_account_name', ldapdb.models.fields.CharField(db_column='sAMAccountName', max_length=200)),
+                ('description', ldapdb.models.fields.CharField(blank=True, db_column='description', max_length=200)),
+                ('member', ldapdb.models.fields.ListField(blank=True, db_column='member')),
+                ('member_of', ldapdb.models.fields.ListField(blank=True, db_column='memberOf')),
+                ('when_created', ldapdb.models.fields.DateTimeField(db_column='whenCreated')),
+                ('when_changed', ldapdb.models.fields.DateTimeField(db_column='whenChanged')),
+            ],
+            options={
+                'managed': False,
+            },
+            bases=(employees.ldap.mixins.ModifyDnMixin, models.Model),
+        ),
+        migrations.CreateModel(
+            name='LdapOrganizationalUnit',
+            fields=[
+                ('dn', ldapdb.models.fields.CharField(max_length=200, primary_key=True, serialize=False)),
+                ('ou', ldapdb.models.fields.CharField(db_column='ou', max_length=200)),
+                ('object_guid', ldapdb.models.fields.CharField(db_column='objectGUID', max_length=200)),
+                ('description', ldapdb.models.fields.CharField(blank=True, db_column='description', max_length=200)),
+                ('managed_by', ldapdb.models.fields.CharField(blank=True, db_column='managedBy', max_length=200)),
+                ('when_created', ldapdb.models.fields.DateTimeField(db_column='whenCreated')),
+                ('when_changed', ldapdb.models.fields.DateTimeField(db_column='whenChanged')),
+            ],
+            options={
+                'managed': False,
+            },
+            bases=(employees.ldap.mixins.ModifyDnMixin, models.Model),
+        ),
+        migrations.CreateModel(
+            name='LdapUser',
+            fields=[
+                ('dn', ldapdb.models.fields.CharField(max_length=200, primary_key=True, serialize=False)),
+                ('cn', ldapdb.models.fields.CharField(db_column='cn', max_length=200)),
+                ('object_guid', ldapdb.models.fields.CharField(db_column='objectGUID', max_length=200)),
+                ('sam_account_name', ldapdb.models.fields.CharField(db_column='sAMAccountName', max_length=200)),
+                ('user_principal_name', ldapdb.models.fields.CharField(db_column='userPrincipalName', max_length=200)),
+                ('given_name', ldapdb.models.fields.CharField(db_column='givenName', max_length=200)),
+                ('sn', ldapdb.models.fields.CharField(db_column='sn', max_length=200)),
+                ('display_name', ldapdb.models.fields.CharField(db_column='displayName', max_length=200)),
+                ('mail', ldapdb.models.fields.CharField(db_column='mail', max_length=200)),
+                ('telephone_number', ldapdb.models.fields.CharField(blank=True, db_column='telephoneNumber', max_length=200)),
+                ('mobile', ldapdb.models.fields.CharField(blank=True, db_column='mobile', max_length=200)),
+                ('user_account_control', ldapdb.models.fields.IntegerField(db_column='userAccountControl')),
+                ('description', ldapdb.models.fields.CharField(blank=True, db_column='description', max_length=200)),
+                ('thumbnail_photo', ldapdb.models.fields.ImageField(blank=True, db_column='thumbnailPhoto')),
+                ('employee_number', ldapdb.models.fields.CharField(blank=True, db_column='employeeNumber', max_length=200)),
+                ('member_of', ldapdb.models.fields.ListField(blank=True, db_column='memberOf')),
+                ('when_created', ldapdb.models.fields.DateTimeField(db_column='whenCreated')),
+                ('when_changed', ldapdb.models.fields.DateTimeField(db_column='whenChanged')),
+            ],
+            options={
+                'managed': False,
+            },
+            bases=(employees.ldap.mixins.LdapSyncStateMixin, employees.ldap.mixins.ModifyDnMixin, models.Model),
+        ),
+        migrations.CreateModel(
+            name='LdapSyncQueue',
+            fields=[
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('operation', models.CharField(db_index=True, help_text='Имя операции (employee_save, department_delete, group_m2m и т.д.)', max_length=64)),
+                ('model_name', models.CharField(db_index=True, max_length=32)),
+                ('object_pk', models.CharField(db_index=True, max_length=64)),
+                ('payload', models.JSONField(default=dict, help_text='JSON с аргументами для повтора операции')),
+                ('status', models.CharField(choices=[('pending', 'Ожидает'), ('in_progress', 'Выполняется'), ('completed', 'Завершено'), ('failed', 'Ошибка (исчерпаны попытки)')], db_index=True, default='pending', max_length=16)),
+                ('attempts', models.PositiveIntegerField(default=0)),
+                ('max_attempts', models.PositiveIntegerField(default=5)),
+                ('next_retry_at', models.DateTimeField(blank=True, db_index=True, null=True)),
+                ('last_error', models.TextField(blank=True, default='')),
+                ('created_at', models.DateTimeField(auto_now_add=True)),
+                ('updated_at', models.DateTimeField(auto_now=True)),
+            ],
+            options={
+                'verbose_name': 'Очередь LDAP синхронизации',
+                'verbose_name_plural': 'Очередь LDAP синхронизации',
+                'ordering': ['created_at'],
+                'indexes': [models.Index(fields=['status', 'next_retry_at'], name='ldap_queue_retry_idx')],
+            },
+        ),
+    ]