From 75ac930d0c88f1e3d99358ab823dc5cf628ad3f6 Mon Sep 17 00:00:00 2001 From: Matt Mabrey Date: Fri, 18 Sep 2026 09:21:29 -0700 Subject: [PATCH] waves 11-13: planning docs for CR-019, CR-020, D18 CR-019 - usage/activity metrics (admin console), wave 11 CR-020 - bulk editing of users, wave 12 D18 - Okta/AD deprovisioning detection and auto-disable, wave 13 Raised by Matt Mabrey 2026-09-17. Decision record and task breakdowns only in this commit - no feature code yet. --- docs/waves/decisions-2026-09-17.md | 327 +++++++++++++++++++++++++++++ docs/waves/wave-11.md | 223 ++++++++++++++++++++ docs/waves/wave-12.md | 193 +++++++++++++++++ docs/waves/wave-13.md | 200 ++++++++++++++++++ 4 files changed, 943 insertions(+) create mode 100644 docs/waves/decisions-2026-09-17.md create mode 100644 docs/waves/wave-11.md create mode 100644 docs/waves/wave-12.md create mode 100644 docs/waves/wave-13.md diff --git a/docs/waves/decisions-2026-09-17.md b/docs/waves/decisions-2026-09-17.md new file mode 100644 index 0000000..6f5aaeb --- /dev/null +++ b/docs/waves/decisions-2026-09-17.md @@ -0,0 +1,327 @@ +# Decisions — September 17, 2026 + +Three items. Like `D11`, `D15`, `D16` and `D17`, these are new scope raised after +R2 closed out (see `docs/reference/completion.md`, T9.7), not a reopening of +anything already decided there. + +`CR-019` and `CR-020` get `CR` ids because they are field/product-facing feature +requests — the same kind of thing `CR-001`-`CR-018` were — not internal +engineering calls made mid-build. `D18` gets a `D` id because it is exactly that: +an internal security/architecture call, the same category as `D15` (Okta vs. +LDAPS), not a feature a user asked for. + +Requested/raised by Matt Mabrey, 2026-09-17. + +--- + +## CR-019 — Usage and activity metrics (admin console) + +- **Area:** Admin Console / Monitoring +- **Priority:** proposed High — the current admin console has no reliable way to + answer "who is using this and how much," which is the same visibility gap the + original UX review found in `B4` (numbers that look authoritative but are not). +- **Source:** Matt Mabrey, 2026-09-17. + +### Why this is not already built + +`D5`/`T7.10` already shipped a usage-analytics feature — `html/wp-usage.js`, with +a report at `admin.js:666-699` — but it reads `localStorage`, which is scoped to +one browser. The report's own empty state says exactly this: *"No usage recorded +in this browser yet."* It cannot show who across the team is active or what they +use, because each person's activity exists only on their own machine. This is the +same class of defect `B4` named for the pipeline strip — a number that looks +authoritative but is not — just never generalized to this feature. `CR-019` is +therefore new server-side work, not a UI addition on top of what exists. + +Two things already in the schema are relevant and reused rather than duplicated: +`User.last_login_at` (one timestamp, no history) and `AuditLog` (append-only, +already records business mutations — WP created, status changed, role changed — +per user, with a timestamp). Neither captures navigation or feature-open events, +which is the actual gap. + +### Intent + +Give admins a real, server-backed picture of who is using the suite, what parts +of it they use, and how active they are — replacing the per-browser report as +the thing anyone actually looks at. + +### Decisions, made 2026-09-17 + +1. **Retention: indefinite.** No automatic purge of usage-event data. (Separate + from `AuditLog`'s own retention, which this does not change.) +2. **Scope: suite-wide, filterable.** Not per-project by default; the console + provides filters (date range, project, user, tool/page) rather than scoping + the data itself. +3. **Export: raw view shows real identities; export supports sanitization.** + The admin console's own tables and the CSV export both default to real + usernames — this is an internal audit tool, not a public one. But the export + also offers a "sanitize" toggle that replaces the actor with a **stable + pseudonymous id** (a per-user hash, consistent across rows and across + export runs) rather than dropping the identity field outright — so an + external system (Power BI or similar) can still group and trend "by user" + without ever receiving a real name. Flagged here as the recommended + approach rather than the only one Matt confirmed in so many words: if a + fully-anonymous (no stable id at all) export turns out to be what's + actually wanted, that is a one-line change to the same feature, raise it + in the PR rather than treating it as blocking. +4. **The old per-browser report is retired**, not kept alongside the new one. + Once `CR-019` ships, `admin.js`'s existing `usage-admin` panel (reading + `WPUsage.load(...)` per browser) is removed rather than left next to the + real report, where it would show a smaller, misleading number for whoever + happens to have it open. `html/wp-usage.js` and its two call sites + (`html/work-package-suite-app.js`'s wizard dwell-tracking, and the + creator's equivalent) are a separate question — the *recording* code can + stay or go independent of the *admin report* being retired, since dwell + events were never reliably tied to a real identity anyway. Default to + removing both unless a task finds a reason to keep the recorder; log that + reason rather than deciding it here. + +### Acceptance criteria + +- A new admin-only tab in `admin.html` (same role gate as the User Directory) + shows: active users over a selectable date range (day/week/month), each + user's last-active timestamp, and a breakdown of which tools/pages get + opened and how often. +- Server-side event capture, keyed to the authenticated session (real identity, + not a browser-local guess) — a new table, not an extension of `AuditLog`, + since page-open/navigation events are not business mutations and mixing them + in would make `AuditLog` noisy for its existing, narrower purpose. +- The console's filters cover date range, project, user, and tool/page, and + combine (e.g., "user X, last 30 days, field view only"). +- CSV export from the console, in both raw (real usernames) and sanitized + (stable pseudonymous id per user) modes. +- Retention is indefinite; nothing in this item purges data. +- View-only, refresh-on-load. No alerting — matches how the rest of the admin + console works today; if that changes later it is new scope, not a rider on + this item. +- The old per-browser "Usage" report and its admin-console panel are removed + in the same wave, not left running alongside the new one. +- Accessible per `CLAUDE.md`'s standing `C1` rules (this is a new component, + not a legacy one — it ships accessible or it is not done, same as + everything else built since wave 7). + +### Frontend/backend boundary + +This needs server work, the same way `CR-004`/`CR-018`/`B4` did: a real table, +a capture endpoint, an aggregation endpoint, and an export endpoint. If a task +under this item is being built by writing to `localStorage`, it is rebuilding +the exact defect this item exists to replace — stop and say so, per +`CLAUDE.md`. + +### Scheduling + +New wave. Wave 10 (Okta) is merged, so this does not wait on anything. +Task breakdown: `docs/waves/wave-11.md`. + +--- + +## CR-020 — Bulk editing of users in the admin console + +- **Area:** Admin Console / User Directory +- **Priority:** proposed High — three of the four actions below touch access + control (role, project assignment, active/disabled) and the fourth is a hard + delete; getting the guardrails right matters more than getting it built fast. +- **Source:** Matt Mabrey, 2026-09-17. + +### Why this is not already built + +Every user-editing action in `html/users.js` today is one row, one action: +a role `` controls, keyboard-reachable, `aria-live` on any +count that updates without a page reload, focus visible throughout. + +**Done when:** + +- [ ] the tab is reachable only by an admin (verify: a non-admin session gets + no tab and the route itself refuses the request server-side, not just a + hidden tab) +- [ ] every filter is a real form control, keyboard-operable +- [ ] both export buttons produce the files T11.4 defines +- [ ] works at 390px and 1440px + +--- + +### T11.6 — CR-019: retire the per-browser Usage report + +- **Items:** `CR-019` +- **Depends on:** T11.5 +- **Blocks:** T11.7 +- **Surface:** `html/` +- **Files:** `html/admin.js` (the `usage-admin` panel, `admin.js:666-699`), + `html/wp-usage.js` and its two call sites + +**Do:** Remove the old per-browser `usage-admin` panel from `admin.js` now that +the real one exists, per the 2026-09-17 decision. Decide what happens to +`wp-usage.js`'s recording calls (wizard dwell-tracking, the creator's +equivalent): they were never reliably tied to a real identity, so they are not +a data source the new report can adopt. Default to removing the recorder too +unless it is still doing something useful on its own (re-read what it actually +records before deciding — do not assume from this file alone). + +**Do not:** leave the old panel in place "just in case." Two activity reports +showing two different numbers is worse than one. + +**Done when:** + +- [ ] the old `usage-admin` panel and its markup are gone from `admin.html`/ + `admin.js` +- [ ] a decision on `wp-usage.js` itself is recorded (removed, or kept with a + stated reason) — not left ambiguous +- [ ] nothing else in the app references the removed code; grep confirms + +--- + +### T11.7 — CR-019: verification + +- **Items:** `CR-019` +- **Depends on:** T11.6 +- **Blocks:** nothing +- **Surface:** `html/` + `server/` +- **Files:** as touched above + +**Do:** Full verification per `CLAUDE.md`: run the app locally, exercise the +new tab at 390px and 1440px, before/after screenshots, run the existing smoke +test and `seed_demo.py`, run the full suite. + +**Done when:** + +- [ ] all `CR-019` acceptance criteria in `decisions-2026-09-17.md` are met or + a failure is stated with a reason +- [ ] screenshots committed +- [ ] smoke test and `seed_demo.py` both still pass +- [ ] full test suite passes + +--- + +## Wave 11 exit criteria + +- [ ] real, server-side activity data exists per user, indefinitely retained +- [ ] the admin console shows it, filterable by date/project/user/tool +- [ ] export works in both raw and sanitized form +- [ ] the old per-browser report is gone, not duplicated +- [ ] `CR-019` fully accounted for, no open acceptance criteria diff --git a/docs/waves/wave-12.md b/docs/waves/wave-12.md new file mode 100644 index 0000000..d796392 --- /dev/null +++ b/docs/waves/wave-12.md @@ -0,0 +1,193 @@ +# Wave 12 — Bulk editing of users + +**Items:** `CR-020` +**Depends on:** wave 10 merged (it is). Independent of wave 11 (`CR-019`) — no +shared files, may build in either order or in parallel. +**Decision record:** `docs/waves/decisions-2026-09-17.md` + +Six tasks, in build order. + +--- + +### T12.1 — CR-020: bulk endpoints + +- **Items:** `CR-020` +- **Depends on:** nothing (first task) +- **Blocks:** T12.2, T12.4 +- **Surface:** `server/` +- **Files:** `server/app.py` + +**Do:** Add bulk variants of the four existing single-user actions — role +change, active/disabled, project assignment (add/remove + project-role), and +delete. Each takes a list of `user_id`s plus the action's parameters and +applies `require_see_user`/`require_manage_user`/`grantable_roles` **per row**, +exactly as the single-user endpoint does today — a super user's bulk request +cannot reach further than their existing single-user requests can. Do not skip +the self-action guard: an actor's own account is rejected out of any batch +that would disable, demote, or delete it, same as today. + +Each successful row writes its own `AuditLog` entry via `log_event`, same +action names the single endpoints already use. A row that fails is reported in +the response (user id, reason) and does not stop the rest of the batch from +being attempted. + +**Do not:** invent a single opaque "bulk_action" audit entry in place of the +per-row entries. Do not build a soft-delete path for bulk delete that doesn't +exist for the single case — bulk delete stays a hard delete, matching +`delete_user` today. + +**Done when:** + +- [ ] each of the four bulk actions is callable with a list of user ids and a + single set of parameters +- [ ] a batch containing the actor's own account rejects only that row, not + the whole batch — verified for disable, demote, and delete +- [ ] a super user's batch that includes a user/project outside what they + manage rejects only that row, with a stated reason +- [ ] every successful row produces its own `AuditLog` entry, identical in + shape to what the single-user endpoint would have written +- [ ] a batch with some failing rows still applies to the rows that succeed, + and the response lists exactly which rows failed and why + +--- + +### T12.2 — CR-020: row selection in the User Directory table + +- **Items:** `CR-020` +- **Depends on:** T12.1 +- **Blocks:** T12.3 +- **Surface:** `html/` +- **Files:** `html/users.js`, `html/users.html` + +**Do:** Add a checkbox per row and a select-all control, respecting whatever +filter (`role`, `active`/`disabled`, project) is currently applied — select-all +selects the filtered set, not every user in the system regardless of what's +shown. A bulk-action toolbar appears once at least one row is checked and +disappears at zero. + +**Done when:** + +- [ ] select-all selects exactly the rows currently visible under the active + filter, not the full unfiltered table +- [ ] changing the filter while rows are selected does something sane and + visible (either clears the selection or keeps it explicit which rows are + still selected) — pick one and state it, don't leave it undefined +- [ ] the toolbar is keyboard-reachable and only present when >=1 row is + selected + +--- + +### T12.3 — CR-020: bulk-action toolbar + +- **Items:** `CR-020` +- **Depends on:** T12.2 +- **Blocks:** T12.5 +- **Surface:** `html/` +- **Files:** `html/users.js` + +**Do:** Wire the toolbar to T12.1's endpoints for role change, activate/ +disable, and project assignment (add to / remove from a project + project- +role). Confirmation before applying uses the `wp-dialog` kit (`T7.9`) — +`wpConfirmDialog`, not a native `confirm()` — naming exactly how many users are +affected. On completion, report per-row results if anything failed (T12.1 +already returns this) rather than a single success/failure toast that hides a +partial failure. + +**Delete is built separately, in T12.5** — do not wire delete here. + +**Done when:** + +- [ ] role change, activate/disable, and project assignment each work end to + end against a multi-row selection +- [ ] the confirmation dialog names the exact affected count before anything is + sent +- [ ] a batch with a partial failure shows which rows failed, not just an + undifferentiated error +- [ ] `aria-live` announces the outcome + +--- + +### T12.4 — CR-020: CSV upload path + +- **Items:** `CR-020` +- **Depends on:** T12.1 +- **Blocks:** T12.6 +- **Surface:** `html/` + `server/` +- **Files:** `html/users.js`, `server/app.py` + +**Do:** An upload accepting a list of usernames plus the action to apply, +following the validate-and-report pattern `CR-005` established: reject and +report bad rows (username not found, actor lacks permission over that user) +rather than silently dropping them. This is a second entry point onto the same +T12.1 endpoints, not a third implementation of the bulk logic. + +**Done when:** + +- [ ] a CSV with a mix of valid and invalid usernames applies to the valid + rows and reports the invalid ones by row, with a reason +- [ ] the same permission/self-action guards from T12.1 apply here — a CSV + cannot reach a user a checkbox-driven batch couldn't +- [ ] duplicate usernames in one CSV are handled without double-applying or + erroring confusingly + +--- + +### T12.5 — CR-020: bulk delete confirmation + +- **Items:** `CR-020` +- **Depends on:** T12.3 +- **Blocks:** T12.6 +- **Surface:** `html/` +- **Files:** `html/users.js` + +**Do:** Wire delete into the toolbar with a heavier confirmation than the other +three actions, per the decision record's recommendation: list the affected +usernames and require typing a confirmation phrase (e.g. `DELETE`) before the +request is sent, regardless of how many rows are selected. This is flagged in +the decision record as a recommendation Matt has not explicitly signed off on +— if the PR reviewer wants a lighter or heavier mechanism, that's the moment to +change it, not a reason to skip building a real confirmation now. + +**Done when:** + +- [ ] the affected usernames are listed in the confirmation dialog before + delete is sent +- [ ] the request is not sent until the confirmation phrase is typed correctly +- [ ] the actor's own account, if somehow selected, is rejected with a clear + reason rather than silently included or silently dropped + +--- + +### T12.6 — CR-020: verification + +- **Items:** `CR-020` +- **Depends on:** T12.4, T12.5 +- **Blocks:** nothing +- **Surface:** `html/` + `server/` +- **Files:** as touched above + +**Do:** Full verification per `CLAUDE.md`: run locally, exercise bulk role +change, activate/disable, project assignment, CSV upload, and bulk delete at +390px and 1440px, before/after screenshots, smoke test, `seed_demo.py`, full +suite. + +**Done when:** + +- [ ] all `CR-020` acceptance criteria in `decisions-2026-09-17.md` are met or + a failure is stated with a reason +- [ ] screenshots committed +- [ ] smoke test and `seed_demo.py` both still pass +- [ ] full test suite passes + +--- + +## Wave 12 exit criteria + +- [ ] all four bulk actions work against both a checkbox selection and a CSV + upload +- [ ] every existing single-user guardrail (self-action, scope, grantable + roles) holds under bulk use +- [ ] bulk delete requires typed confirmation and lists affected usernames +- [ ] partial failures are always reported, never hidden behind a blanket + success +- [ ] `CR-020` fully accounted for, no open acceptance criteria diff --git a/docs/waves/wave-13.md b/docs/waves/wave-13.md new file mode 100644 index 0000000..47cb05e --- /dev/null +++ b/docs/waves/wave-13.md @@ -0,0 +1,200 @@ +# Wave 13 — Okta/AD deprovisioning sync + +**Items:** `D18` +**Depends on:** wave 10 merged (it is). Not blocked by wave 11 or wave 12, but +shares `server/app.py` account-state surface with wave 12 (`CR-020`) — sequence +commits to avoid an avoidable conflict. +**Decision record:** `docs/waves/decisions-2026-09-17.md` + +Six tasks, in build order. **T13.3's fail-closed behavior is the most +important done-when list in this wave — do not relax it to ship faster.** + +--- + +### T13.1 — D18: shrink `AUTH_SESSION_HOURS` + +- **Items:** `D18` +- **Depends on:** nothing (first task, independent of the rest) +- **Blocks:** nothing +- **Surface:** `server/` +- **Files:** `server/auth.py`, `server/.env.example`, `DEPLOYMENT.md` + +**Do:** Change the default from 12 to 2 hours. Document the change and the +reasoning (defense in depth against an already-live session outliving a +deprovisioning event) in `server/.env.example` next to `AUTH_SESSION_HOURS`, +and in `DEPLOYMENT.md`'s auth section. Note in the PR that this is proposed, +not confirmed against the tenant's actual Okta SSO session policy — flag it +for Nick/IT rather than treating 2 as unquestionable. + +**Done when:** + +- [ ] default is 2 hours; still overridable via env, unchanged mechanism +- [ ] `.env.example` and `DEPLOYMENT.md` explain why +- [ ] existing session/auth tests updated for the new default where they + assumed 12 + +--- + +### T13.2 — D18: Okta Management API client + +- **Items:** `D18` +- **Depends on:** nothing (independent of T13.1) +- **Blocks:** T13.3 +- **Surface:** `server/` +- **Files:** new `server/okta_sync.py` (or extend `server/okta_auth.py` — + task's call), `server/.env.example` + +**Do:** A small client for Okta's user-list endpoint, authenticated with a new, +separate credential (e.g. `OKTA_API_TOKEN`) — not the OIDC client secret used +for sign-in. Fetch the full user list (paginated per Okta's API) rather than +one-by-one lookups per local user; this app has ~20-odd accounts today, and a +list-and-diff is simpler and cheaper than N calls. Return each Okta user's +identity-claim value (matching `OKTA_IDENTITY_CLAIM`, already confirmed live +per wave 10) and status. + +Follow the existing pattern for external credentials in this repo (`MICRON_DB_URL`, +`SMTP_PASSWORD`): env-only, never logged, never returned to the browser in an +error message. + +**Do not:** reuse `OKTA_CLIENT_ID`/`OKTA_CLIENT_SECRET` for this. Sign-in and +the management API are different trust boundaries with different scopes; +conflating them means a compromise or rotation of one affects the other +unnecessarily. + +**Done when:** + +- [ ] the client authenticates with its own credential, distinct from the OIDC + client +- [ ] it fetches the complete Okta user list, handling pagination +- [ ] an auth failure or malformed response raises a clear, specific error + rather than returning an empty list indistinguishable from "everyone was + deprovisioned" — this distinction is what T13.3 depends on +- [ ] the credential is never logged or surfaced in any API response + +--- + +### T13.3 — D18: the sync job + +- **Items:** `D18` +- **Depends on:** T13.2 +- **Blocks:** T13.4 +- **Surface:** `server/` +- **Files:** `server/okta_sync.py`, `server/app.py` (or wherever `log_event` + lives) + +**Do:** Compare the Okta user list (T13.2) against local `users` rows. For any +local `is_active=True` user whose Okta identity is missing from the list, or +present with a non-active status, set `is_active=False` and write an +`AuditLog` row (`actor="system:okta_sync"`, action e.g. +`user_deprovisioned_by_sync`, detail naming the Okta status found). Never +touch a user already `is_active=False`. Never re-enable anyone. + +**The fail-closed rule, non-negotiable:** if T13.2's client raises an error of +any kind (network, auth, malformed response, timeout), this task takes **no +action on any account** for that run and logs the failure clearly (server log, +at minimum). An error is never treated as "Okta returned zero active users." +Write a test that asserts this directly: feed the sync a failing client and +assert zero rows changed and zero `AuditLog` entries written. + +**Done when:** + +- [ ] a user missing from Okta's list, or present but not active, is disabled + with a correctly-detailed audit row +- [ ] a user already disabled is left alone (no duplicate audit row each run) +- [ ] an active Okta user already active locally produces no audit row (only + changes are logged, not a clean bill of health every cycle) +- [ ] **a simulated Okta API failure results in zero account changes and zero + audit rows** — this is the one check that must never be skipped or + weakened +- [ ] the job never re-enables an account under any input + +--- + +### T13.4 — D18: run it on a schedule + +- **Items:** `D18` +- **Depends on:** T13.3 +- **Blocks:** T13.6 +- **Surface:** `server/`, `docker-compose.yml` +- **Files:** `server/app.py` (startup hook) or a new sidecar per the `backup` + container's pattern — task's call, per the decision record's noted + alternative + +**Do:** Wire T13.3 to run on an interval (proposed 15 minutes, env-overridable +— e.g. `OKTA_SYNC_INTERVAL_SECONDS`, matching `BACKUP_INTERVAL_SECONDS`'s +naming). Default choice is an in-process background task in the `api` +container; if a separate container is chosen instead, follow the `backup` +service's shape (own Dockerfile or script, `internal` network plus whatever +egress reaching Okta requires — check whether `outbound` as currently defined +is sufficient or Okta needs a distinct allowance). + +**Done when:** + +- [ ] the job runs automatically on the configured interval without manual + invocation +- [ ] interval is env-configurable with a sane default +- [ ] a container restart does not produce a duplicate/overlapping run, and a + slow cycle does not stack with the next one + +--- + +### T13.5 — D18: admin visibility + +- **Items:** `D18` +- **Depends on:** T13.3 +- **Blocks:** T13.6 +- **Surface:** `html/` +- **Files:** `html/users.js` / `html/admin.js` (wherever audit history is + already surfaced) + +**Do:** Confirm an auto-disable reads clearly wherever admins already look at +account history — the actor string (`system:okta_sync`) should be +self-explanatory in context, not require reading server logs to understand. +Do not build new UI beyond making sure the existing audit surface renders this +actor sensibly. Email/notification to admins on auto-disable is a noted +fast-follow (the decision record flags it as open, not required here) — do not +build it in this task; log it instead if it's tempting to add. + +**Done when:** + +- [ ] an auto-disabled account's audit entry is visible and legible in the + existing admin UI without special-casing +- [ ] nothing here silently assumes `CR-019`'s activity view exists yet — this + must work standalone + +--- + +### T13.6 — D18: verification + +- **Items:** `D18` +- **Depends on:** T13.4, T13.5 +- **Blocks:** nothing +- **Surface:** `server/` +- **Files:** as touched above + +**Do:** Full verification per `CLAUDE.md`. Beyond the usual run: specifically +re-run T13.3's fail-closed test in isolation and confirm it still passes after +T13.4's scheduling wrapper is in place — the scheduling layer must not +introduce a path that swallows the client's error and proceeds anyway. + +**Done when:** + +- [ ] all `D18` acceptance criteria in `decisions-2026-09-17.md` are met or a + failure is stated with a reason +- [ ] the fail-closed behavior is verified end to end through the scheduled + wrapper, not just the bare sync function +- [ ] smoke test and `seed_demo.py` both still pass +- [ ] full test suite passes + +--- + +## Wave 13 exit criteria + +- [ ] `AUTH_SESSION_HOURS` defaults to 2, documented +- [ ] the sync job runs on a schedule and correctly disables accounts Okta no + longer shows as active +- [ ] every auto-disable is individually audited with a clearly non-human actor +- [ ] an Okta API failure of any kind changes zero accounts — verified, not + assumed +- [ ] the sync never re-enables an account +- [ ] `D18` fully accounted for, no open acceptance criteria