From fc9f7c9ee45dbcbf8cc124a35fa084431c62f16e Mon Sep 17 00:00:00 2001 From: nns <278048682+nurdotnet@users.noreply.github.com> Date: Wed, 6 May 2026 11:31:02 +0500 Subject: [PATCH] =?UTF-8?q?docs(audit):=20=D0=BF=D0=BE=D0=BB=D0=BD=D1=8B?= =?UTF-8?q?=D0=B9=20=D0=B0=D1=83=D0=B4=D0=B8=D1=82=20=D1=86=D0=B5=D0=BF?= =?UTF-8?q?=D0=BE=D1=87=D0=BA=D0=B8=20=D0=B0=D0=B2=D1=82=D0=BE=D1=80=D0=B8?= =?UTF-8?q?=D0=B7=D0=B0=D1=86=D0=B8=D0=B8=20=E2=80=94=202026-05-06?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Завершающий пункт пакета фиксов по ролям/валидации/удалению. Обход: 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 (косметика). --- docs/audit-2026-05-06.md | 107 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 docs/audit-2026-05-06.md diff --git a/docs/audit-2026-05-06.md b/docs/audit-2026-05-06.md new file mode 100644 index 0000000..dc68ae0 --- /dev/null +++ b/docs/audit-2026-05-06.md @@ -0,0 +1,107 @@ +# Системный аудит авторизации — 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` под `` без guard'а, но все `[Authorize(Roles = "SuperAdmin")]` на endpoint-ах вернут 403. UI покажет 403-страницы пустые таблицы / ошибки. **Findings:** добавить RoleGuard на сам `` чтобы 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 через `` (не учитывал 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/*`. Фикс: обернуть `}>` или добавить ранний 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-секцией.