633bdf3ef0
Some checks are pending
CI / POS (WPF, Windows) (push) Waiting to run
CI / Backend (.NET 8) (push) Successful in 46s
CI / Web (React + Vite) (push) Successful in 41s
Docker API / Build + push API (push) Successful in 1m12s
Docker Web / Build + push Web (push) Successful in 31s
Docker API / Deploy API on stage (push) Successful in 17s
Docker Web / Deploy Web on stage (push) Successful in 12s
Аудит 2026-04-27. Полный отчёт — docs/audit-2026-04-27.md. Что закрыто: — /connect/token (AuthorizationController) теперь отказывает в login если AppUser привязан к удалённой/архивной Organization. SuperAdmin обходит проверку (ему org не нужна). Жалоба: nurnetps@gmail.com мог логиниться после удаления своей org из SuperAdmin консоли. — SuperAdminOrganizationsController.Delete (DELETE org) каскадно деактивирует всех AppUser привязанных к этой org (IsActive=false, OrganizationId=null) и помечает Status='revoked' для всех их OpenIddictTokens. Раньше Org удалялась, а юзеры оставались валидными с активными refresh-tokens на 30 дней. — EmployeesController.Delete теперь soft-delete (IsActive=false, FiredAt). Запрещены: 403 если попытка удалить себя; 403 если попытка удалить Owner (Organization.AccountOwnerUserId == employee.UserId). Сообщения с инструкцией («передайте права», «покинуть через настройки»). — /api/me возвращает hasLiveOrg и hasActiveEmployee — frontend использует это для редиректа на /no-organization вместо белого экрана. — Новая страница /no-organization (NoOrganizationPage) — fallback для orphan AppUser. CTA: создать новую org через публичный /signup или попросить инвайт. Кнопка «выйти». TenantRouteGuard редиректит orphan юзеров туда. — SuperAdminAsOrgBanner: добавлена проверка через useMe — баннер рендерится только если у текущего юзера есть Identity-роль SuperAdmin. Lingering localStorage override от прошлой сессии (другой юзер логинился до этого) автоматически чистится. — auth.ts: clearTokens() теперь сбрасывает superAdminAsOrg и superAdminEditMode. login() вызывает clearTokens() ПЕРЕД запросом чтобы новый юзер не унаследовал override-состояние от предыдущего. — deploy/recovery-restore-orphan-owners.sql — идемпотентный скрипт деактивирующий уже накопленных orphan AppUser (как nurnetps) и revoke их токены. Применён на стейдже: 1 user деактивирован, 9 токенов revoked. — deploy/Dockerfile.api: убран `--no-restore` из publish — два раздельных шага роняли build с NETSDK1064 на свежих analyzer- зависимостях, теперь restore идёт внутри publish. Smoke (стейдж): - nurnetps@gmail.com /connect/token → invalid_grant. - admin@food-market.local /connect/token → access_token выдан. - food-market.zat.kz/, /signup/, app.../login, /health → 200.
135 lines
13 KiB
Markdown
135 lines
13 KiB
Markdown
# Системный аудит — 2026-04-27
|
||
|
||
Полный обход auth, tenant isolation, удаления сущностей, override-режима, локализации, валидации форм. Запущен после прямой жалобы юзера: «удалил себя — могу зайти», «зашёл в SuperAdmin консоль будучи tenant-юзером».
|
||
|
||
## Корневая диагностика nurnetps@gmail.com
|
||
|
||
Состояние БД на момент аудита (см. SQL-скрипты в этом отчёте):
|
||
|
||
```
|
||
users.Id = fbe4255a-c1ad-4355-88c1-ef21dfcd6db2
|
||
users.IsActive = true
|
||
users.OrganizationId = 6237ef17-b720-4076-86d0-0f543023b31a ← удалённая
|
||
users.LockoutEnd = null
|
||
roles = ['Admin'] ← глобальная Identity-роль
|
||
employees = 0 rows
|
||
organizations(id) = 0 rows ← удалена
|
||
OpenIddictTokens = 3 valid refresh + 3 valid access (TTL до 2026-05-27)
|
||
```
|
||
|
||
**Гипотеза А (`/signup` даёт SuperAdmin) — отклонена.** В `AuthSignupController.cs:79` назначается роль `Admin`, не `SuperAdmin`.
|
||
|
||
**Гипотеза Г подтверждена:** при удалении Organization из SuperAdmin консоли:
|
||
1. Связанные `users` НЕ деактивируются и сохраняют `OrganizationId` указывающий на удалённую org (orphan reference, нет FK с CASCADE).
|
||
2. OpenIddict refresh/access tokens НЕ отзываются.
|
||
3. `Employees` либо удаляются (вручную перед DELETE org), либо остаются orphan — в любом случае на `/connect/token` это не влияет.
|
||
|
||
Login повторно проходит, потому что:
|
||
- `users.IsActive=true` (поле есть, но никто не сбрасывает на DELETE org).
|
||
- Пароль валиден.
|
||
- Identity-роль `Admin` глобальная.
|
||
- На бэке нет проверки «AppUser.OrganizationId должен указывать на живую Organization».
|
||
- На фронте после login нет проверки «активный Employee в орге».
|
||
|
||
Override-баннер видит обычный tenant-юзер потому что (см. фикс #6) `SuperAdminLayout` рендерится по факту наличия любых Identity-ролей в JWT, а не строго `SuperAdmin`.
|
||
|
||
## Найденные проблемы
|
||
|
||
### #1 — DELETE Organization не каскадирует на AppUser/Employees/токены
|
||
**Категория:** security / data-integrity
|
||
**Серьёзность:** critical
|
||
**Воспроизведение:** SuperAdmin удаляет архивированную org → AppUser-ы этой org остаются `IsActive=true` с валидными refresh-tokens; могут логиниться; JWT содержит `org_id` указывающий в никуда.
|
||
**Корневая причина:** `SuperAdminOrganizationsController.Delete` (api/Controllers/SuperAdmin) делает `_db.Organizations.Remove(o)` без побочных эффектов; FK от `users.OrganizationId` к `organizations.Id` отсутствует на уровне БД.
|
||
**Фикс:** перед `Remove(org)` — `users.IsActive=false` + `Employees.IsActive=false` + revoke всех refresh-tokens юзеров через `IOpenIddictTokenManager`.
|
||
|
||
### #2 — `/connect/token` не проверяет наличие живой organization
|
||
**Категория:** security / auth
|
||
**Серьёзность:** critical
|
||
**Воспроизведение:** см. nurnetps — login проходит при удалённой org.
|
||
**Фикс:** в кастомизации token endpoint (или сразу после signin) проверять что `User.OrganizationId IS NOT NULL` и существует не-архивная Organization, иначе reject с понятным сообщением «Организация не найдена или удалена. Обратитесь к владельцу».
|
||
|
||
### #3 — `EmployeesController.Delete` — hard-delete без гардов
|
||
**Категория:** security / UX
|
||
**Серьёзность:** high
|
||
**Воспроизведение:** Admin может удалить себя или владельца org через DELETE /api/employees/{id} без сопротивления.
|
||
**Фикс:** проверки `e.UserId == currentUserId` → 403, `e.UserId == org.AccountOwnerUserId` → 403, soft-delete (`IsActive=false`) вместо `Remove`.
|
||
|
||
### #4 — Tenant guard не проверяет активный Employee
|
||
**Категория:** security / multi-tenancy
|
||
**Серьёзность:** high
|
||
**Воспроизведение:** orphan AppUser с `OrganizationId` указывающим на удалённую/несоответствующую org попадает на `/dashboard` и любые tenant-API.
|
||
**Фикс:** middleware/filter после `[Authorize]` — `EXISTS(Employee WHERE UserId=@uid AND OrganizationId=@oid AND IsActive=true)`. SuperAdmin override обходит проверку (ему так и надо). Если нет — 403 + специфический код `NoActiveEmployee`, фронт ловит и редиректит на `/no-organization`.
|
||
|
||
### #5 — Override-баннер показывается не-SuperAdmin
|
||
**Категория:** UX / security perception
|
||
**Серьёзность:** high
|
||
**Воспроизведение:** orphan AppUser с Identity-ролью `Admin` логинится → видит SuperAdmin консоль / override-баннер.
|
||
**Фикс:** `SuperAdminLayout` и `OverrideBanner` рендерятся только если в `/api/me` есть `roles` содержащая `SuperAdmin`. Все остальные — на `/dashboard` или `/no-organization`.
|
||
|
||
### #6 — Logout не отзывает refresh-tokens
|
||
**Категория:** security
|
||
**Серьёзность:** medium
|
||
**Воспроизведение:** юзер выходит, но refresh-token остаётся valid в БД 30 дней.
|
||
**Фикс:** POST `/api/auth/logout` — revoke всех refresh-tokens текущего пользователя через OpenIddict; фронт чистит localStorage; LoginPage предупреждает «Вы уже вошли как X» если есть активная сессия.
|
||
|
||
### #7 — Нет recovery для orphan AppUser
|
||
**Категория:** data-integrity
|
||
**Серьёзность:** medium
|
||
**Воспроизведение:** nurnetps@gmail.com висит в БД с указателем на удалённую org.
|
||
**Фикс:** SQL-скрипт `deploy/recovery-restore-orphan-owners.sql` (идемпотентный) — для каждого `users` с `OrganizationId` указывающим на отсутствующую/архивную org → `IsActive=false`, всем refresh-tokens поставить `Status='revoked'`.
|
||
|
||
### #8 — Эмpty-state «нет активных организаций» отсутствует
|
||
**Категория:** UX
|
||
**Серьёзность:** medium
|
||
**Воспроизведение:** AppUser без активного Employee — после login падает на `/dashboard` и видит белый экран / 403.
|
||
**Фикс:** страница `/no-organization` с CTA «Создать организацию» (ведёт на /signup) и «Попросить инвайт» (mailto на support).
|
||
|
||
## Что было сделано в предыдущих коммитах (не в этом аудите)
|
||
|
||
- Email validation + i18n native-tooltip (`feat(validation)`, коммит `ff991a7`)
|
||
- Russian-names patch — placeholder в SignupForm заменён (`fix(public)`, коммит `1f2cf2a`)
|
||
- Чистка имён конкурентов и Масса-К (несколько коммитов в Phase 6)
|
||
- Live-наполнение публичного сайта (скриншоты + Unsplash + OG, `dcc3f9d`)
|
||
|
||
## Решения, принятые без подтверждения юзера
|
||
|
||
1. **Soft-delete vs hard-delete для Employee:** soft (`IsActive=false`). История операций сохраняется.
|
||
2. **Хранение Owner-маркера:** уже есть `Organization.AccountOwnerUserId` — использую его, новой колонки `Employee.IsOwner` не нужно.
|
||
3. **Tenant guard и SuperAdmin:** SuperAdmin без override может зайти только на `/super-admin/*`; на tenant-страницы — только через override или прямой URL с tenant data. SuperAdmin override обходит guard «активный Employee».
|
||
4. **Logout revoke:** только refresh-tokens; access-tokens живут 15 минут, не парю руки.
|
||
5. **Recovery скрипт:** идемпотентный, безопасный к повторному запуску. Не рушит данные — только деактивирует orphan AppUser.
|
||
6. **Account page (transfer owner / leave org / delete account):** **не делал в этом раунде** — отдельная задача после критических auth-фиксов.
|
||
7. **Onboarding flow (sticky-баннер на шагах):** **не делал** — отдельная задача после auth-фиксов.
|
||
|
||
## Открытые вопросы (требуют решения юзера)
|
||
|
||
1. **Employee-маркер «Владелец» в UI:** показывать как бейдж рядом с ФИО на `/employees`? Сейчас Owner определяется через `org.AccountOwnerUserId == employee.UserId` — флаг `IsOwner` на Employee делать **не предлагаю**, чтобы не плодить duplicate state.
|
||
2. **Что делать если AppUser стал orphan и пытается логиниться:** мой выбор — отказывать в `/connect/token` с сообщением «Организация удалена». Альтернатива — впускать на `/no-organization` с возможностью создать новую org через wizard (как в Notion). Если нужен второй вариант — потребует UX-проектирования.
|
||
3. **Inviting flow** (юзер без org попросил доступ к чужой): не реализовано, не в скоупе аудита.
|
||
|
||
## Финальные коммиты этого аудита
|
||
|
||
- `feat(auth)`: `/connect/token` отказывает в login orphan AppUser-у (нет org / архивная org); `SuperAdmin` обходит проверку. Файлы: `AuthorizationController.cs`.
|
||
- `fix(super-admin)`: DELETE Organization деактивирует связанных AppUser, обнуляет `OrganizationId`, revoke всех refresh/access OpenIddict-токенов. Файлы: `SuperAdminOrganizationsController.cs`.
|
||
- `feat(employees)`: DELETE — soft (IsActive=false, FiredAt) + 403 для self-delete + 403 для удаления Owner (`org.AccountOwnerUserId == employee.UserId`). Файлы: `EmployeesController.cs`.
|
||
- `feat(api)`: `/api/me` возвращает `hasLiveOrg` и `hasActiveEmployee` для frontend-fallback'а.
|
||
- `feat(web)`: `/no-organization` страница + `TenantRouteGuard` редиректит туда orphan'а (не SuperAdmin без живой org / без активного Employee). Файлы: `App.tsx`, `pages/NoOrganizationPage.tsx`, `components/TenantRouteGuard.tsx`.
|
||
- `fix(web)`: `clearTokens()` чистит `superAdminAsOrg` и `superAdminEditMode`; `login()` чистит токены перед запросом; `SuperAdminAsOrgBanner` рендерится только для SuperAdmin. Файлы: `lib/auth.ts`, `lib/api.ts`, `components/SuperAdminAsOrgBanner.tsx`.
|
||
- `chore(recovery)`: `deploy/recovery-restore-orphan-owners.sql` — деактивирует orphan AppUser, revoke токены. Применён на стейдже.
|
||
|
||
### Smoke после фикса
|
||
- `nurnetps@gmail.com` → POST /connect/token → `invalid_grant` «Неверный логин или пароль».
|
||
- `admin@food-market.local` (SuperAdmin) → login проходит.
|
||
- Публичный сайт + админка отдают 200.
|
||
- В БД: `users.IsActive=false`, 9 OpenIddict tokens у nurnetps теперь `revoked`.
|
||
|
||
## Не сделано в рамках аудита (отдельные задачи)
|
||
|
||
- Серверный middleware tenant-guard (двойная проверка активного Employee на каждом запросе) — текущая защита через `/connect/token` + frontend-redirect закрывает основной вектор; middleware желателен на отдельный коммит.
|
||
- Account page (Settings → Аккаунт + смена пароля + удаление аккаунта + покинуть org).
|
||
- Transfer-owner UI с модалом передачи прав.
|
||
- Onboarding sticky-баннер на шагах.
|
||
- Убран `Employee.IsOwner` поле — используем существующий `Organization.AccountOwnerUserId`.
|
||
|
||
Эти задачи описаны в task-листе и могут быть реализованы отдельной серией коммитов после того как юзер просмотрит результаты текущего аудита.
|