From 06931f3706a97dcda200759a856776026fce0197 Mon Sep 17 00:00:00 2001 From: lizerk Date: Thu, 19 Mar 2026 23:12:22 +0300 Subject: [PATCH] Audit and fix LDAP module: imports, docstrings, tests --- backend/employees/ldap/orm_models.py | 74 +++++++++----- .../ldap/repositories/ldap_repository.py | 15 +++ .../ldap/services/department_service.py | 5 +- .../employees/ldap/services/group_service.py | 24 ++--- .../employees/ldap/services/user_service.py | 16 +-- backend/employees/ldap/tests/conftest.py | 19 +++- .../ldap/tests/unit/test_group_service.py | 15 ++- .../ldap/tests/unit/test_position_service.py | 36 ++++--- .../ldap/tests/unit/test_user_service.py | 21 ++-- .../migrations/0042_ldapsyncqueue.py | 99 +++++++++++++++++++ 10 files changed, 236 insertions(+), 88 deletions(-) create mode 100644 backend/employees/migrations/0042_ldapsyncqueue.py diff --git a/backend/employees/ldap/orm_models.py b/backend/employees/ldap/orm_models.py index 23829e51..5a67cf73 100644 --- 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') diff --git a/backend/employees/ldap/repositories/ldap_repository.py b/backend/employees/ldap/repositories/ldap_repository.py index fc7ee856..6e56e447 100644 --- 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", ] diff --git a/backend/employees/ldap/services/department_service.py b/backend/employees/ldap/services/department_service.py index 0123cf32..e21a9956 100644 --- 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 соединение. diff --git a/backend/employees/ldap/services/group_service.py b/backend/employees/ldap/services/group_service.py index c0f53d8c..43021692 100644 --- 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 diff --git a/backend/employees/ldap/services/user_service.py b/backend/employees/ldap/services/user_service.py index 8c8a3005..63eff10e 100644 --- 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 соединение. diff --git a/backend/employees/ldap/tests/conftest.py b/backend/employees/ldap/tests/conftest.py index 843ed39f..973f4ec3 100644 --- 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 diff --git a/backend/employees/ldap/tests/unit/test_group_service.py b/backend/employees/ldap/tests/unit/test_group_service.py index 0f4b73b4..4178045e 100644 --- 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: diff --git a/backend/employees/ldap/tests/unit/test_position_service.py b/backend/employees/ldap/tests/unit/test_position_service.py index 025fc31c..58b402fd 100644 --- 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() diff --git a/backend/employees/ldap/tests/unit/test_user_service.py b/backend/employees/ldap/tests/unit/test_user_service.py index 7dd2b1e8..190b6fb4 100644 --- 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: diff --git a/backend/employees/migrations/0042_ldapsyncqueue.py b/backend/employees/migrations/0042_ldapsyncqueue.py new file mode 100644 index 00000000..fdd6f79e --- /dev/null +++ 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')], + }, + ), + ]