- Department move: set ldap_user.base_dn via ModifyDnMixin (replaces DirectoryService)
- Position sync: assign/unassign POS-groups via PositionService on position change
- DB update: position_id saved to Employee in atomic block
- Best-effort: position sync errors logged but don't interrupt update
- Removed 30 lines of commented-out DirectoryService code
- Removed unused Position import
- Removed unused old_pos/new_pos/pos_in_payload variables
- Fixed stale comment about displayName auto-update in save()
- Position sync remains as a 3-line TODO comment
AD forbids modifying RDN (CN/OU) via modify_s - only via rename.
ModifyDnMixin now:
1. Tracks _original_rdn_value (cn/ou) from __init__/from_db
2. On save(), if RDN changed: reverts to old value → save() → modify_dn
3. This fixes NOT_ALLOWED_ON_RDN for both admin and API edits
Simplified user_service: just set ldap_user.cn = new_cn and call save().
The mixin handles the rest automatically.
AD forbids modifying RDN (CN) through modify_s - must use rename.
- Removed LdapUser.save() override that synced cn→displayName
(this caused cn to be in modlist → NOT_ALLOWED_ON_RDN error)
- user_service now sets displayName explicitly instead of cn
- CN is only changed via conn.modify_dn() after save()
- Flow: set displayName + other attrs → save() → modify_dn(cn)
- Added save() override in LdapUser to auto-update displayName from cn
- Fixed user_service to set ldap_user.cn BEFORE save() (not after)
- This ensures displayName is synced when givenName/sn change
- Previously displayName wasn't updated because cn was set after save()
- Now: set cn → save() → displayName auto-synced → modify_dn()
dict(QueryDict) wraps every value in a list: {'first_name': ['Вадимин']}
QueryDict.dict() returns single values: {'first_name': 'Вадимин'}
This caused LDAP fields to be saved as "['Вадимин']" instead of 'Вадимин'
because the list was stringified by ldapdb before writing to AD.
ldapdb's build_rdn() checks field.primary_key AND field.db_column,
but the 'dn' field has primary_key=True and db_column=None,
while 'cn'/'ou' fields have db_column set but primary_key=False.
No field satisfies both conditions → save() always raises
'Could not build Distinguished Name'.
Fix: ModifyDnMixin.build_rdn() extracts RDN from existing DN
for loaded objects, and builds from rdn_attributes for new ones.
- Moved get_ldap_str() helper to ldap_utils.py for reuse
- Updated user_service.py to use shared helper from utils
- Updated admin.py to safely extract string values from LDAP fields
- LDAP fields can be lists, strings, or None - helper handles all cases
- Fixes sync_from_ldap_to_django and compare_ldap_django actions
- Prevents 'list' object errors when comparing or assigning LDAP fields
- Added _get_ldap_str() helper to safely extract string values from LDAP fields
- LDAP fields (given_name, sn, etc.) can be returned as lists, not strings
- Fixes AttributeError: 'list' object has no attribute 'strip'
- Used helper in CN generation code (lines 761-764)
- DirectoryService module no longer exists
- Commented out position/department sync code that used DirectoryService
- Marked with TODO for future refactoring
- Fixes ModuleNotFoundError during LDAP sync operations
- Replaced ORM .save() with direct ldap3.Connection.modify()
- ORM save() tries to rebuild DN which fails without proper cn attribute
- Direct modify only changes the employeeNumber attribute
- Fixes Exception: Could not build Distinguished Name
- ldapdb calls timezone.utc.normalize() for datetime field conversion
- Added normalize() method that works the same as localize() for UTC
- Fixes AttributeError: '_UtcCompat' object has no attribute 'normalize'
- Check if Employee with same email exists before creating new one
- Update existing Employee and link it to LDAP instead of creating duplicate
- Prevents UNIQUE constraint failed error on email field
- Sets is_ldap_managed=True and updates user data from LDAP
- Added _resolve_ldap_users() helper to fetch LDAP objects by DN individually
- ldapdb does not support dn__in lookup used by Django admin queryset
- All 3 actions now use _resolve_ldap_users instead of iterating queryset directly
- Made _UtcCompat inherit from datetime.tzinfo instead of plain class
- Implemented required tzinfo methods: utcoffset, tzname, dst
- Fixes TypeError: tzinfo argument must be None or of a tzinfo subclass
- Resolves admin panel crashes when viewing LDAP users with timestamps
- Removed objectGUID field from LdapUser, LdapGroup, and LdapOrganizationalUnit models
- objectGUID contains binary data that causes UnicodeDecodeError when decoded as UTF-8
- Field is not critical for core functionality
- Updated admin.py to not use object_guid attribute
- Удалён LDAP_SERVER_TYPE и все OpenLDAP ветки кода
- user_service.py: только AD (sAMAccountName, UAC, microsoft.modify_password)
- auth_backends.py: AD фильтры и атрибуты (objectCategory, objectGUID)
- Удалён server_configs.py (больше не нужен)
- Удалён OpenLDAP Docker окружение (docker-compose.ldap.yml, ldap/, QUICKSTART_LDAP.md)
- Фронтенд: мульти-шаговая регистрация, AvatarCropper 3:4, verify-email страница
- Убран CSRF для JWT API, IP-ограничения работают независимо
- Полевые ошибки регистрации с красной подсветкой на фронте
ПРОБЛЕМА:
- RegisterAPIView знал о режиме LDAP (вызывал _is_ldap_enabled())
- if ldap_enabled: create_ldap_user() else: create_db_user()
- View имел LDAP-специфичную логику выбора режима
- Нарушение принципа инкапсуляции
РЕШЕНИЕ:
Создан единый метод в LdapUserCreationMixin:
create_user(first_name, last_name, email, phone, password, ...)
- Автоматически выбирает LDAP или БД режим
- Инкапсулирует вызов _is_ldap_enabled()
- View больше не знает о существовании LDAP
ИЗМЕНЕНИЯ:
mixins.py:
+ create_user() - фасад, выбирает режим автоматически
• create_ldap_user() - остался для внутреннего использования
• create_db_user() - остался для внутреннего использования
auth.py:
- Удалён import _is_ldap_enabled
- Удалён блок if ldap_enabled: ... else: ...
- Заменён на: emp, error = self.create_user(...)
РЕЗУЛЬТАТ:
- View НЕ знает про LDAP вообще
- Вся LDAP логика инкапсулирована в миксине
- Чистое разделение ответственности
- Упрощение: -18 строк в view
БЫЛО в RegisterAPIView:
ldap_enabled = _is_ldap_enabled()
if ldap_enabled:
emp, err = self.create_ldap_user(...)
else:
emp = self.create_db_user(...)
СТАЛО:
emp, err = self.create_user(...)
Django check: успешно
ПРОБЛЕМА:
- RegisterAPIView содержал ~30 строк LDAP-специфичной логики
- DirectoryService, DirectoryUserDTO, обработка исключений - всё в основном методе
- Нарушение принципа единственной ответственности
- Сложность тестирования и переиспользования
РЕШЕНИЕ:
Создан api/v1/employees/views/mixins.py с миксинами:
1. LdapUserCreationMixin:
- create_ldap_user() - создание пользователя в LDAP через DirectoryService
- create_db_user() - создание пользователя напрямую в БД (fallback без LDAP)
- Централизованная обработка DirectoryLdapError/DirectoryDbError
- Возвращает (Employee, None) при успехе или (None, Response) при ошибке
2. LdapPasswordMixin:
- update_ldap_password() - обновление пароля в LDAP
- Подготовка для использования в PasswordChangeView и других endpoints
ИЗМЕНЕНИЯ:
- auth.py: RegisterAPIView использует LdapUserCreationMixin
- Блок if ldap_enabled заменён на вызовы self.create_ldap_user() / self.create_db_user()
- _helpers.py: удалён _is_ldap_enabled (перемещён в mixins.py)
- Уменьшено количество строк в RegisterAPIView: ~190 → ~170
ПРЕИМУЩЕСТВА:
- Чёткое разделение: общая логика регистрации vs LDAP-специфика
- Переиспользование: миксины можно применить в других views
- Тестируемость: можно мокать методы миксина
- Читаемость: основной метод фокусируется на бизнес-логике
Django check: успешно
ПРОБЛЕМЫ СТАРОЙ СТРУКТУРЫ:
- 5 файлов сигналов в корне employees/ (захламление)
- Непоследовательность: signals_ldap.py vs signals_department.py
- apps.py импортирует каждый файл явно (5 строк)
- Нет группировки по назначению (LDAP разбросан)
НОВАЯ СТРУКТУРА:
employees/
signals/
__init__.py # единая точка входа
common.py # общие сигналы (было signals.py)
birthday.py # дни рождений
ldap/
__init__.py
employee.py # Employee ↔ LDAP
department.py # Department ↔ LDAP OU
group.py # Group ↔ LDAP Group
ПРЕИМУЩЕСТВА:
- Чёткая иерархия: общие сигналы и LDAP-специфичные
- Единый импорт в apps.py: import employees.signals
- Понятные имена модулей (employee.py вместо signals_ldap.py)
- Масштабируемость: легко добавить signals/email/, signals/webhooks/ и т.д.
ИЗМЕНЕНИЯ:
- Создана структура signals/ и signals/ldap/
- Перемещены и переименованы 5 файлов сигналов
- Обновлены docstrings с новыми путями
- Исправлены относительные импорты в common.py
- apps.py: упрощён с 5 импортов до 1
- Django check: успешно
Git корректно распознал переименования (R), история сохранена.
ПРОБЛЕМА:
- cn/ou были установлены как primary_key в LdapUser/LdapGroup/LdapOrganizationalUnit
- CN/OU не уникальны глобально (могут быть в разных OU)
- .objects.get(cn=...) возвращал случайную группу при дубликатах
- Нарушался дизайн django-ldapdb, где dn - базовый primary_key
РЕШЕНИЕ:
- Убран primary_key=True из cn/ou полей
- Используется dn (Distinguished Name) как primary_key из базового класса
- DN - уникальный идентификатор объекта в LDAP, требуется для всех операций
- Удалено избыточное поле distinguished_name
ИЗМЕНЕНИЯ:
- orm_models.py: cn/ou теперь обычные CharField, dn наследуется из ldapdb.models.Model
- group_utils_orm.py: .get(cn=...) → .filter(cn=...).first() с предупреждением
- group_service.py: find_dn() и find_dn_wrapped() используют .filter().first()
ОБОСНОВАНИЕ ОТКАЗА ОТ objectGUID:
- objectGUID неизменяем и уникален, НО:
- LDAP протокол требует DN для всех операций (add/modify/delete/rename)
- django-ldapdb жёстко завязан на self.dn для save()/delete()
- GUID можно использовать только в search filter, результат всё равно возвращает DN
- DN - естественный primary key для LDAP объектов
Добавлена подробная документация о:
- Ограничениях django-ldapdb (ModifyDN, динамический DN, collision handling)
- Архитектуре использования ORM vs ldap3 для разных операций
- Структуре DN для пользователей/групп/OU в Active Directory
- Роли base_dn в моделях (только для SUBTREE search, не для create/move)
Улучшены docstring для:
- LdapUser: объяснено где реально хранятся пользователи (по отделам, Dismissed)
- LdapGroup: объяснена структура групп (глобальные, роли отделов)
- LdapOrganizationalUnit: структура OU отделов
- get_users_base() и get_base_dn(): приоритеты и покрытие DN
Это решает вопрос о том, как модель работает с пользователями в разных OU
при переводах между отделами и увольнениях.
Создан отчет LDAP_MODULE_AUDIT.md:
- Анализ используемого и неиспользуемого кода
- Список методов DirectoryService и их статус
- Определены кандидаты на удаление (~500+ строк)
- Рекомендации по дальнейшей оптимизации
Ключевые находки:
✅ Используется: DirectoryService (12 методов), Services (User/Department/Group), ORM models
⚠️ НЕ используется: Position/Role сервисы, дублирующие методы групп, helper методы
Потенциал оптимизации: ~500-700 строк кода можно удалить
Критические исправления:
- actions.py: заменить bulk update() на save() для EmployeeDepartment
Причина: bulk update НЕ вызывает Django signals!
Было: .update(is_active=False) - сигнал не срабатывает
Стало: for loop с save() - сигнал срабатывает для каждой записи
- _helpers.py: удалить неиспользуемую функцию _ldap_try()
Нигде не используется после рефакторинга
- _helpers.py: удалить неиспользуемые импорты
DirectoryLdapError, DirectoryServiceError, DirectoryDbError, status
- actions.py: обновить комментарий с правильным названием сигнала
sync_department_member_to_ldap вместо устаревшего sync_member
Функционал сохранен: при увольнении сотрудника теперь корректно
срабатывают сигналы для каждого EmployeeDepartment, что приводит
к корректной синхронизации с LDAP (перемещение из OU отделов).
Упрощены ViewSets:
- PositionViewSet: 146 → 137 строк (-9 строк, -6%)
- DepartmentRoleViewSet: 362 → 248 строк (-114 строк, -31%)
Изменения PositionViewSet:
- Удалены вызовы DirectoryService().reconcile_position()
- Удалены вызовы DirectoryService().delete_position_group()
- Удалены импорты: DirectoryService, _is_ldap_enabled, _ldap_try
- Actions (set_groups, add_groups, remove_groups): только работа с БД
- perform_create/update/destroy: базовые методы без LDAP
Изменения DepartmentRoleViewSet:
- create(): только создание в БД, удалены LDAP блоки
- update(): упрощен до базового метода
- destroy(): только удаление из БД
- assign(): только update_or_create RoleAssignment
- revoke(): только update is_active=False
- Удалены все if ldap_enabled блоки
- Удалены импорты: DirectoryService, DirectoryLdapError, DirectoryDbError, _is_ldap_enabled
- Удалена обработка ошибок LDAP (try/except блоки)
Итого: -123 строки, все ViewSets теперь работают только с БД
Примечание: Полноценная LDAP синхронизация для Position и DepartmentRole
требует реализации отдельных сигналов (аналогично signals_group.py).
Это может быть следующим этапом рефакторинга.
Изменения:
- EmployeeActionViewSet: 240 → 115 строк (-125 строк, -52%)
- _apply_effects(): удалена вся LDAP логика (~140 строк)
Удалено:
- Прямые вызовы DirectoryService
- Обработка LdapSyncState
- Логика перемещения в OU=Dismissed и обратно
- Удаление из департаментов в LDAP
- Все try/except блоки для LDAP ошибок
- Импорты: DirectoryService, DirectoryLdapError, DirectoryServiceError, DirectoryDbError, LdapSyncState, settings, _is_ldap_enabled
Осталось только работа с БД:
- При ACTION_DISMISSED: is_active=False + деактивация EmployeeDepartment
- При других действиях: is_active=True
LDAP синхронизация через существующие сигналы:
- sync_employee_to_ldap_on_save: обрабатывает is_active (активация/деактивация)
- sync_member (signals_department.py): обрабатывает EmployeeDepartment (удаление из отделов)
Функционал сохранен, код стал в 2 раза короче
Изменения:
- VerifyEmailAPIView: 331 → 271 строк (-60 строк)
- Удалена вся логика if ldap_enabled с DirectoryService
- Удалены проверки LdapSyncState и обработка LDAP ошибок
- Активация пользователя теперь через сигнал sync_employee_to_ldap_on_save
- Используется _ldap_changes для передачи данных в сигнал
- Удален импорт DirectoryServiceError
Логика:
- ViewSet устанавливает is_active = True и _ldap_changes
- Сигнал автоматически синхронизирует активацию в LDAP
- Обработка ldap_enabled и ошибок происходит в сигнале
- Код стал в 3 раза короче и понятнее
- Удален метод ui_context (42 строки)
- Все его данные доступны через отдельные endpoints:
- dept → GET /departments/{id}/
- roles → можно добавить /departments/{id}/roles/
- links → GET /departments/{id}/members/
- user_perms → GET /departments/{id}/user-perms/
- choices → UI логика (фронтенд)
Преимущества:
- Кешируемые отдельные endpoints
- Фронт запрашивает только нужное
- Проще тестировать
- Более RESTful
-42 строки (480 → 438)
- Удалены все @transaction.atomic из actions (set_head, set_member_role, add_member, remove_member)
- Удален импорт transaction
- Одиночные операции БД уже атомарны по умолчанию
- Django request уже в транзакции если настроен ATOMIC_REQUESTS
-5 строк (485 → 480)
Методы дублировали стандартную функциональность ModelViewSet:
- create(): serializer.save() делает то же самое
- destroy(): стандартный метод идентичен
-25 строк (510 → 485)
- Создан employees/signals_department.py
- post_save(Department): создание/обновление OU в LDAP
- При created: создает OU, устанавливает description, managedBy
- При update: синхронизирует name, description, head
- post_delete(Department): удаление OU из LDAP
- post_save(EmployeeDepartment): синхронизация членства
- При is_active=True: перемещает пользователя в OU отдела + роль
- При is_active=False: перемещает обратно в Users OU
Подключено в employees/apps.py
- Удалены импорты DirectoryService, DirectoryDepartmentDTO, LDAP errors
- Удален импорт _is_ldap_enabled
- Убраны все блоки if ldap_enabled: ... else: ...
- ViewSet теперь только работает с БД (сигналы синхронизируют LDAP)
Методы упрощены до чистой БД логики:
- partial_update: просто обновляет поля
- set_head: просто устанавливает head + head_appointed_at
- set_member_role: обновляет link.role и RoleAssignment
- add_member: создает/активирует EmployeeDepartment
- remove_member: деактивирует EmployeeDepartment
- create: просто создает Department
- destroy: просто удаляет Department
DepartmentViewSet теперь чистый CRUD: -189 строк (699 → 510)
- Убрана логика создания LDAP пользователей из сигналов
- Сигналы ТОЛЬКО синхронизируют изменения существующих LDAP пользователей
- Создание LDAP пользователей - ответственность RegisterAPIView
- Убран _ldap_password из perform_create (создание через API не создает LDAP пользователей)
- Создан LdapPasswordMixin для endpoint'ов работы с паролями
- perform_create теперь только создает Employee в БД (административное создание)
Архитектурное улучшение: разделение ответственности
- Auth views (RegisterAPIView) - создают LDAP пользователей напрямую
- Сигналы - только синхронизация изменений
- LdapPasswordMixin - для endpoint'ов смены пароля
- Удалён ExternalSystemSyncMixin из EmployeeViewSet
- ViewSet теперь использует временные атрибуты (_ldap_password, _ldap_avatar, _ldap_changes)
- Создан employees/signals_ldap.py с post_save/post_delete handlers
- Сигналы автоматически синхронизируют Employee с LDAP через DirectoryService
- Убран дублирующий блок required_perms_by_action из EmployeeViewSet
- Упрощены perform_create и perform_update (-170 строк)
Архитектурное улучшение: разделение ответственности ViewSet (HTTP) и signals (синхронизация)
- sync_service: заменены read_attrs + modify_user_attrs на LdapUser ORM
для записи employeeNumber при импорте пользователей
- Убран неиспользуемый импорт modify_user_attrs
