|
|
@@ -11,7 +11,7 @@
|
|
|
>
|
|
|
> Each finding is referenced as **F<N>** for later citation.
|
|
|
>
|
|
|
-> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (5 fixed, 22 open), 42 sev-1.
|
|
|
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (6 fixed, 21 open), 42 sev-1.
|
|
|
|
|
|
---
|
|
|
|
|
|
@@ -296,6 +296,51 @@
|
|
|
separate audit signal. Combined with F3, a leaked service token is
|
|
|
unconstrained Admin.
|
|
|
- **Severity: 2**
|
|
|
+- **Status:** Fixed. Three layers:
|
|
|
+ 1. **Active-status gate.** Migration
|
|
|
+ `20260504110000_add_disabled_at_to_users` adds nullable
|
|
|
+ `disabled_at` to `users`; `User::isDisabled()` exposes the
|
|
|
+ predicate; `ImpersonationMiddleware` returns `403 user_disabled`
|
|
|
+ for any disabled target (mirroring the unknown-user 403 so
|
|
|
+ attackers cannot use the response to distinguish "missing" from
|
|
|
+ "disabled"). `AuthController::upsertOidc` and `upsertLocal`
|
|
|
+ short-circuit with 403 before recomputing role on a disabled row,
|
|
|
+ so a disabled user cannot drift their role via OIDC group
|
|
|
+ membership while disabled.
|
|
|
+ 2. **Distinct audit signal.** Migration
|
|
|
+ `20260504110001_add_actor_via_to_audit_log` adds `actor_via`
|
|
|
+ (`oidc|local|admin-token|service|reporter|consumer|system`).
|
|
|
+ `ImpersonationMiddleware` threads the resolved user's
|
|
|
+ `is_local` flag onto `AuthenticatedPrincipal`, and
|
|
|
+ `AuditContextMiddleware` derives `actor_via` from it — so
|
|
|
+ audit rows split impersonated-OIDC from impersonated-local
|
|
|
+ without joining `users`. `/admin/audit-log?actor_via=local`
|
|
|
+ surfaces only local-admin actions for review.
|
|
|
+ 3. **Admin user-CRUD.** New `UsersController` exposes
|
|
|
+ `GET /api/v1/admin/users`, `GET /{id}`, and
|
|
|
+ `PATCH /{id}` (body `{disabled: bool}`). PATCH wraps the
|
|
|
+ state change + `user.disabled` / `user.enabled` audit emit in
|
|
|
+ `Connection::transactional()` per F4. Refused with 409 on
|
|
|
+ self-disable (`cannot_disable_self`) or on disabling the
|
|
|
+ local-admin row (`cannot_disable_local_admin`) — the local
|
|
|
+ admin is the documented break-glass path; operators wanting
|
|
|
+ to lock it disable it via `LOCAL_ADMIN_PASSWORD_HASH` in the
|
|
|
+ UI's env. UI page at `/app/users` (admin-only sidebar entry).
|
|
|
+ OIDC and local-login controllers route the upstream
|
|
|
+ `403 user_disabled` to `/no-access` (OIDC) or to a generic
|
|
|
+ "invalid credentials" flash (local — probe-resistant).
|
|
|
+ Regression tests: `api/tests/Integration/Auth/DisabledUserTest.php`
|
|
|
+ covers (a) impersonation 403 on disabled rows,
|
|
|
+ (b) `upsertOidc` rejection without role recompute or audit drift,
|
|
|
+ (c) `upsertLocal` rejection on disabled local admin,
|
|
|
+ (d) `actor_via` derivation for impersonated local vs OIDC
|
|
|
+ vs admin-token paths,
|
|
|
+ (e) admin disable + audit emit + idempotency,
|
|
|
+ (f) self-disable / local-admin-disable 409 guards,
|
|
|
+ (g) `/audit-log?actor_via=local` filter.
|
|
|
+ The remaining "allow-list" framing of F11 is by-design per SPEC §8:
|
|
|
+ any user the UI BFF has upserted is impersonatable. The disabled
|
|
|
+ flag is the operator's lever to revoke that consent.
|
|
|
|
|
|
### F12 — Local-admin lookup matches on `display_name` without uniqueness guarantee
|
|
|
- **Files:** `api/src/Infrastructure/Auth/UserRepository.php:50-60, 119-160`
|
chiappa