lokses
c42b1b533e
Статический аудит: архитектура, приложения/модели, сильные стороны, покрытие тестами, безопасность, конфигурация, техдолг по приоритетам, план действий. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
169 lines
18 KiB
Markdown
169 lines
18 KiB
Markdown
# Аудит проекта ACORE
|
||
|
||
**Система управления аптечной сетью** · корпоративный портал + внутренние интеграции
|
||
Дата аудита: **2026-07-22** · Ветка: `main` · Django 6.0.3 / Python 3.14 / PostgreSQL
|
||
|
||
---
|
||
|
||
## 1. Резюме
|
||
|
||
ACORE — внутренний портал аптечной сети: управление сотрудниками, аптеками, справочниками, ИТ-инвентарём, задачами, лентой публикаций, плюс живые интеграции с Active Directory и Mattermost. Проект в **хорошей инженерной форме**: архитектура последовательная, паттерны выдержаны, безопасность продумана на уровне выше среднего для внутренних систем. Растёт быстро — 13 приложений, ~14,4 тыс. строк Python (без миграций и тестов).
|
||
|
||
**Общая оценка: 7.5/10.** Фундамент добротный. Главные риски не архитектурные, а процессные: неравномерное покрытие тестами, большой объём незакоммиченной работы и хранение секретов интеграций открытым текстом.
|
||
|
||
| Показатель | Значение |
|
||
|---|---|
|
||
| Приложений | 13 |
|
||
| Моделей | 39 |
|
||
| Python-код (без миграций/тестов) | ~14 400 строк |
|
||
| Миграций | 56 |
|
||
| Тестов | 127 функций (~2000 строк) |
|
||
| Шаблонов | 39 HTML |
|
||
| JS-модулей (кастомных) | 12 |
|
||
| `manage.py check` | чисто |
|
||
| `check --deploy` (production) | 2 минорных предупреждения |
|
||
| Миграции консистентны | да (`No changes detected`) |
|
||
|
||
---
|
||
|
||
## 2. Архитектура
|
||
|
||
### Технологический стек
|
||
- **Backend:** Django 6.0.3, PostgreSQL (psycopg2), Django REST Framework (точечно, 5 endpoint-ов).
|
||
- **Async:** Celery 5.5 + Redis + django-celery-beat/results.
|
||
- **Frontend:** серверный рендеринг + **HTMX** (навигация без перезагрузки), **Tailwind CSS v4** (сборка через CLI), ванильный JS для виджетов. SPA-фреймворков нет — сознательно.
|
||
- **Админка:** django-unfold (кастомная тема) + django-import-export.
|
||
- **Интеграции:** ldap3 (Active Directory), mattermostautodriver + httpx (Mattermost).
|
||
- **Контент:** django-tinymce (WYSIWYG) + nh3 (санитизация HTML против XSS).
|
||
- **Справочники:** django-fast-treenode (иерархические деревья).
|
||
|
||
### Ключевые архитектурные решения
|
||
1. **Единый справочный слой** (`core.Directory` / `DirectoryType`) — иерархические справочники (организации, отделы, должности, регионы, типы/бренды оборудования, статусы). На него опирается почти всё. Реализовано аккуратно, с автоперестройкой дерева после миграций и импорта.
|
||
2. **Интеграции по единому паттерну:** сервис-клиент → sync-сервис → сигналы → Celery-задачи → management-команды. Благодаря этому AD лёг рядом с Mattermost без изобретения велосипеда.
|
||
3. **Единый шелл фронта** (`templates/index.html`) + страницы-фрагменты HTMX. Общие утилиты вынесены (`core/utils/htmx.py`, `permissions.py`, `templatetags/common_tags.py`).
|
||
4. **Роли через `auth.Group`**, а не поле-enum (миграция `27a917e`).
|
||
|
||
---
|
||
|
||
## 3. Приложения и модели
|
||
|
||
| Приложение | Назначение | Модели | Тесты |
|
||
|---|---|---|---|
|
||
| `core` | Справочники, конфиги коннекторов, портал-вью | Directory, DirectoryType, ConnectorConfig | 21 (смоук страниц, PII-доступ, дерево) |
|
||
| `employees` | Сотрудники (кастомный User), права | Employee, EmployeePermissionOverride, PermissionGroup | **0** |
|
||
| `locations` | Аптеки, склады, офисы (MTI) | Location, Pharmacy, PharmacyPhone, PharmacyHistory | **0** |
|
||
| `it_manager` | ИТ-инвентарь, заявки, интернет-договоры | Equipment, **EquipmentModel** (каталог), EquipmentAssignment, Consumable, Ticket, TicketComment, WorkLog, ServiceCall, InternetProvider, InternetContract | 10 |
|
||
| `feed` | Лента и публикации (TinyMCE) | Post, PostComment | 40 |
|
||
| `tasks` | Задачи (роли Битрикс-стиля) | Task, TaskComment | **0** ⚠ |
|
||
| `ad_connector` | Active Directory (провижининг) | ADSyncLog | 30 |
|
||
| `mattermost_connector` | Mattermost (аккаунты, каналы) | MattermostUser/Team/Channel + membership + sync-settings (7) | 19 |
|
||
| `visit_manager` | Учёт посещений, графики, смены | AttendanceLog, WorkShift, Shift, Holiday, Schedule* (9) | **0** |
|
||
| `hiring` | Заявки на найм | HiringRequest | 7 |
|
||
| `utilities` | Показания счётчиков | UtilityReading | 0 |
|
||
| `mail_connector`, `mailparser`, `portal_connector` | Заглушки / служебное | — | 0 |
|
||
|
||
### Заметные модели
|
||
- **`Employee`** — кастомный `AbstractBaseUser`, богатый: контакты, PII, локация (аптека/офис/склад), руководитель, интеграционные поля AD/Mattermost, гибкие права через `EmployeePermissionOverride` (grant/revoke с датами действия). Валидация в `clean()` разделяет аптечных/офисных сотрудников.
|
||
- **`EquipmentModel` + `Equipment`** (свежий рефакторинг) — каталог моделей отделён от экземпляров: тип+бренд+модель заводятся один раз, экземпляры несут только серийник/локацию. Массовый ввод и массовая выдача.
|
||
- **`Task`** — три роли (ответственный/соисполнители/наблюдатели), видимость через `for_user()`, наблюдатели не меняют статус.
|
||
|
||
---
|
||
|
||
## 4. Сильные стороны
|
||
|
||
✅ **Безопасность продумана.** Отдельный коммит hardening (`faa06b9`): исправлен open redirect в логине, добавлена аутентификация на API, MIME-валидация загрузок (magic bytes + Pillow verify), HttpOnly/SameSite куки, лимиты загрузки. Есть полноценный `production.py` (DEBUG=False, SSL-redirect, HSTS, secure cookies) — `check --deploy` даёт лишь 2 минорных предупреждения.
|
||
|
||
✅ **Разделение PII.** Личные данные (телефоны, ИНН, дата рождения, экстренный контакт) отделены от публичных полей — и в DRF-сериализаторах (`EmployeeSerializer` / `EmployeeDetailSerializer`), и на детальной странице сотрудника (`_can_view_employee_pii`: staff / сам / непосредственный руководитель). Покрыто тестами.
|
||
|
||
✅ **XSS-защита ленты.** Тело поста (HTML из TinyMCE) всегда прогоняется через `nh3` в `Post.save()` — чистка в модели, а не во view, поэтому защищён любой путь записи. Загрузка картинок отдельным endpoint-ом с проверкой содержимого; `data:`-URI вырезаются.
|
||
|
||
✅ **Оптимизация запросов.** Осознанная борьба с N+1: `Prefetch` с `select_related`, агрегаты вместо циклов `.count()`, `distinct=True` в `Count` при OR-джойнах по M2M.
|
||
|
||
✅ **Устойчивость к сбоям.** Сигналы обёрнуты в try/except — недоступный Celery-брокер или AD не роняют сохранение сотрудника. Reverse-миграции пишутся (где возможно).
|
||
|
||
✅ **Единый UX-язык.** HTMX + Tailwind + переиспользуемые виджеты (people-picker дерево, searchable-select, пагинация). Украинский интерфейс выдержан.
|
||
|
||
---
|
||
|
||
## 5. Тестирование
|
||
|
||
**127 тестовых функций, ~2000 строк.** Покрытие качественное там, где оно есть, но **неравномерное**:
|
||
|
||
**Покрыто хорошо:**
|
||
- `feed` (40) — публикация, права, санитизация, картинки, комментарии.
|
||
- `ad_connector` (30) — LDAP-клиент на мок-каталоге ldap3 (`MOCK_SYNC`), sync-сценарии, сигналы, страница сопоставления.
|
||
- `core` (21) — смоук всех страниц портала, PII-доступ, дерево справочников.
|
||
- `mattermost_connector` (19), `it_manager` (10), `hiring` (7).
|
||
|
||
**Не покрыто вообще (0 тестов):**
|
||
- ⚠ **`tasks`** — модуль с нетривиальной логикой видимости и ролей, но без тестов (задача осталась в бэклоге).
|
||
- ⚠ **`employees`** — кастомная модель User с валидацией `clean()`, правами-оверрайдами; критичный код без тестов.
|
||
- ⚠ **`visit_manager`** — 9 моделей (графики, смены, посещения) без единого теста.
|
||
- `locations`, `utilities`, mail/portal-заглушки.
|
||
|
||
**Инфраструктурный нюанс:** discovery `manage.py test apps.X` падает на структуре пакетов — тесты запускают явными метками модулей (`apps.feed.tests`). CI отсутствует — тесты гоняются вручную.
|
||
|
||
---
|
||
|
||
## 6. Безопасность — находки
|
||
|
||
| Приоритет | Находка | Детали |
|
||
|---|---|---|
|
||
| 🔴 Высокий | **Секреты интеграций открытым текстом** | `ConnectorConfig.password` / `token` — обычные `CharField` в БД. Пароль сервисной учётки AD и токен Mattermost лежат в базе как есть. Рекомендация: шифрование на уровне поля (`django-fernet-fields` или аналог) либо вынос в секрет-менеджер/переменные окружения. |
|
||
| 🟡 Средний | **Нет тестов на критичный код доступа** | `tasks.for_user()`, `employees` права, PII-гейтинг частично — регрессии в контроле доступа не отлавливаются автоматически. |
|
||
| 🟢 Низкий | `SECURE_HSTS_PRELOAD` не выставлен | `check --deploy` W021 — не даёт подать сайт в preload-список. Косметика. |
|
||
| 🟢 Низкий | Синхронные операции AD с паролем блокируют запрос | Сохранение формы ждёт LDAP-вызов (до 15 с таймаут при недоступном контроллере). Ошибка не роняет сохранение, но UX-задержка возможна. |
|
||
|
||
**Хорошо:** `.env` и `db.sqlite3` **не** в git; `.gitignore` покрывает env/sqlite/pycache/node_modules/шаблон-референс; SECRET_KEY/ALLOWED_HOSTS/DATABASE_URL — из окружения.
|
||
|
||
---
|
||
|
||
## 7. Конфигурация и деплой
|
||
|
||
- **Настройки разделены** на `base` / `local` / `production`. Production корректен.
|
||
- **БД:** PostgreSQL и в dev, и в prod (через `DATABASE_URL`). Лежащий в корне `db.sqlite3` — мёртвый артефакт, стоит удалить.
|
||
- **Celery:** приложение сконфигурировано (`acore/celery.py`), брокер из env, локально `CELERY_TASK_ALWAYS_EAGER=True` (без брокера задачи синхронно). Beat-задачи есть в mattermost/ad коннекторах.
|
||
- **Локализация:** `LANGUAGE_CODE='ru'`, `TIME_ZONE='Europe/Kiev'`, UI на украинском, `verbose_name` на русском, комментарии — смесь ru/uk. Работает, но единого языка нет.
|
||
- **Нет:** README, `docs/`, CI/CD (`.github`/`.gitlab-ci`), Makefile, зафиксированного процесса деплоя.
|
||
|
||
---
|
||
|
||
## 8. Технический долг — сводка по приоритетам
|
||
|
||
### 🔴 Высокий
|
||
1. **Незакоммиченная работа.** В рабочем дереве **116 изменённых/новых файлов** — целые модули (feed, tasks, employee-detail, equipment-каталог, виджеты) не в истории git. Последний коммит — `c3f56f8`. Риск потери работы; история не отражает реальное состояние. **Закоммитить логическими блоками.**
|
||
2. **Секреты открытым текстом** (см. §6).
|
||
|
||
### 🟡 Средний
|
||
3. **Тесты для `tasks`, `employees`, `visit_manager`** — критичная бизнес-логика без покрытия.
|
||
4. **Нет CI** — тесты не гоняются автоматически на push; регрессии всплывают вручную (в этой сессии дважды находились сломанные смоук-URL).
|
||
5. **Рост числа сигналов на `Employee.post_save`** (9 ресиверов между AD и Mattermost, у каждого свой pre_save-кеш). При третьей интеграции (Carbonio в планах) станет хрупким — напрашивается единый диспетчер изменений.
|
||
6. **Reverse-миграции частично ломаются** на восстановлении non-null полей (напр. `0018_equipment_model_catalog`) — forward безупречен, но откат в dev упадёт. Приемлемо, но стоит знать.
|
||
|
||
### 🟢 Низкий
|
||
7. **Дубль зависимости:** `import-export==0.3.1` (устаревший неофициальный) соседствует с `django-import-export==4.4.0`. Проверить и убрать лишнее.
|
||
8. **Единый язык** кода/комментариев/verbose_name.
|
||
9. **Мёртвые артефакты:** `db.sqlite3` в корне, `.env2`.
|
||
10. **Портал наполовину макет:** email, chat, документы, календарь, часть HR — статичные страницы без бэкенда. Стоит явно решить, что реализуем, а что убрать из меню.
|
||
|
||
---
|
||
|
||
## 9. Рекомендации (порядок действий)
|
||
|
||
1. **Закоммитить накопленное** логическими блоками — снять риск №1 немедленно.
|
||
2. **Зашифровать секреты `ConnectorConfig`** перед подключением живого AD в прод — именно эта логика трогает внешнюю систему, где ошибка видна всей компании.
|
||
3. **Покрыть тестами `tasks` и `employees`** — минимум контроль доступа и валидацию. Мок-сценарии для AD/Mattermost уже почти готовые тесты.
|
||
4. **Поднять минимальный CI** (GitHub Actions / GitLab CI): `manage.py check` + прогон тестов + `makemigrations --check`. Дёшево, ловит регрессии автоматически.
|
||
5. **Написать README** с процессом запуска (dev-сервер, Tailwind-сборка, Celery, тесты) и деплоя.
|
||
6. **Отрефакторить сигналы `Employee`** в единый диспетчер до третьей интеграции.
|
||
7. **Причесать:** удалить `db.sqlite3`/`.env2`, разобраться с дублем `import-export`, определиться с макет-страницами.
|
||
|
||
---
|
||
|
||
## 10. Вывод
|
||
|
||
Фундамент выбран правильно и выдержан — проект масштабируется **добавлением модулей, а не переписыванием**. Паттерны интеграций, справочный слой, безопасность, борьба с N+1 — всё на уровне выше среднего для внутренней системы. Основной долг — **процессный**, а не архитектурный: закоммитить работу, зашифровать секреты, дописать тесты на критичные модули и поднять CI. Закрыв эти пункты, проект будет готов к спокойной эксплуатации и дальнейшему росту.
|
||
|
||
---
|
||
*Аудит подготовлен на основе статического анализа кода, конфигурации, миграций, тестов и git-истории. Данные собраны инструментально (`manage.py check --deploy`, `makemigrations --check`, подсчёт моделей/тестов/строк).*
|