Files
magistr/BUG_REPORT.md
2026-07-04 15:47:12 +03:00

18 KiB
Raw Blame History

Отчёт по ошибкам и рискам проекта Magistr

Дата проверки: 2026-07-03

Проверено вручную: backend Spring Boot, мультитенантность, авторизация, генерация расписания, основные frontend JS-файлы. Автотесты не запущены: в окружении нет mvn и ./mvnw.

Нужно исправить в первую очередь

1. Высокая — JWT имеет небезопасные production-дефолты

Файлы:

  • backend/src/main/java/com/magistr/app/config/auth/JwtProperties.java:13-17
  • backend/src/main/resources/application.properties:18-22
  • backend/src/main/java/com/magistr/app/controller/AuthController.java:162-168

Проблема: если на проде не задан JWT_SECRET, приложение использует известный дефолтный секрет dev-only-change-this-jwt-secret-32-bytes-minimum. При этом JWT_REFRESH_COOKIE_SECURE по умолчанию false, и refresh-cookie может уходить без флага Secure.

Риск: подделка access-токенов при известном секрете; утечка refresh-cookie при ошибочной HTTP/прокси-конфигурации.

Как исправить:

  • На production-профиле падать при отсутствии JWT_SECRET и при секрете короче требуемой длины.
  • Для production сделать app.jwt.refresh-cookie-secure=true обязательным.
  • Разделить dev/prod профили: дефолтный секрет оставить только в application-dev.properties.

2. Высокая — race condition при ротации refresh-токена

Файл: backend/src/main/java/com/magistr/app/config/auth/RefreshTokenService.java:50-78

Проблема: rotate() читает refresh-токен через findByTokenHash(), проверяет активность, затем отзывает старый и создаёт новый. Нет блокировки строки или атомарного UPDATE ... WHERE revoked_at IS NULL. Два параллельных запроса с одним refresh-токеном могут оба пройти проверку и создать две активные сессии.

Риск: повторное использование одного refresh-токена, раздвоение сессий, некорректная ротация rotatedToTokenHash.

Как исправить:

  • Добавить pessimistic lock в репозиторий (@Lock(PESSIMISTIC_WRITE) для поиска по tokenHash) внутри транзакции.
  • Либо сделать атомарное обновление: UPDATE auth_refresh_tokens SET revoked_at = now, rotated_to_token_hash = :newHash WHERE token_hash = :hash AND revoked_at IS NULL AND expires_at > now и создавать новый токен только если обновлена 1 строка.
  • Добавить тест на два параллельных refresh-запроса.

3. Высокая — AuthContext может протечь между запросами при 403

Файл: backend/src/main/java/com/magistr/app/config/auth/AuthorizationInterceptor.java:35-49

Проблема: пользователь кладётся в AuthContext на строке 45, а при нехватке роли метод возвращает false на строке 49. В таком сценарии afterCompletion() текущего interceptor'а может не выполниться, и ThreadLocal останется в потоке до следующей очистки.

Риск: утечка контекста пользователя между запросами в servlet thread pool. Сейчас это в основном риск безопасности и будущих багов, но его лучше устранить сразу.

Как исправить: перед каждым return false после AuthContext.setCurrentUser(user) вызывать AuthContext.clear(). Ещё лучше — не устанавливать AuthContext, пока не пройдена ролевая проверка, или обернуть обработку отказов в безопасный helper.


4. Высокая — точечные изменения расписания не проверяются на конфликты

Файл: backend/src/main/java/com/magistr/app/controller/ScheduleOverrideController.java:71-127

Проблема: при MOVE/REPLACE валидируется только существование нового преподавателя, аудитории и слота. Не проверяется, свободны ли преподаватель/аудитория/группа в выбранную дату и пару.

Риск: учебный отдел может создать override, который назначит преподавателя или аудиторию на две пары одновременно. Основные правила расписания конфликты проверяют, а override — нет.

Как исправить:

  • Перед сохранением override построить расписание на lessonDate и проверить занятость нового timeSlot, teacher, classroom, групп/подгрупп базового слота.
  • Переиспользовать/вынести конфликтную логику из ScheduleRuleAdminController в сервис.
  • Вернуть 409 Conflict с описанием конфликтующего занятия.

5. Средняя/высокая — расписание преподавателя не показывает пары, где он назначен через override

Файлы:

  • backend/src/main/java/com/magistr/app/service/ScheduleQueryService.java:47-58
  • backend/src/main/java/com/magistr/app/repository/ScheduleRuleRepository.java:36-54

Проблема: если поиск выполняется только по teacherId, сервис строит расписание через buildScheduleForTeacher(teacherId), то есть берёт только базовые правила, где преподаватель указан в слоте. Если override заменил преподавателя на этого teacherId, базовое правило в выборку не попадёт, и после applyOverrides() такая пара не появится.

Риск: преподаватель не увидит замену/переназначенную ему пару в своём расписании.

Как исправить:

  • При поиске по teacherId дополнительно учитывать overrides с newTeacher.id = teacherId за период.
  • Либо строить расписание по группам для затронутых override-слотов и после применения override фильтровать по преподавателю.
  • Добавить тест: базовый преподаватель A, override newTeacher=B, поиск по teacherId=B должен вернуть пару.

6. Средняя — обновление существующего тенанта может удалить рабочее подключение и сохранить нерабочее

Файлы:

  • backend/src/main/java/com/magistr/app/controller/DatabaseController.java:104-116
  • backend/src/main/java/com/magistr/app/config/tenant/TenantRoutingDataSource.java:135-148
  • backend/src/main/java/com/magistr/app/config/tenant/TenantConfigWatcher.java:147-158

Проблема: при добавлении тенанта с уже существующим доменом старый DataSource сначала удаляется и закрывается (removeTenant()), затем добавляется новый. Hikari настроен с setInitializationFailTimeout(-1), поэтому невалидная БД может быть добавлена без ошибки. initDatabaseForTenant() ловит исключения Flyway и не пробрасывает их наружу, поэтому API может вернуть успех и записать нерабочий конфиг.

Риск: админ одной ошибкой в JDBC URL/пароле может выключить рабочий tenant и разнести нерабочую конфигурацию через ConfigMap.

Как исправить:

  • Сначала создать и проверить новый DataSource во временном объекте (testConnection, Flyway migrate) и только после успеха атомарно заменить старый.
  • initDatabaseForTenant() должен возвращать результат или бросать исключение, чтобы DatabaseController не писал невалидный ConfigMap.
  • При ошибке оставлять старое подключение активным.

7. Средняя — watcher не применяет изменения URL/логина/пароля существующего тенанта

Файл: backend/src/main/java/com/magistr/app/config/tenant/TenantConfigWatcher.java:102-125

Проблема: syncTenants() добавляет только новые домены и удаляет исчезнувшие. Если в tenants.json изменились url, username или password для уже существующего domain, текущий DataSource не заменяется.

Риск: после обновления ConfigMap часть pod'ов продолжит ходить в старую БД/со старыми credentials до рестарта.

Как исправить: сравнивать весь TenantConfig для существующих доменов. При отличии — безопасно пересоздавать DataSource после успешной проверки нового подключения.


8. Средняя — отключена проверка TLS-сертификата Kubernetes API

Файл: backend/src/main/java/com/magistr/app/config/tenant/ConfigMapUpdater.java:74-76,104-119

Проблема: createInsecureClient() доверяет любому сертификату (checkServerTrusted пустой). Комментарий объясняет это self-signed CA, но в Kubernetes правильный CA доступен в serviceaccount volume.

Риск: MITM внутри сети кластера может подменить Kubernetes API и получить serviceaccount token/подменить ConfigMap.

Как исправить: использовать CA из /var/run/secrets/kubernetes.io/serviceaccount/ca.crt и стандартную проверку hostname/cert chain. Не использовать trust-all клиент.


9. Средняя — кабинет кафедры может перезаписать дисциплину другой кафедры при импорте

Файл: backend/src/main/java/com/magistr/app/controller/DepartmentWorkspaceController.java:80-90

Проблема: импорт ищет дисциплину только по глобальному имени (findByName) и затем без проверки меняет departmentId на кафедру текущего пользователя. Если дисциплина с таким названием уже принадлежит другой кафедре, она будет переназначена.

Риск: потеря принадлежности дисциплины и связей расписания/календарей у другой кафедры.

Как исправить:

  • Не менять departmentId существующей дисциплины другой кафедры.
  • Сделать уникальность по (department_id, lower(name)), если одинаковые названия допустимы у разных кафедр.
  • При конфликте возвращать понятную ошибку или создавать отдельную запись в рамках кафедры.

10. Средняя — кафедра может смотреть workload по чужим кафедрам

Файл: backend/src/main/java/com/magistr/app/controller/WorkloadController.java:43-130

Проблема: endpoints /api/workload/* доступны роли DEPARTMENT и принимают произвольный departmentId, но не сверяют его с AuthContext.departmentId. Также при departmentId отсутствующем запрос строится по всем группам/кафедрам.

Риск: оператор кафедры может получить загруженность преподавателей/аудиторий/кафедр за другие подразделения, если это не было задумано бизнес-правилами.

Как исправить: для роли DEPARTMENT принудительно использовать AuthContext.getCurrentUser().departmentId() и игнорировать/запрещать чужой departmentId. Аналогично проверить ScheduleSearchController, если расписание не должно быть глобально видимым.


11. Средняя — семестры можно создать вне учебного года или с пересечениями

Файлы:

  • backend/src/main/java/com/magistr/app/controller/AcademicCalendarAdminController.java:95-129,201-211
  • backend/src/main/java/com/magistr/app/service/AcademicDateService.java:32-34
  • backend/src/main/java/com/magistr/app/repository/SemesterRepository.java:13

Проблема: при создании/обновлении семестра проверяется только endDate >= startDate. Нет проверки, что семестр лежит внутри своего учебного года и не пересекается с другим семестром. При этом расписание ищет findFirstByStartDateLessThanEqualAndEndDateGreaterThanEqual(), то есть при пересечении дата будет сопоставляться с произвольным первым семестром.

Риск: генерация расписания и чётность недель будут работать некорректно на пересекающихся датах.

Как исправить:

  • Валидировать границы семестра относительно AcademicYear.startDate/endDate.
  • Запрещать пересечение семестров в рамках учебного года.
  • Желательно добавить DB-level exclusion constraint/range constraint или хотя бы сервисную проверку и тест.

12. Низкая/средняя — access-токен хранится в localStorage

Файл: frontend/admin/js/api.js:5-7,155-163

Проблема: access JWT хранится в localStorage, откуда его может украсть любой XSS в админке. Refresh-токен уже вынесен в HttpOnly cookie, но access-токен остаётся доступным JS.

Риск: при XSS злоумышленник получает Bearer token и может выполнять API-запросы до истечения TTL.

Как исправить:

  • По возможности держать access token в памяти и обновлять через HttpOnly refresh cookie.
  • Усилить CSP (script-src без inline), запретить небезопасные вставки HTML.
  • Провести отдельный XSS-аудит всех innerHTML.

13. Низкая — frontend местами вставляет текст ошибки через innerHTML без экранирования

Файл: frontend/admin/settings/js/views/database.js:37-49

Проблема: e.message вставляется в innerHTML без escapeHtml(). Сейчас большинство серверных ошибок фиксированные, но часть ошибок может включать текст из внешних систем/драйверов.

Риск: потенциальный XSS в настройках БД при попадании HTML в сообщение ошибки.

Как исправить: заменить на textContent или оборачивать escapeHtml(e.message).


14. Низкая — backend image скачивает OpenTelemetry javaagent по latest

Файл: backend/Dockerfile:7

Проблема: сборка всегда скачивает latest javaagent с GitHub без pin версии/checksum.

Риск: невоспроизводимые сборки; внезапные несовместимости; supply-chain риск.

Как исправить: закрепить конкретную версию javaagent и проверять checksum.

Дополнительно проверить после исправлений

  1. Добавить Maven Wrapper (mvnw), чтобы проверки запускались одинаково в CI и локально.
  2. Прогнать mvn test и добавить тесты на:
    • параллельную ротацию refresh-токена;
    • override с конфликтом преподавателя/аудитории;
    • расписание преподавателя, назначенного через override;
    • обновление существующего tenant config;
    • запрет чужого departmentId для роли DEPARTMENT.
  3. Не изменять существующие Flyway-миграции. Если нужны DB constraints/indexes — добавлять новую миграцию V3__...sql.