Files
Project-SDE-WP-Suite/docs/waves/wave-10.md
Matt Mabrey df20b8f18d wave-10: close out claim mapping and redirect URI, live in production
Confirmed by an actual live Okta sign-in after main (cc64c88) deployed: preferred_username is the right identity claim, and the redirect URI works. Matt matched his existing pre-Okta admin account rather than getting JIT-provisioned as a duplicate. Two deploy-time snags recorded, both Case B (config, not data): OKTA_CLIENT_ID/SECRET/ISSUER left empty in Portainer at first (caught cleanly by is_configured()), then OKTA_ISSUER missing its https:// scheme (surfaced as httpx.UnsupportedProtocol, not a deliberate app error - BL-028 still stands). Neither needed the backup. BTG pilot group now includes Cody and Cameron, awaiting Adrian.
2026-09-09 13:34:32 -07:00

234 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Wave 10 — Okta OIDC authentication
Fresh wave 10. The label was previously used by the LDAPS work under `D13`/`D14`, built on
`feat/ldaps-directory-auth`; that branch was deleted rather than merged and never appeared
in `IMPLEMENTATION.md`'s wave table, so it carries no claim on the number. See
`docs/waves/decisions-2026-09-02.md` (`D15`) for why LDAPS was retired before deployment
and Okta chosen instead.
Depends only on `main` as it stands after `D15`. Not sequenced behind any other wave.
## Tasks
- **T10.1 — Add the Okta OIDC client.** Authlib as a dependency. Config via env vars
(`OKTA_ISSUER`, `OKTA_CLIENT_ID`, `OKTA_CLIENT_SECRET`, `OKTA_REDIRECT_URI`), same
pattern `AUTH_SECRET_KEY` already uses in `server/auth.py`.
- **T10.2 — Login-redirect and callback routes.** A route that sends the browser to
Okta's authorize endpoint, and a callback route that exchanges the code for tokens and
validates the ID token. Access gating is Okta's job, not this app's: only accounts
assigned to the app integration in Okta can reach it at all, so there is no app-side
required-group or claim check layered on top. This is a deliberate difference from D13,
which had to gate on a required AD group itself because an LDAPS bind alone could not
distinguish an assigned user from any other domain account.
- **T10.3 — Identity matching and JIT provisioning.** Reuses D13's shape
(`_provision_from_directory`-style matching) keyed off an OIDC claim instead of an LDAP
search result. **Open dependency:** which claim carries the AD `sAMAccountName`
equivalent (`preferred_username`, `upn`, or a custom claim) is asked of security and not
yet answered. Build with a configurable claim name and a documented default, not a
hardcoded one, so the answer can drop in without a code change.
- **T10.4 — Remove the local password path entirely.** Drop `password_hash` (Alembic
migration; plain `op.drop_column`, matching existing precedent for other NOT NULL
columns on `users` — no `batch_alter_table` needed), remove the bcrypt-based
`login()`, remove the username/password form. Real deletion, matching `D15`'s "full
replacement," not a toggle or a fallback. Scope corrected by `D16` after hazard
review turned up more call sites than the original bullet named:
- `create_user()` and `admin_reset_password()` in `server/app.py` (admin console's
"add user" and "reset password" routes) — rework to drop the password field
entirely rather than break.
- `html/users.js`'s "add user" form (`nu-password`) and "Reset password" button —
matching frontend change.
- `server/manage_users.py` — reworked per `D16` from account *creation* to
*promotion*: `create-admin`/`create`/`reset-password` (password-based) are
replaced by a promote-by-username command that operates on a row Okta's JIT
provisioning (T10.3) already created, never a hand-typed new one. This is now
the documented admin-bootstrap path — see `D16`.
- `tests/browser_check.py` and `tests/launcher_check.py` — stop calling
`auth.hash_password()` to seed fixture rows.
- `server/smoketest.py` and `server/seed_demo.py` — currently authenticate via
`POST /api/auth/login`. Switch to minting a session with `auth.create_token()`
directly, the same technique `browser_check.py` already uses, so both scripts
(named explicitly in `CLAUDE.md`'s verification section) keep working without
depending on `T10.7`'s timing.
- **T10.5 — Frontend: login becomes a redirect, not a form.** `login.html`/`login.js`
change to a "Sign in with Okta" flow. Sign-out lands back on the app's own login page.
- **T10.6 — Deployment docs and env var reference.** `DEPLOYMENT.md`,
`server/.env.example`, `server/README.md` describe the Okta config in place of the LDAP
config they never ended up describing (D13 never shipped, so these still describe the
original local-password system today).
Built: all three rewritten — the five `OKTA_*` vars documented the same way
`AUTH_SECRET_KEY` already was, the login-portal/self-service-reset sections replaced
with the Okta flow and the promote-not-create admin bootstrap (D16), the smoke-test
walkthrough updated for `WP_SMOKE_USER`-only / must-share-`AUTH_SECRET_KEY`-and-DB
(T10.4's `smoketest.py` rewrite). Also `docker-compose.yml`, not originally named in
this bullet: its `api` service sets `environment:` as an explicit allowlist, not
`env_file`, so the documented vars would silently never reach the container without
adding them there too — found and fixed in the same commit rather than shipping docs
for a config path that doesn't work. `OKTA_IDENTITY_CLAIM` mirrors
`okta_auth.py`'s own default (`preferred_username`) in the compose file rather than
defaulting to an empty string, which would 503 every sign-in.
Logged, not fixed here (out of scope): `users.failed_attempts`/`locked_until` are
vestigial (still reset on every Okta sign-in, nothing increments them since local
`login()` is gone); `server/README.md`'s own "Production — Docker Compose" section
is a self-contained alternate quickstart that already diverged from the real root
`docker-compose.yml` before this task and still does.
- **T10.7 — Test coverage without a live Okta dependency.** A fake-OIDC-provider test
seam, mirroring `ldap_fake.py`, so the suite runs with no live Okta tenant reachable.
Built: `server/okta_fake.py` (env-driven, `WP_OKTA_FAKE_DIRECTORY`, production-refusing
the same way `ldap_fake.py` does), dispatched from `okta_auth._build_oauth()` before
the real Authlib client is considered. Only the two Authlib calls that touch the
network — `authorize_redirect` / `authorize_access_token` — are faked; `app.py`'s
`okta_login()`/`okta_callback()` (the `?next=` guard, the disabled-account check, JIT
provisioning, the identity-claim lookup) run unmodified against the fake, same
boundary the LDAP predecessor drew around the anonymous-bind guard. Two fake-only
routes (`/_fake_provider`, `/_fake_provider/consent`) stand in for Okta's own sign-in
screen and are registered in `app.py` only when the fake is active at import time —
in production they do not exist, not merely refuse. `tests/browser_check.py`'s
`start_server()` now takes an optional `extra_env` and sets
`WP_OKTA_FAKE_DIRECTORY` unconditionally (same reasoning `ldap_fake`'s equivalent
used: almost nothing signs in, but the one check that does should not fail
mysteriously). `tests/url_state_check.py` scenario 2 is un-skipped and drives the
real round trip — login.html's button, the fake picker page, the fake consent
redirect, `okta_callback()` — proving `?next=` survives it, same tightened
"actually left login.html" assertion the LDAP predecessor's own bug fix used.
`tests/okta_auth_check.py` is new: the production guard, single-use/replay on the
authorization code, an unsolicited callback hit, a tampered state, a denied consent,
an unknown identity, a disabled account, JIT provisioning, an existing admin
surviving unchanged, same-site vs. off-site `?next=`, and `OKTA_IDENTITY_CLAIM`
genuinely working under a non-default claim name — 22/22.
The dev sandbox this was built in has no headless browser and no way to install
one, so `tests/url_state_check.py` and `tests/browser_check.py` (both need
`tests/cdp.py`'s real headless Chromium) could not be run there — only
`okta_auth_check.py`'s HTTP-level coverage of the same mechanism. Run for real
afterward on a machine with Docker, via a separate general-purpose tool
(`headless-py-test-runner`, kept out of this repo — it is not Work Package Suite
specific): `url_state_check.py` 26/26, including scenario 2's real click-through of
the fake-Okta round trip, and `browser_check.py` 71/71. Gap closed.
**Validated 2026-09-03:** `tests/okta_auth_check.py` 22/22 (no browser needed) ·
`tests/url_state_check.py` 26/26 · `tests/browser_check.py` 71/71 — all three run
clean, the last two against real headless Chromium via Docker.
- **T10.8 — Verification.** 390px and 1440px, full suite, done-when checks per task,
matching the rigor D13 was held to.
Done-when checks per task, verified against the actual code rather than
re-reading this file's own claims: T10.1 (Authlib pinned, the four env vars,
`is_configured()`/`describe()`), T10.2 (the login/callback routes, no
app-side group or claim gate layered on Okta's own), T10.3 (identity-claim
matching, JIT at the lowest role, local deprovisioning still enforced after
Okta approves), T10.4 (zero remaining references to `bcrypt` /
`password_hash` / `hash_password` / `verify_password` anywhere in `.py` or
`.js`, `manage_users.py promote`, `smoketest.py` / `seed_demo.py` /
`browser_check.py` / `launcher_check.py` all minting via `create_token()`),
T10.5 (`login.html` is one Okta link, no password field). All matched what
this file already claimed — no drift found.
Full suite, run through the Docker test runner (all 41 files in `tests/`,
bare invocation): 39 passed clean. `tests/token_check.py` "failed" at exit 2,
but that is a harness mismatch, not a check failure — it is a two-step
snapshot/diff tool (`--out` to capture, `--compare A B` to diff) and prints
usage + exits 2 when run with no arguments, which is what a bare full-suite
pass does to every file. `tests/generalinfo_check.py` scored 48/49 — the one
failure is a raw `rgba()` shadow literal in `wp-creation-styles.css`,
confirmed via `git show HEAD` to already be committed and unrelated to this
wave (it is the Micron asset picker's dropdown shadow from the `D11` merge,
2026-08-20, predating this wave by two weeks). Logged as `BL-031` rather than
fixed here — an unrelated CSS token-rule violation is not this wave's to fix.
`tests/okta_auth_check.py` re-run fresh (no browser needed): 22/22.
390px and 1440px: `tests/baseline_shots.py` captured all fourteen shots
(login, launcher, sop, creator, admin, users, field × two widths) into
`docs/reference/baseline/`. `login-390.png`/`login-1440.png` and
`users-390.png`/`users-1440.png` visually confirmed: the login page is a
single "Sign in with Okta" button with no username/password form at either
width, and the User Directory's table and "Add a user" form both carry no
password column and no reset-password action anywhere, at either width.
Wave 10 is complete. The three items in "Still open" below are external
(security team / Okta admin), not blocked on any task in this wave.
- **T10.10 — Audit `manage_users.py promote`.** Raised in review after T10.8:
`cmd_promote()` changed a user's role with no audit trail at all, unlike the
identical role change from the web Admin Console (`app.py`'s
`set_user_role()``log_event()`, action `"role_changed"`). Not a new
privilege — anyone with Portainer/container-exec access to `wp_api` already
has shell access to the database directly, same trust tier D16 already named
for this command — but there was no record of who ran it or what changed.
Built: `cmd_promote()` now writes an `AuditLog` row with the same
`action`/`detail` shape `set_user_role()` uses (`{"from": old_role, "to":
role}`), tagged `"via": "cli"` (mirrors JIT provisioning's own `"via":
"okta_jit"` tag) and `actor="cli:manage_users"` — a container shell exec
carries no signed-in identity to attribute the change to a real person, so
it names the tool rather than guessing one. Verified end to end against a
scratch SQLite database: the audit row lands with the exact expected shape,
the role change persists, and the existing "no such user" refusal still
exits 1 with no partial write.
- **T10.9 — Rollback-aware deploy runbook.** Raised after hazard review found
`DEPLOY-runbook-2026-08-04.md`'s Rollback section has no case for a migration whose
`downgrade()` cannot restore the data it drops — see `D17`. `T10.4`'s
`1d60a608bb51_drop_local_password` is exactly that: the schema comes back, the
bcrypt hashes do not.
Built: `DEPLOY-runbook-2026-09-03.md`, following the 2026-08-04 runbook's structure
(fill-in table, numbered deploy steps, case-by-case Rollback section, Notes). Names
the five `OKTA_*` vars as newly required (the precedent's "no new environment
variables" note does not carry over), treats the pre-deploy backup as the only way
back once the migration commits, and splits Rollback into the fixable case (Okta app
integration misconfigured — fix and redeploy `api`, no data at risk, migration stays
applied) versus the severe case (abandoning Okta for local-password code — only the
destructive backup restore gets there, reusing the 2026-08-04 runbook's own Case C
procedure). D16's no-break-glass posture is stated plainly rather than left implicit.
Logged, not fixed here (out of scope): `okta_auth.describe()`'s startup log line
(referenced by `DEPLOYMENT.md`/`server/README.md`) has no caller anywhere in
`server/app.py` — nothing actually prints it at process start. The runbook's Step 4
therefore verifies via a live Okta sign-in rather than a log line, and this gap is
flagged in `docs/waves/backlog.md` as a candidate fix (wiring `describe()` into
startup) since it directly bears on deploy verifiability. `DEPLOY-login-portal.md`
is now fully stale (bcrypt, `create-admin --password`, none of which still exist) —
not touched, no task claims it.
## Still open
- The `Business Technology Group` pilot assignment in Okta. Originally six names
(Carlee Swihart, Drew Hilliard, Matt Mabrey, Nick Siegfried, Rachel Schreiber, Terry
Sajan); Cody and Cameron added 2026-09-09. Adrian added only Matt at first,
deliberately, pending the live sign-in confirmation below — awaiting his response to
add the rest of the group now that it has.
Closed since first written: admin bootstrap and break-glass posture, previously open
questions, decided in `D16` (2026-09-03) and folded into `T10.4` above.
**Closed 2026-09-09, live in production:** the redirect/callback URI
(`https://wp.controls.dev/api/auth/okta/callback`) is confirmed working, and so is the
OIDC claim mapping (`T10.3`) — `preferred_username` (the code's documented default,
never actually confirmed by name in Request 50649's thread) is correct, no
`OKTA_IDENTITY_CLAIM` override needed. Both settled by an actual live sign-in against
the real Okta tenant after `main` was merged (`cc64c88`) and deployed: Matt signed in
as himself, matched his existing pre-Okta admin account by `find_user()` rather than
JIT-provisioning a duplicate (the account already existed — this app has ~40 real
users, not the seeded test fixture), landed on `index.html` signed in, admin role and
project access untouched. One real deploy-time snag on the way, worth recording since
it's exactly the Case B scenario `DEPLOY-runbook-2026-09-03.md` anticipated: the first
redeploy left `OKTA_CLIENT_ID`/`OKTA_CLIENT_SECRET`/`OKTA_ISSUER` as empty rows in
Portainer (env var names added, values never filled in) — caught via
`is_configured()`'s all-four-required check failing closed (the 503 "Sign-in is
temporarily unavailable"), not silently. A second snag after filling those in:
`OKTA_ISSUER` was pasted without its `https://` scheme, which surfaced as
`httpx.UnsupportedProtocol` from Authlib's OIDC discovery fetch rather than anything
the app's own code raises deliberately — the exact case the runbook's Notes flagged as
having no startup-time confirmation (`okta_auth.describe()` still has no caller,
`BL-028`). Both fixed by correcting the env var values in Portainer and redeploying;
neither needed the backup or the database.