Project-SDE-WP-Suite/docs/waves/decisions-2026-09-17.md

# 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 `<select>` per row, an activate/disable button per row, a
per-user project-membership checklist opened one user at a time, and a
per-row delete. There is no row selection in the User Directory table at all.
Server-side, every corresponding endpoint
(`/api/auth/users/{id}/active`, `/role`, `/project-role`, `/projects`, and
`DELETE /api/auth/users/{id}`) takes exactly one `user_id`. Bulk editing is new
UI (selection) and, for most actions, either a loop over the existing
single-user endpoints or new endpoints that accept a list — the task decides
which, per user count and transaction-safety needs.

**Deletion is already a hard delete today** (`delete_user`, `server/app.py:1121`
— `db.delete(u)`, not a deactivate). Bulk delete inherits that: it is not this
item's job to invent a soft-delete pattern that does not exist for the single
case, but the confirmation step around it has to be sized for the fact that a
bad multi-select now removes more than one account at once, permanently.

### Decisions, made 2026-09-17

1. **All four bulk actions are in scope:** role change, activate/disable,
   project assignment (add to / remove from a project, including the
   project-role), and delete.
2. **Selection works two ways:** checkboxes (select-all and individual) in the
   existing User Directory table, respecting whatever filter is already
   applied (role, active/disabled, project) — and a CSV upload, for a one-off
   bulk operation against an external list (e.g., an offboarding list that
   didn't originate in this app). Both are in scope, not a choice between them.

### Acceptance criteria

- The User Directory table gains row checkboxes and a select-all that respects
  the current filter; a bulk-action toolbar appears once at least one row is
  selected.
- CSV upload as an alternative to checkbox selection: a list of usernames plus
  the action to apply. Validates every row, reports rejected ones by row (bad
  username, user not found, actor lacks permission over that user) rather than
  silently skipping them — the same validate-and-report pattern `CR-005`
  established for list uploads.
- Every existing single-user guardrail carries forward unchanged: an actor
  cannot include their own account in a bulk action that would disable, demote,
  or delete it; a super user's bulk action is scoped to only the users and
  projects `require_see_user`/`require_manage_user` already let them touch
  today (a super user cannot use a bulk action to reach a user or project
  outside what they manage, even via CSV); `grantable_roles` still gates which
  roles an actor may assign in bulk, the same as one at a time.
- Every affected row is written to `AuditLog` individually, exactly as the
  single-user endpoints do today (one `role_changed` / `user_deleted` / etc.
  row per user) — a bulk action is many audited changes, not one opaque batch
  entry, so per-user history stays intact and readable in isolation.
- Confirmation before applying, using the `wp-dialog` kit (`T7.9`), not a
  native `confirm()`. The dialog names exactly how many users are affected and,
  for delete specifically, lists the affected usernames before committing.
- Partial failure is reported, not hidden: if some rows in a batch fail (scope,
  already-deleted, bad CSV row), the action applies to what it can and states
  exactly which rows failed and why. It never reports success on a batch that
  partly failed.
- Accessible per `C1`: real controls, keyboard-operable selection and bulk-
  action toolbar, `aria-live` announcing the result.

### Recommended, not yet confirmed — raise in the PR if this is wrong

- **Bulk delete gets an extra confirmation step beyond naming the count** —
  proposed as typing a confirmation phrase (e.g. the word `DELETE`) regardless
  of how many rows are selected, since one bad multi-select now removes more
  than one account, permanently, with no soft-delete to fall back on. This is
  a recommendation, not a confirmed requirement — Matt has not signed off on
  the exact mechanism.
- **Project assignment is add/remove, not replace-the-whole-list** — a bulk
  "add these users to project X" or "remove these users from project X"
  action, rather than a bulk action that overwrites a user's entire project
  list. Proposed because add/remove is less likely to clobber project
  memberships the actor didn't intend to touch; a replace-the-whole-list
  version is a materially different (and riskier) feature if that turns out to
  be what's actually wanted.

### Frontend/backend boundary

Selection state (which rows are checked) is fine as client-side UI state — it
is not persisted data. Everything the bulk action actually does (role,
active/disabled, project membership, delete) already requires server work
today for the single-user case, and bulk does not change that: no new
localStorage-derived state, no client-side aggregation of what changed.

### Scheduling

New wave. Independent of wave 11 (`CR-019`) — the two do not touch the same
code and can build in either order or in parallel. Task breakdown:
`docs/waves/wave-12.md`.

---

## D18 — Detecting and acting on Okta/AD deprovisioning

- **Raised by:** Matt Mabrey, 2026-09-17, in response to a direct question about
  what happens when someone's AD account is removed and Okta subsequently drops
  them.
- **Amends:** nothing decided, closes a gap `D15`/`D16` left unaddressed. Those
  items designed how identity flows *into* this app (Okta authenticates, this
  app JIT-provisions and owns roles); neither addressed what happens when an
  identity is withdrawn. This app has never had any deprovisioning signal —
  push or pull — since Okta went live.

### What was found

Traced through `server/auth.py` and `server/okta_auth.py`: this app has no
connection back to Okta after initial sign-in. Consequences, confirmed against
the actual code:

1. A local `users` row is never touched by anything Okta-side. `is_active`
   stays `True` indefinitely unless an admin manually disables or deletes the
   account through the User Directory. There is no flag distinguishing a
   terminated employee's account from a current one.
2. `get_current_user` (`server/auth.py:237`) re-checks `is_active` and
   `token_version` on **every request** — so a manual disable takes effect
   immediately, on the very next request. The gap is not enforcement, it's
   detection: nothing tells an admin to go flip that switch.
3. A session already live when someone is deprovisioned keeps working, fully,
   for up to `AUTH_SESSION_HOURS` (12 hours today, `server/auth.py:63`),
   because validity is checked against this app's own JWT and DB state only,
   never against Okta.
4. A NEW session can't be established once Okta drops the account —
   `okta_login`/`okta_callback` requires completing Okta's own sign-in, which
   Okta itself refuses. The front door closes on its own; the back door (an
   already-open session, and the stale local record) does not.

### Decisions, made 2026-09-17

1. **Build a scheduled sync against Okta's Management API**, not an Event Hook.
   This app has, since `D15`, only ever reached out to Okta — never received
   anything from it — and a polling design keeps that shape rather than
   introducing a new inbound, internet-reachable endpoint with its own
   signature-verification surface. Traded deliberately: this is
   poll-interval-late rather than real-time, which is judged acceptable for an
   HR/offboarding-driven event, not a to-the-second requirement.
2. **Revised 2026-09-23, in response to Matt's question about idle time
   instead of a flat session length: sessions now slide on activity, with a
   hard ceiling underneath.** A flat `AUTH_SESSION_HOURS` forces a re-check
   with Okta on a fixed schedule regardless of activity; a pure idle timer
   with no ceiling does the opposite — a continuously-active session would
   never force a fresh Okta check on its own, which is a worse fit for the
   exact threat this item exists to address (someone still clicking around
   after being deprovisioned). Decided: **both**.
   - `AUTH_IDLE_MINUTES` (new, default **30**): a session with no request
     for this long stops being valid. Implemented as a sliding JWT expiry —
     the token is reissued with a fresh `exp` on activity, throttled so the
     cookie isn't rewritten on literally every request.
   - `AUTH_SESSION_HOURS` (existing var, meaning changes to an **absolute
     ceiling**): no session survives past this many hours from the original
     sign-in, no matter how continuously active it is. Default changing from
     12 to a proposed **8** — flagged as a recommendation, not confirmed.
   - Both defaults, and the mechanism itself, should be sanity-checked
     against the tenant's actual Okta SSO session policy — if Okta's own
     session silently outlives either number, re-authentication here is
     likely a fast redirect, not a real re-login screen, so these numbers
     cost less than they look like they do. Confirm before treating either
     as final.
3. **The sync only ever disables an account — it never re-enables one.** A
   rehire showing active in Okta again does not automatically restore access;
   an admin re-enabling the account is a deliberate act, consistent with
   `D16`'s posture that this app never auto-grants access on its own initiative.
4. **Fail closed on the side of INACTION, not disablement.** This is the
   opposite failure direction from `D16`'s login-time posture ("if Okta is
   unreachable, the app is unreachable for everyone"). Here, an Okta API
   error, timeout, empty response, or anything the sync can't confidently
   parse must result in **no change to any account** that cycle, plus a
   logged failure. A sync job that treats "couldn't reach Okta" as "nobody is
   active" is a far worse outcome than a missed cycle — it would lock out the
   entire org on an Okta API hiccup. This is the single most important
   acceptance criterion in this item.
5. **Every auto-disable is audited individually**, same as every other
   account-state change in this app: an `AuditLog` row per user, with an actor
   value that's clearly the sync job and not a person (e.g.
   `system:okta_sync`), so it reads correctly in the User Directory's history
   and is never confused with an admin's own action.
6. **New credential required:** a read-scoped Okta API token (or an Okta
   service-app OAuth2 client), separate from the `OKTA_CLIENT_ID`/
   `OKTA_CLIENT_SECRET` pair used for sign-in. This needs provisioning by
   whoever administers the Okta tenant — the same dependency that gated the
   original OIDC rollout (D15's "security scoping reply").

### Recommended, not yet confirmed — raise in the PR if this is wrong

- **Sync interval:** proposed every 15 minutes. Frequent enough that the
  detection gap is small, infrequent enough not to hammer Okta's API or need
  special rate-limit handling. Not confirmed with IT/security.
- **Where the job runs:** proposed as an in-process background task inside the
  existing `api` container (it already has `outbound` network access to reach
  Okta, and already holds the Okta client config) rather than a new sidecar
  container. The `backup` container (`docker-compose.yml`) is the existing
  precedent for a scheduled-interval container in this stack, if isolation
  from the `api` process is preferred instead — a reasonable alternative, not
  the recommendation.
- **Admin visibility:** at minimum, an auto-disable is a normal, readable
  `AuditLog` entry (visible whever admin already reviews audit history, and
  naturally covered once `CR-019`'s activity view exists). Whether it should
  also trigger an email/notification to admins is a genuine open question —
  proposed as a fast-follow rather than blocking this item, since `D10`
  already established the pattern for admin-controlled email toggles this
  could reuse.

### Frontend/backend boundary

Entirely server-side and infrastructure. No new `localStorage` state, no
frontend surface beyond what already reads `is_active` and `AuditLog` today
(the User Directory, and eventually `CR-019`'s activity view).

### Scheduling

New wave, independent of wave 11 and wave 12 in the sense that nothing here is
blocked by them — but note it touches the same `is_active`/account-state
surface `CR-020`'s bulk actions touch in `server/app.py`. Not a hard
dependency; sequence commits to avoid an avoidable merge conflict, per
`CLAUDE.md`'s "one task per PR" spirit. Task breakdown: `docs/waves/wave-13.md`.