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.
This commit is contained in:
327
docs/waves/decisions-2026-09-17.md
Normal file
327
docs/waves/decisions-2026-09-17.md
Normal file
@@ -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 `<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. **Also shrink `AUTH_SESSION_HOURS`** as defense in depth, independent of the
|
||||
sync job, so the already-live-session window itself is smaller regardless
|
||||
of how fast the sync runs. Proposed default: **2 hours**, down from 12 —
|
||||
flagged as a recommendation, not confirmed: Okta's own SSO session (set on
|
||||
the Okta side, separate from this app's cookie) may make re-authentication
|
||||
silent rather than a real re-login if it outlives this app's shorter
|
||||
session, in which case 2 hours costs little. Confirm against the tenant's
|
||||
actual Okta session policy before treating 2 as final; if Okta's SSO
|
||||
session is itself long-lived, a shorter number here doesn't hurt, either.
|
||||
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`.
|
||||
Reference in New Issue
Block a user