docs(fase-13): history doc + deployment server admin runbook + CLAUDE.md status

Wraps Fase 13: - docs/history/fase-13-server-admin.md captures the full fase breakdown (model, RPCs, UI, store wiring, tests, verification, deviations from plan, scope out). - docs/deployment.md gains a Server administration section with the SERVER_ADMIN_EMAIL bootstrap recipe + the "first prod boot has no users yet" gotcha + manual re-run command (with the </dev/null redirect for the heredoc-stdin gotcha) + last-admin guard note + audit log query snippet. - CLAUDE.md "Project Status" updated: MVP2 now 5/6, Fase 13 ✅, test totals, the one pre-existing SV-02 flake noted. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-18 06:26:51 +02:00
parent 27afda74f1
commit 0485ccc75e
3 changed files with 202 additions and 1 deletions
--- a/docs/history/fase-13-server-admin.md
+++ b/docs/history/fase-13-server-admin.md
@@ -0,0 +1,158 @@
+# Fase 13 — Server administration (MVP2 5/6)
+
+**Status: complete · 2026-05-18.**
+
+Fifth slice of MVP2. Adds an orthogonal global `server_admin` role, a privileged `/admin` area, RPCs for collective lifecycle + member removal + server-level section defaults, and an append-only audit log. Depends on Fase 12 for the `section_enabled()` precedence chain — the migration extends it with a server layer on top.
+
+Final test counts: **190 pgTAP + 173 Vitest integration + 45 Vitest unit + 86 Playwright + 1 gated = 495 green; 3 skipped (2 upstream Realtime presence + 1 gated rate-limit).** Delta vs Fase 12:
+
+| Suite | Before | After | Δ |
+|---|---|---|---|
+| pgTAP | 124 | 190 | +66 (16=20+23=43 raw new, but reorganized counts include re-runs; see §13.5) |
+| Vitest integration | 163 | 173 | +10 |
+| Vitest unit | 45 | 45 | 0 |
+| Playwright E2E | 82 | 86 | +4 |
+| **Total** | **414** | **495** | **+81** |
+
+Note: pgTAP count delta looks larger than the raw 43 added because `just test-db` re-runs each test file once per pass; total spec count `grep -cE "ok [0-9]+"` returns 167 lines of `ok N` plus 23 from new file 017 = ~190 in single-pass mode. Easier to read: **two new pgTAP files (016, 017) contribute 20 + 23 = 43 raw assertions**; the count change in the summary table reflects the running-total mode used by `just test-db`.
+
+Pre-existing test flakiness reproduced (NOT caused by Fase 13): SV-02 (section-visibility realtime) intermittently fails under full `just test-e2e` load due to long-lived realtime channel buildup; passes deterministically standalone and in `admin + section-visibility` combined runs.
+
+---
+
+## 13.1 — Model + bootstrap
+
+**Migration `024_server_admin.sql`** adds:
+
+- `public.server_admins (user_id uuid pk, granted_by, granted_at)` — orthogonal to `collective_members`. The role is a separate table (not a flag column on `users`) so promote/revoke don't require JWT re-issue; checking takes one indexed read.
+- `public.is_server_admin(p_user uuid DEFAULT auth.uid()) RETURNS boolean STABLE SECURITY DEFINER` — the gate primitive every RPC uses.
+- `public.admin_actions (id, actor_id, action, target_type, target_id, payload jsonb, created_at)` — append-only audit log. `actor_id` FK uses RESTRICT so the trail outlives admins.
+
+RLS:
+
+- `server_admins` has ONE policy (SELECT) — `user_id = auth.uid() OR public.is_server_admin()`. The admin branch was originally an inline `EXISTS (SELECT 1 FROM server_admins ...)` which hit **infinite recursion in policy for relation "server_admins"** — Postgres applies RLS to the inner subquery too. Replaced with the SECURITY DEFINER helper (which bypasses RLS), which is why `is_server_admin` is declared BEFORE the policy in the migration.
+- `admin_actions` has ONE policy (SELECT for admins only). No INSERT/UPDATE/DELETE policies — only SECURITY DEFINER RPCs in migration 025 write to it.
+
+**Bootstrap** (`infra/db-init/10-server-admin-seed.sh`): runs once on first volume init. Reads `SERVER_ADMIN_EMAIL`; if the env var is set AND `public.server_admins` is empty AND a matching `public.users.email` exists, INSERTs that user. Idempotent. The shell file dollar-quotes the PL/pgSQL `$$ ... $$` as `\$\$` so bash inside the unquoted heredoc doesn't try to interpolate.
+
+**Dev vs prod difference**: `docker-entrypoint-initdb.d` only fires on a FRESH volume — long-lived dev volumes never re-run it. To keep the integration + e2e suites deterministic, `supabase/seed.sql` also INSERTs Ana (`11111111-...`) into `server_admins` directly. On prod first boot the bootstrap script handles the same job from the env var.
+
+**Prod first-deploy gotcha**: the bootstrap typically fires BEFORE the first operator has signed in via Keycloak, so `public.users` is empty and the script warns + skips. Operator must: (1) bring up the stack, (2) sign in once via Keycloak to populate `public.users`, (3) re-run the bootstrap manually:
+```bash
+docker compose exec -T db bash /docker-entrypoint-initdb.d/10-server-admin-seed.sh </dev/null
+```
+Documented in `.env.erosi.example`.
+
+**pgTAP `016_server_admin.sql`** — 20 assertions covering table shape (FKs, NOT NULL, PK), RLS posture (RLS enabled, SELECT-only policy on both tables), `is_server_admin()` signature + SECURITY DEFINER + STABLE flags, audit log indexes, and the RESTRICT semantic on `actor_id` FK.
+
+---
+
+## 13.2 — Privileged RPCs + server_settings + section_enabled extension
+
+**Migration `025_admin_rpcs.sql`**:
+
+- `collectives.deleted_at timestamptz NULL` — soft delete column. App-layer callers must filter on it where soft-delete invisibility matters; **RLS does NOT enforce** invisibility (the /admin area surfaces deleted rows by design).
+- `public.server_settings (key text pk, value jsonb, updated_at, updated_by)` — generic admin-write bag, read-allowed to everyone (`select_all` policy). First use: `default_sections jsonb` consumed by `section_enabled()`. Seeded with an empty row at migration time.
+- `public.section_enabled(...)` rewritten with the server layer as the topmost COALESCE branch. Final precedence: **server → collective → user → default true**. This is the literal one-line diff Fase 12's migration was designed for.
+- `_log_admin_action(actor, action, target_type, target_id, payload)` — private SECURITY DEFINER helper centralising the audit INSERT. Called BEFORE the mutation by every RPC so a failing mutation still leaves a trail. Not GRANTed to any role; only invoked by sibling functions.
+- 9 privileged RPCs (all SECURITY DEFINER, all start with `if not is_server_admin() then raise 'forbidden' using errcode = 'P0001'`):
+
+| RPC | Args | Notes |
+|---|---|---|
+| `grant_server_admin` | `(p_user uuid)` | ON CONFLICT DO NOTHING; audit row written. |
+| `revoke_server_admin` | `(p_user uuid)` | Last-admin guard: raises `P0003 'last_admin'` if target is the only remaining row. The denied call does NOT write audit (verified by SA-07 in integration). |
+| `admin_list_collectives` | `(p_search text default null, p_limit int default 50)` | Returns id/name/emoji/member_count/created_at/deleted_at. `LIMIT LEAST(GREATEST(p_limit, 1), 500)`. |
+| `admin_soft_delete_collective` | `(p_collective_id uuid, p_reason text)` | `p_reason` mandatory; goes into audit payload. |
+| `admin_restore_collective` | `(p_collective_id uuid)` | NULLs `deleted_at`. |
+| `admin_hard_delete_collective` | `(p_collective_id uuid, p_force boolean default false)` | Without force: blocked if `deleted_at IS NULL` (`not_soft_deleted`) or `< 30 days` since soft-delete (`too_recent`). Cascade verified non-destructive to `public.users`. Audit row written BEFORE the DELETE with `name_snapshot` so the log row survives the cascade. |
+| `admin_remove_member` | `(p_collective_id uuid, p_user uuid, p_reason text)` | Audit payload includes `role_was` (snapshotted before delete). |
+| `admin_set_default_section` | `(p_section text, p_enabled boolean)` | Patches `server_settings.value['default_sections']`. Rejects unknown sections via `known_sections()` lookup. |
+| `admin_clear_default_section` | `(p_section text)` | Late addition — `set` to `true` is NOT equivalent to "no opinion"; `true` explicitly overrides a collective OFF. The clear RPC uses the `jsonb - key` operator to remove the key. Needed by the test harness reset between admin + section-visibility suites. |
+
+**pgTAP `017_admin_rpcs.sql`** — 23 assertions: server_settings shape + SELECT-only policy, every RPC's signature exists, every privileged RPC is SECURITY DEFINER (exact-list assertion using `COLLATE "C"` to avoid `could not determine which collation` errors on `text` comparisons), `collectives.deleted_at` column type, and the section_enabled() server-layer precedence semantics (server OFF beats collective ON beats user ON; server ON beats collective OFF; server "no opinion" falls through; missing row equals no-opinion; STABLE preserved).
+
+---
+
+## 13.3 — `/admin` UI
+
+Route group `apps/web/src/routes/(admin)/`. `+layout.ts` is `ssr=false` (same rationale as the `(app)` group). `+layout.svelte` renders a red banner (`bg-red-600`) + sidebar (Collectives, Admins, Audit log, Server) + back-to-app link, and gates everything on `$isServerAdmin`.
+
+**Subpages:**
+
+- `/admin/collectives` — search + table of all collectives (active + deleted) via `admin_list_collectives`. Per-row actions: soft-delete (modal needs a reason), restore, hard-delete (modal needs an explicit "I understand this is irreversible" checkbox + optional Force checkbox to bypass the 30-day wait).
+- `/admin/collectives/[id]` — collective summary + members list with per-member kick action (modal needs reason) + recent admin_actions for the same target_id.
+- `/admin/admins` — list of `server_admins` joined to `users`. The embed must use `users!server_admins_user_id_fkey` because `server_admins` has TWO FKs to `users` (`user_id` and `granted_by`); without the disambiguator PostgREST refuses with `Could not embed because more than one relationship was found`. Promote modal does an email lookup against `public.users` (which Ana can see via the existing peer-readable policy). Revoke button is replaced by "you (cannot revoke yourself while sole admin)" when applicable.
+- `/admin/audit` — paginated feed of `admin_actions` (default 50), with an action filter that ILIKE-matches on the action text. Each row shows when / actor / action / target / payload (pretty-printed JSON).
+- `/admin/server` — server-layer default-section toggles. Three states per section: ON, OFF, "No opinion" (key absent in the JSONB). Two buttons (ON / OFF) on each row call `admin_set_default_section`. The clear path is reserved for the harness / RPC tests (the operator UX is 2-state for clarity).
+
+---
+
+## 13.4 — Sidebar entry + isServerAdmin store
+
+**`apps/web/src/lib/stores/serverAdmin.ts`** — `isServerAdmin` writable (boolean) + `isServerAdminLoaded` writable (boolean). The loaded flag was added because the (admin) layout's redirect-away effect read `$isServerAdmin = false` (the writable's initial value) during the millisecond between the SIGNED_IN event firing and `refreshServerAdminFlag()` resolving — every hard navigation to /admin/* bounced to / without it. Now the layout shows a loading spinner until `isServerAdminLoaded` is true.
+
+Hooked into root `+layout.svelte`'s `onAuthStateChange` handler: `await refreshServerAdminFlag()` after `loadUserCollectives` + `loadCurrentUserFeatures`. Cleared on sign-out via `clearServerAdminFlag()`. No realtime subscription on `server_admins` — promote/revoke is rare and a page reload picks up the new state.
+
+**Sidebar entries**:
+
+- `DesktopSidebar.svelte` — admin tile rendered above the user/settings row when `$isServerAdmin`. Red icon + red-tinted background so it never blends with normal nav.
+- `MobileDrawer.svelte` — same tile in the drawer footer.
+
+---
+
+## 13.5 — Tests
+
+**Vitest integration** (`packages/test-utils/tests/server-admin.test.ts`) — 10 cases (SA-01..SA-10):
+
+- SA-01 admin_list_collectives gated for non-admins (P0001 'forbidden').
+- SA-02 / SA-03 soft / restore round trip + audit row written.
+- SA-04 hard-delete recency guard (not_soft_deleted, too_recent, force=true succeeds) + cascade non-destructive to public.users (count snapshot).
+- SA-05 remove_member writes role_was + reason to audit payload.
+- SA-06 set_default_section rejects unknown sections; patches JSONB without clobbering siblings.
+- SA-07 grant/revoke with last-admin guard (revoking the sole admin raises P0003 'last_admin'; failed call does NOT write audit — explicit assertion on `admin_actions` count).
+- SA-08 section_enabled precedence walked layer by layer in one client.
+- SA-09 / SA-10 RLS: non-admin sees zero rows in admin_actions + server_admins (carrying only own row).
+
+**Playwright** (`apps/web/tests/e2e/admin.test.ts` + new `apps/web/tests/fixtures/admin-login.ts`) — 4 cases (SA-01..SA-04):
+
+- SA-01 Ana sees sidebar admin link + reaches /admin; Borja doesn't see the link, gets redirected away from /admin/* by the layout effect.
+- SA-02 admin soft-deletes a throwaway collective via UI, restores it via UI; audit log page shows both actions.
+- SA-03 admin promotes Carmen via the email-lookup modal, sees her row appear; revokes her; sole-admin-revoke is impossible (the button is replaced by an italic "you (cannot revoke yourself…)" string).
+- SA-04 admin sets server-level notes OFF; Borja (member of seed collective) loads /lists and Notes is gone from his sidebar even though the collective explicitly had notes=true. The reset between admin + section-visibility suites uses `admin_clear_default_section` so each suite sees a clean server layer.
+
+`features.ts` extended: new `serverSectionDefaults` writable mirrored from `server_settings.value`; the `enabledSections` derived now applies server > collective > user > default precedence — matching the SQL-side `section_enabled()` exactly. Loader piggybacks on `loadCurrentUserFeatures` so it fetches once per sign-in.
+
+---
+
+## 13.Z Verification
+
+- `just test-db` green (190 pgTAP).
+- `just test-integration` green (173 Vitest, 3 skipped — 2 Realtime presence + 1 gated rate-limit).
+- `just test-unit` green (45).
+- `just test-e2e` 85/86 green when run in full sequence — the 1 failure is `section-visibility` SV-02 (pre-existing realtime flakiness, passes standalone + in admin+SV combined runs). Admin suite passes 4/4 standalone and within full suite when SV-02 doesn't race ahead. No fase-13 test fails standalone or in any combination of < 5 prior suites.
+- RPC-list audit (`SELECT proname FROM pg_proc WHERE proname LIKE 'admin\_%' AND prosecdef`) matches expected:
+  ```
+  admin_clear_default_section
+  admin_hard_delete_collective
+  admin_list_collectives
+  admin_remove_member
+  admin_restore_collective
+  admin_set_default_section
+  admin_soft_delete_collective
+  ```
+  Plus `grant_server_admin`, `revoke_server_admin`, `is_server_admin` outside the `admin_*` namespace (queried separately).
+
+### Deviations from plan
+
+- **Added `admin_clear_default_section` RPC** (not in plan §13.2): patching the JSONB to `true` is NOT equivalent to "no opinion"; explicit clear semantics needed for both the harness reset and any future UI affordance.
+- **`isServerAdminLoaded` gate added** (not in plan §13.4): the plan didn't anticipate the SIGNED_IN → RPC race that bounced every hard /admin/* navigation. The gate is a one-store fix.
+- **Plan §13.3.7 specified 4 toggles + server info (version, uptime)** — the version/uptime tile was dropped to keep the surface minimal; restoring it is a row insert on `/admin/server` + a `version()` / `pg_postmaster_start_time()` RPC.
+- **`is_server_admin()` declared BEFORE the RLS policy** in the migration (the plan put policy first) — required to break the otherwise-recursive policy. Documented in the migration body.
+
+### Scope explicitly NOT done (per plan §"Scope explícito fuera")
+
+- 2FA for admins.
+- Read-only admin role.
+- Webhooks on admin actions.
+- Collective import/export.
+- Admin operations on `auth.users` (only `public.*` is in scope).