Pārlūkot izejas kodu

Docs: mark R01-N23 fixed in 22a3840

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 2 dienas atpakaļ
vecāks
revīzija
3853ddace5
1 mainītis faili ar 39 papildinājumiem un 5 dzēšanām
  1. 39 5
      doc/REVIEW_01.md

+ 39 - 5
doc/REVIEW_01.md

@@ -808,12 +808,46 @@ note. Do not delete entries — they're history.
 
 ### R01-N23 — `users` table not deleted; emails not redacted on demote
 - **Severity**: MEDIUM (privacy / GDPR-ish).
-- **Status**: open — documented in `UserController` header comment as
-  intentional ("users are never deleted; they'd orphan audit rows").
-- **Where**: `src/Controllers/UserController.php` lines 16-25.
-- **What**: A user demoted from admin (or who simply leaves the org) keeps
+- **Status**: fixed-in-`22a3840` — went with the audit's
+  Suggested-fix plan exactly: nullable `users.tombstoned_at` column
+  (migration 006) plus a soft erasure flow surfaced as an explicit
+  admin action. `User::publicEmail()` / `publicDisplayName()` return
+  the documented `(former user)` label when tombstoned; live views
+  (Users page, anywhere a `User` object surfaces) consult those
+  helpers, while `email` / `displayName` on the snapshot — and
+  therefore everything that flows into the `audit_log.user_email`
+  column — keep the historical value verbatim. The trail is the
+  authoritative record; erasure regulations permit retention for
+  legal / security purposes (documented in admin-manual §5.1.1
+  with that stance spelled out for the operator). New
+  `tombstone()` controller action routed at
+  `POST /users/{id}/tombstone` with a hidden `action`
+  (`tombstone` / `restore`) field; pure-static `tombstoneGuardrail()`
+  rejects self-tombstone, blocks tombstoning the last remaining
+  admin (because tombstone forces `is_admin=0`), and short-circuits
+  no-ops. A successful sign-in (OIDC or local-admin / forceAdmin)
+  clears `tombstoned_at` — the marker is a display redaction, not an
+  access control; if the org actually wants to revoke access the
+  OIDC tenant is the right place. Audit action labels: TOMBSTONE
+  and RESTORE on `entity_type=user`, picked up automatically by the
+  `/audit` filter dropdown via `distinctActions()`. Tests:
+  325 / 853 (was 311 / 823). UserRepositoryTest grew by five cases
+  (auto-untombstone on both sign-in paths, force-demote, restore-
+  doesn't-promote, public-label assertion). UserControllerTest grew
+  by eight (tombstoneGuardrail matrix).
+- **Where**:
+  - `migrations/006_users_tombstoned.sql` (new column).
+  - `src/Domain/User.php` (snapshot field + display helpers).
+  - `src/Repositories/UserRepository.php` (`setTombstoned()` + auto-
+    clear in `upsertFromOidc`).
+  - `src/Controllers/UserController.php` lines 16-25 (header
+    rewritten) + new `tombstone()` action and
+    `tombstoneGuardrail()`.
+  - `views/users/index.twig`, `views/audit/index.twig` (display).
+  - `doc/admin-manual.md` §5.1.1, `SPEC.md` §7.
+- **What**: A user demoted from admin (or who simply leaves the org) kept
   their `email` and `display_name` in the `users` table forever; the audit
-  log's `user_email` column also retains it. There is no redact / tombstone
+  log's `user_email` column also retained it. There was no redact / tombstone
   flow.
 - **Why it matters**: Privacy regulations may require name/email erasure
   on request. The current model can't honour that without breaking audit.