fc9f7c9ee4
Some checks failed
CI / POS (WPF, Windows) (push) Waiting to run
CI / Backend (.NET 8) (push) Successful in 53s
CI / Web (React + Vite) (push) Successful in 42s
Docker API / Build + push API (push) Successful in 1m19s
Docker Web / Build + push Web (push) Successful in 35s
Docker API / Deploy API on stage (push) Failing after 38s
Docker Web / Deploy Web on stage (push) Successful in 13s
Завершающий пункт пакета фиксов по ролям/валидации/удалению. Обход: 1. /connect/token — IsActive + BelongsToLiveOrg + SuperAdmin bypass. 2. JWT cookie vs Bearer — все три AuthN-схемы переопределены в OpenIddictValidationAspNetCoreDefaults; cookie не активна для API. 3. X-Org-Override — фильтрует по IsInRole(SuperAdmin), подделать нельзя. 4. Tenant query filters — ITenantEntity и IOptionalTenantEntity подключаются через reflection, фильтр консистентен с tenant.context. 5. Smoke per-role — sidebar+RoleGuard за один проход покрывает все tenant-роуты; tenant-admin на /super-admin URL → описан risk + future fix. 6. Reset password / deactivate account — токены revoke в БД одним SQL. 7. Catch-22 для SuperAdmin платформы — он не Employee и не имеет OrganizationId, через текущие endpoint-ы deactivate невозможен. Findings разбиты на critical (закрыто этим пакетом), high/medium (не закрыто — будущая серия) и low (косметика).
108 lines
11 KiB
Markdown
108 lines
11 KiB
Markdown
# Системный аудит авторизации — 2026-05-06
|
||
|
||
Финальный пункт пакета фиксов по системе ролей. Прохожу цепочку
|
||
авторизации от логина до серверной защиты конкретных endpoint-ов
|
||
и фиксирую все findings; критичные — сразу починены, в коммитах
|
||
этого же дня.
|
||
|
||
## 1. Логин: OpenIddict /connect/token
|
||
|
||
`AuthorizationController.cs`:
|
||
- Password grant: `_userManager.FindByNameAsync` + `CheckPasswordSignInAsync` + проверка `User.IsActive`.
|
||
- После успешного password-grant — дополнительная проверка `CheckUserStillBelongsToLiveOrgAsync` (исключение для SuperAdmin): отказывает в токене, если `User.OrganizationId` указывает на удалённую/архивированную org. Это закрывает orphan-AppUser сценарий из аудита 2026-04-27.
|
||
- Refresh grant: повторно проверяет `IsActive` и `BelongsToLiveOrg`.
|
||
- Поле `org_id` пишется в JWT-claims как `HttpContextTenantContext.OrganizationClaim`.
|
||
|
||
**Status: OK.** Проверки покрывают: deactivated user, orphan org, SuperAdmin override.
|
||
|
||
## 2. JWT cookie vs Bearer
|
||
|
||
API использует только Bearer-токены через OpenIddict. Cookie-схему AspNetCore Identity подавляет `AddAuthentication` (см. `Program.cs:108-113` — все три схемы переопределены в `OpenIddictValidationAspNetCoreDefaults`). Это критично — иначе `[Authorize]` бы редиректил API-запросы на `/Account/Login`.
|
||
|
||
**Status: OK.**
|
||
|
||
## 3. X-Org-Override (impersonation)
|
||
|
||
`HttpContextTenantContext.cs`:
|
||
- `OrgOverrideHeader = "X-Org-Override"`.
|
||
- `TryGetHttpOverrideOrg`: возвращает `true` ТОЛЬКО если `User.IsInRole("SuperAdmin")` И header присутствует. Обычный юзер не может задать override (даже если подсунет header — `IsInRole` фильтрует).
|
||
- В режиме override `IsTenantOverride=true`. Tenant-фильтр в `AppDbContext.ApplyTenantFilter` строится так:
|
||
```
|
||
(IsSuperAdmin && !IsTenantOverride) || OrganizationId == _tenant.OrganizationId
|
||
```
|
||
То есть SuperAdmin без override видит всё; SuperAdmin в override — фильтр обязан применяться к выбранному `OrganizationId`. Ровно так, как нужно.
|
||
|
||
**Status: OK.** Проверка роли защищает от подделки header'а.
|
||
|
||
## 4. Tenant query filters
|
||
|
||
`AppDbContext.cs:109-153`:
|
||
- Для каждого `ITenantEntity` через reflection ставится `HasQueryFilter`.
|
||
- Для `IOptionalTenantEntity` (системные справочники с nullable `OrganizationId`) — отдельный фильтр: NULL-записи видны всем, остальные — обычная изоляция.
|
||
- Все Identity-таблицы (Users/Roles/UserRoles) — НЕ tenant-scoped (они не реализуют ITenantEntity), запросы к ним идут без фильтра. Это by design — Identity управляется через UserManager/RoleManager.
|
||
|
||
**Status: OK.**
|
||
|
||
## 5. Smoke (UI) ожидаемое поведение по ролям
|
||
|
||
Согласно `AppLayout.buildNav` (после step 7) и `RoleGuard` (новый в этом пакете):
|
||
|
||
| Юзер пытается зайти | Поведение |
|
||
|---|---|
|
||
| Cashier на `/super-admin/orgs` | TenantRouteGuard / RoleGuard **не пускает** на /super-admin (он под отдельным layout с `[Authorize(Roles = "SuperAdmin")]` на эндпойнтах). Юзер увидит «Нет доступа» из RoleGuard и/или 403 от API. |
|
||
| Storekeeper на `/settings/employees` | RoleGuard `roles=['Admin']` → «Нет доступа». |
|
||
| Cashier на `/catalog/counterparties` | RoleGuard `roles=['Admin']` → «Нет доступа». |
|
||
| Tenant-Admin на `/super-admin/...` | TenantRouteGuard для не-SuperAdmin не редиректит туда (он только tenant-роуты охраняет); сам `/super-admin` под `<SuperAdminLayout>` без guard'а, но все `[Authorize(Roles = "SuperAdmin")]` на endpoint-ах вернут 403. UI покажет 403-страницы пустые таблицы / ошибки. **Findings:** добавить RoleGuard на сам `<Route path="/super-admin">` чтобы Tenant-Admin не видел индиго-sidebar админа платформы. → Не сделано в этом пакете, описано как future. |
|
||
| 401 на любом запросе | `api.ts` interceptor: попытка refresh; если refresh упал — `clearTokens()` + редирект на `/login`. |
|
||
|
||
## 6. Reset пароля и инвалидация токенов
|
||
|
||
`SuperAdminEmployeesController.ResetPassword`:
|
||
- `_userMgr.RemovePasswordAsync` + `AddPasswordAsync(temp)`.
|
||
- `UPDATE OpenIddictTokens SET Status='revoked' WHERE Subject = userId AND Status='valid'` — обрывает все активные сессии.
|
||
|
||
`SuperAdminEmployeesController.ToggleAccountActive` при `IsActive=false`:
|
||
- Те же `revoked` для всех valid токенов.
|
||
|
||
**Status: OK.**
|
||
|
||
## 7. Catch-22: SuperAdmin блочит свою же учётку
|
||
|
||
`SuperAdminEmployeesController` оперирует на сущности `Employee` конкретной org (`/api/super-admin/organizations/{orgId}/employees/...`). SuperAdmin платформы — это `User` БЕЗ `OrganizationId` и БЕЗ `Employee`. Через этот контроллер до его учётки не дойти.
|
||
|
||
Других endpoint-ов, через которые можно `User.IsActive=false` для произвольного user-id — НЕТ. `SuperAdminOrganizationsController.Delete` деактивирует только тех, чей `OrganizationId` совпадает с удаляемой org — SuperAdmin платформы туда не попадает (`u.OrganizationId == null`).
|
||
|
||
**Status: OK сейчас.** Future risk: если добавится `/api/super-admin/users/...` с возможностью deactivate любого user-id, нужен гард `if (currentUserId == targetUserId) → 403 «нельзя себя»`. Запишу как TODO.
|
||
|
||
## 8. Findings (зафиксированы / не зафиксированы)
|
||
|
||
### Critical (зафиксировано в этом пакете)
|
||
|
||
| # | Описание | Где | Коммит |
|
||
|---|---|---|---|
|
||
| 1 | `Manager` — лишняя системная роль, путала UI и Authorize-гарды | SystemRoles, 13 контроллеров, DevDataSeeder | `fce9be9` |
|
||
| 2 | Системная роль выкидывала alert вместо show-permissions | EmployeeRolesPage | `77de34f` |
|
||
| 3 | ИИН-формы маркированы как «ИНН/ИИН» (РФ-термин) | EmployeesPage, Counterparties, SuperAdmin* | `9a31650` |
|
||
| 4 | Salary через `<input type=number>` (не учитывал org-настройку копеек) | EmployeesPage | `9f9d273` |
|
||
| 5 | type=email не требовал TLD на patternMismatch | TextInput общий | `ed7740e` |
|
||
| 6 | Удаление сотрудника одноступенчатое, нельзя «уволить → удалить» отдельно | Employee domain + EmployeesController + UI | `049e847` |
|
||
| 7 | Sidebar показывал Cashier/Storekeeper лишние пункты | AppLayout + RoleGuard + App.tsx | `542eff2` |
|
||
|
||
### High / Medium (не зафиксировано — отдельная серия)
|
||
|
||
- **Tenant-Admin может открыть `/super-admin` URL** и увидеть пустой индиго-sidebar (API вернёт 403 на каждый запрос). Нет RoleGuard на сам Route `/super-admin/*`. Фикс: обернуть `<Route element={<RoleGuard roles={['SuperAdmin']}><SuperAdminLayout /></RoleGuard>}>` или добавить ранний return в SuperAdminLayout.
|
||
- **`Authorize(Policy = "AdminAccess")`** в `MoySkladImportController`/`AdminJobsController`/`AdminCleanupController` — policy в `Program.cs:118-119` пропускает Admin **или** SuperAdmin. SuperAdmin без override проходит — нужен ли он там? Если нет (cleanup задачи tenant-scoped), тогда либо `IsTenantOverride` обязателен, либо policy сузить до Admin. Это не critical, но архитектурно хочется единообразия.
|
||
- **Catch-22 защита для будущего `/api/super-admin/users/...`** — если такой endpoint появится, нужно `if (currentUserId == targetUserId) → 403`. Сейчас такого endpoint-а нет, риска тоже нет.
|
||
- **Identity-Manager-роль** (`AddToRoleAsync(user, "Manager")`) использовалась только в DevDataSeeder и signup; обе ветки убраны в `fce9be9`. У существующих юзеров Identity-роль `Manager` может остаться в БД (раньше signup её ставил → нет, signup ставил `Admin`, Manager только для dev-сидов). Нужно ли `RoleManager.DeleteAsync(Manager)`? Решение: оставил; роль есть в БД, но нигде не назначается и не используется в коде. Безопасно.
|
||
- **DevDataSeeder продолжает создавать Demo Market и admin@food-market.local** на каждом старте API. Для production это лишнее — Demo Market и dev-admin засоряют prod-БД. Не критично сейчас (dev-данные предсказуемы), но стоит вынести seed в `IsDevelopment()`.
|
||
- **MoneyInput не используется** в SuperAdminOrgEmployeesPage (там нет поля Salary). При добавлении Salary в SuperAdmin-form'у нужно сразу применять MoneyInput.
|
||
|
||
### Low (косметика / документация)
|
||
|
||
- `EmployeesPage.useEffect` имеет логику дефолтной роли «Менеджер ?? roles[0]» — после удаления Manager-сидера всегда упадёт на roles[0]. Не баг, но стоит переписать на `Кассир` или `Кладовщик` как дефолт.
|
||
- `EmployeeRole.cs` summary упоминает «Менеджер/Кладовщик/Закупщик/Бухгалтер» — устарело, обновить.
|
||
|
||
## Итог
|
||
|
||
Все 8 пунктов задачи закрыты или зафиксированы в этом отчёте. 7 атомарных коммитов между `fce9be9..542eff2`. Билд и API, и web проходят чисто (0 errors). Финальный отчёт по аудиту оставляю в отдельном коммите вместе с обновлённой docs-секцией.
|