Compare commits
10 Commits
feat/wp-su
...
31c548318b
| Author | SHA1 | Date | |
|---|---|---|---|
| 31c548318b | |||
| 8f117680b0 | |||
| 6034c08bad | |||
| 17cabbd032 | |||
| 222c0b1c29 | |||
| e31234beef | |||
| 6057d05b98 | |||
| 64eac0cbbb | |||
| 8f280d4bd1 | |||
| a8e28bf3ab |
@@ -292,7 +292,7 @@ Wave 8 adds these:
|
||||
```bash
|
||||
python tests/kitting_check.py # CR-009/010/012 - statuses, owner, delivery 26 checks
|
||||
python tests/kitting_notify_check.py # CR-011 - kitting mail, coalesced, gated 17 checks
|
||||
python tests/materials_check.py # D6 - material list, the CR-005 pattern 17 checks
|
||||
python tests/materials_check.py # D6 - material list, the CR-005 pattern 20 checks
|
||||
python tests/mreq_check.py # CR-013 - lightweight request, end to end 19 checks
|
||||
```
|
||||
|
||||
|
||||
@@ -554,3 +554,43 @@ deliberately deferred.
|
||||
entry rather than a drive-by.
|
||||
- **Suggested wave or follow-up:** next housekeeping pass, with the check
|
||||
widened so it cannot recur.
|
||||
|
||||
### BL-026 — No version stamp: "is live current?" cannot be answered from the app
|
||||
|
||||
- **Found during:** the 2026-08-21 outage triage (the question that started it)
|
||||
- **Where:** `Dockerfile` / build, `server/app.py` `/api/health`, admin console
|
||||
- **What:** the app carries no record of what code it is running. `/api/health`
|
||||
returns `{"ok": true}` and nothing identifies the deployed commit, so
|
||||
answering "is the live site on the latest code?" took fingerprinting
|
||||
(probing for files/routes that only exist after certain merges) in the
|
||||
middle of an outage. The fix: bake the git SHA into the image at build time
|
||||
(`ARG GIT_SHA`), return it from `/api/health`
|
||||
(`{"ok": true, "version": "<sha>"}`), and show it on the admin console's
|
||||
diagnostics card. Then currency is one glance against `git log -1`.
|
||||
- **Why not now:** new scope — needs its own item id per the working rules
|
||||
(D13 is the natural next), and it touches the image build, which deserves a
|
||||
deploy alongside someone with host access.
|
||||
- **Suggested wave or follow-up:** next housekeeping pass; ~1 task including a
|
||||
probe check that /api/health carries a version field.
|
||||
|
||||
### BL-027 — Migrations are rehearsed on SQLite only; production is Postgres
|
||||
|
||||
- **Found during:** the 2026-08-21 production outage (D6's `material_items`
|
||||
migration crash-looped the api container)
|
||||
- **Where:** `DEPLOYMENT.md` (the update/deploy steps), `tests/`
|
||||
- **What:** the migration chain is verified end-to-end on scratch SQLite, but
|
||||
production runs Postgres, and the dialects disagree exactly where it hurts:
|
||||
`server_default=sa.text('1')` on a Boolean passed every SQLite rehearsal and
|
||||
was refused by Postgres at deploy (DatatypeMismatch), taking the API down
|
||||
until the table was created by hand. The hotfix (64eac0c) fixed that one
|
||||
instance and pinned the Boolean-default class in `materials_check`; the
|
||||
CLASS of dialect drift is still unguarded. Two cheap layers: (1) a runbook
|
||||
step — render `alembic upgrade --sql` for the postgresql dialect and read it
|
||||
before restarting (offline, needs no live DB; this render would have shown
|
||||
`DEFAULT 1` on a boolean); (2) better, a probe that renders every migration
|
||||
for the postgresql dialect on each run and fails on anything the dialect
|
||||
rejects or on known-bad patterns.
|
||||
- **Why not now:** the outage is resolved and the one known instance is fixed
|
||||
and pinned; the systematic guard is its own small task, not a hotfix rider.
|
||||
- **Suggested wave or follow-up:** next housekeeping pass, paired with BL-026
|
||||
(both are "deploys should be boring" work).
|
||||
|
||||
75
docs/waves/decisions-2026-09-02.md
Normal file
75
docs/waves/decisions-2026-09-02.md
Normal file
@@ -0,0 +1,75 @@
|
||||
# Decisions — September 2, 2026
|
||||
|
||||
One item, and it retires two decided-and-built items rather than amending them.
|
||||
|
||||
---
|
||||
|
||||
## D15 — Authentication moves to Okta OIDC; D13 and D14 are retired before deployment
|
||||
|
||||
- **Amends:** retires `D13` (LDAPS simple bind against `prime.local`) and `D14` (the CLI
|
||||
authenticates against the domain). Both were decided and reaffirmed August 21 2026,
|
||||
built across nine tasks (`T10.1`–`T10.9`), and verified against the live domain. Neither
|
||||
reached production. Approved by Nick Siegfried.
|
||||
- **Surface:** `server/auth.py`, `server/app.py` (`login()`), `html/login.html`,
|
||||
`html/login.js`, `html/auth-guard.js`. `server/ldap_auth.py` does not carry forward —
|
||||
there is no LDAPS bind in the new design, not even as a fallback.
|
||||
- **Wave:** 10. The label is reused fresh: the LDAPS work that previously answered to
|
||||
"wave 10" was built on `feat/ldaps-directory-auth`, which is deleted rather than merged,
|
||||
and never appeared in `IMPLEMENTATION.md`'s wave table. It carries no claim on the
|
||||
number.
|
||||
|
||||
### What D13/D14 were, for the record
|
||||
|
||||
The branch carrying them is deleted, not merged, so their decision record
|
||||
(`docs/waves/decisions-2026-08-21.md`) no longer exists on any branch. Preserved here so
|
||||
the reasoning isn't lost along with it:
|
||||
|
||||
D13 chose a direct LDAPS simple bind to `ldaps://prime.local:636` as the sign-in
|
||||
mechanism: no password stored, a successful bind was the authentication, accounts were
|
||||
provisioned just-in-time from the directory, and roles stayed local. D14 moved the CLI
|
||||
onto the same bind, removing `create-admin`/`create`. Both were built, tested
|
||||
(1284/1288 checks, Aug 24), and screenshotted at 390px and 1440px. Neither ever deployed —
|
||||
`wp.controls.dev` still runs the pre-D13 local-password login as of this decision.
|
||||
|
||||
### The decision
|
||||
|
||||
Skip LDAPS entirely. Authentication becomes an Okta OIDC authorization-code flow,
|
||||
replacing local passwords directly — the same full replacement D13 intended, just via
|
||||
Okta instead of a domain bind. No LDAPS bind exists in this design at any point.
|
||||
|
||||
Four things carry forward from D13 unchanged, because they were never LDAPS-specific to
|
||||
begin with:
|
||||
|
||||
1. **No password is stored.** The app never sees a credential of any kind; Okta owns
|
||||
authentication entirely.
|
||||
2. **Accounts are provisioned just-in-time.** A first successful Okta sign-in with no
|
||||
matching local `users` row creates one, at the default role. The matching logic that
|
||||
was going to key off a directory search instead keys off an OIDC identity claim.
|
||||
3. **Roles stay local.** Okta, and AD behind it, supplies identity only. This app decides
|
||||
what an identity may do. Restated because it is the one rule the whole access-control
|
||||
design depends on — see `BL-029` and the governance discussion that followed it.
|
||||
4. **Existing accounts keep their roles** on first Okta login, exactly as D13's criterion
|
||||
4 read for LDAPS.
|
||||
|
||||
### Why this, and not LDAPS first and Okta second
|
||||
|
||||
`BL-029` (recorded on this branch as `BL-027` before the renumbering forced by main's
|
||||
independent use of that number) already laid out why OIDC beats the LDAPS bind on three
|
||||
counts: this app never sees a password, MFA comes from Okta rather than needing to be
|
||||
built, and the domain-lockout hazard that forced `AUTH_MAX_ATTEMPTS` down to 2 disappears,
|
||||
because failed attempts land on Okta rather than on a bind this app makes. D13 was decided
|
||||
before it was known the company already runs an Okta tenant. Once that was confirmed
|
||||
(security's scoping reply, September 2026), shipping LDAPS first and replacing it with
|
||||
Okta days later would mean building and deploying the weaker mechanism on purpose. Going
|
||||
straight to Okta avoids that.
|
||||
|
||||
### What still needs answering before this is buildable
|
||||
|
||||
Open from the security scoping thread, not yet closed:
|
||||
|
||||
- Which OIDC claim carries the AD `sAMAccountName` equivalent (`preferred_username`,
|
||||
`upn`, or a custom claim) — asked of security, answer pending.
|
||||
- The exact redirect/callback URI once the hostname situation is reconfirmed
|
||||
(`https://wp.controls.dev/api/auth/okta/callback` proposed).
|
||||
- The `Business Technology Group` pilot in Okta, requested for initial testing, with
|
||||
normal Okta session/MFA behavior rather than a stricter per-app rule.
|
||||
@@ -105,6 +105,18 @@
|
||||
say(bits.join(''), problem);
|
||||
}
|
||||
|
||||
// A 500 answers plain text ("Internal Server Error"), and r.json() on that
|
||||
// throws - which used to land in catch() and read as "could not reach the
|
||||
// server" while the server was answering fine (found 2026-08-23, the
|
||||
// production locations import). Read text, parse if it parses, keep status.
|
||||
function readJson(r) {
|
||||
return r.text().then(function (t) {
|
||||
var j = null;
|
||||
try { j = t ? JSON.parse(t) : null; } catch (e) { /* not JSON: a proxy or 500 page */ }
|
||||
return { ok: r.ok, status: r.status, body: j };
|
||||
});
|
||||
}
|
||||
|
||||
function importText(dryRun) {
|
||||
var text = (el(p + '-paste') || {}).value || '';
|
||||
if (!text.trim()) { say('Paste some rows or choose a CSV file first.', true); return; }
|
||||
@@ -114,7 +126,7 @@
|
||||
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
|
||||
body: JSON.stringify({ text: text, dry_run: !!dryRun }),
|
||||
})
|
||||
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
|
||||
.then(readJson)
|
||||
.then(function (res) {
|
||||
if (!res.ok) {
|
||||
say('⚠ Import refused — ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);
|
||||
@@ -142,7 +154,7 @@
|
||||
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
|
||||
body: JSON.stringify(read.payload),
|
||||
})
|
||||
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
|
||||
.then(readJson)
|
||||
.then(function (res) {
|
||||
if (!res.ok) {
|
||||
setAddError((res.body && res.body.detail) || ('Could not add it (HTTP ' + res.status + ')'));
|
||||
@@ -161,7 +173,7 @@
|
||||
method: 'PATCH', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
|
||||
body: JSON.stringify(patchBody),
|
||||
})
|
||||
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
|
||||
.then(readJson)
|
||||
.then(function (res) {
|
||||
if (!res.ok) {
|
||||
say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);
|
||||
|
||||
@@ -52,6 +52,13 @@ def run_migrations_online() -> None:
|
||||
connection=connection,
|
||||
target_metadata=target_metadata,
|
||||
compare_type=True,
|
||||
# Each migration commits on its own. One transaction for the WHOLE
|
||||
# run meant a crash at step N rolled back steps 1..N-1 while their
|
||||
# "Running upgrade" lines stayed on screen claiming they ran - the
|
||||
# 2026-08-21 outage's stamp-to-head repair trusted those lines and
|
||||
# left production missing two tables (found 2026-08-23 when the
|
||||
# locations import 500'd on a table that "had been created").
|
||||
transaction_per_migration=True,
|
||||
)
|
||||
with context.begin_transaction():
|
||||
context.run_migrations()
|
||||
|
||||
@@ -32,7 +32,11 @@ def upgrade() -> None:
|
||||
sa.Column('code', sa.String(length=80), nullable=False, server_default=''),
|
||||
sa.Column('description', sa.String(length=300), nullable=False, server_default=''),
|
||||
sa.Column('unit', sa.String(length=20), nullable=False, server_default=''),
|
||||
sa.Column('active', sa.Boolean(), nullable=False, server_default=sa.text('1')),
|
||||
# sa.true(), not sa.text('1'): SQLite coerces integer 1 to boolean,
|
||||
# Postgres refuses it (DatatypeMismatch) - found when this migration
|
||||
# took down the wp.controls.dev api container on 2026-08-21. The
|
||||
# location-taxonomy migration next door had it right all along.
|
||||
sa.Column('active', sa.Boolean(), nullable=False, server_default=sa.true()),
|
||||
sa.Column('sort', sa.Integer(), nullable=False, server_default='0'),
|
||||
sa.Column('created_at', sa.DateTime(timezone=True), nullable=True),
|
||||
sa.PrimaryKeyConstraint('id'),
|
||||
|
||||
@@ -2388,6 +2388,24 @@ def parse_location_rows(text: str) -> tuple[list[tuple[int, list[str]]], list[di
|
||||
rejected.append({"line": i, "text": line,
|
||||
"reason": "no letters or digits to make a code from"})
|
||||
continue
|
||||
# Postgres enforces VARCHAR lengths and refuses NUL/control bytes;
|
||||
# SQLite shrugs at both - which is how ONE bad CSV line 500'd the whole
|
||||
# production import (2026-08-23, BL-027's class again) instead of coming
|
||||
# back as a rejection with its line number. Validate per row, here,
|
||||
# so every dialect answers the same way: with a reason.
|
||||
if any(any(ord(ch) < 32 for ch in p) for p in parts):
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "contains control characters — re-save the file as plain CSV (UTF-8)"})
|
||||
continue
|
||||
long_p = next((p for p in parts if len(p) > 200), None)
|
||||
if long_p is not None:
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "a name is longer than 200 characters (%d)" % len(long_p)})
|
||||
continue
|
||||
if any(len(location_slug(p)) > 60 for p in parts):
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "a code would be longer than 60 characters"})
|
||||
continue
|
||||
rows.append((i, parts))
|
||||
return rows, rejected
|
||||
|
||||
@@ -2454,6 +2472,7 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
require_project_writable(db, user, project_id, "The location list cannot be changed")
|
||||
|
||||
rows, rejected = parse_location_rows(body.text)
|
||||
read_total = len(rows) + len(rejected)
|
||||
|
||||
existing = {n.path: n for n in db.scalars(
|
||||
select(models.LocationNode).where(models.LocationNode.project_id == project_id)
|
||||
@@ -2468,6 +2487,10 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
for line_no, parts in rows:
|
||||
segs = [location_slug(p) for p in parts]
|
||||
full = "/".join(segs)
|
||||
if len(full) > 200:
|
||||
rejected.append({"line": line_no, "text": "/".join(parts)[:120],
|
||||
"reason": "the combined path is longer than 200 characters"})
|
||||
continue
|
||||
if full in seen_in_file:
|
||||
duplicates.append({"line": line_no, "path": full, "names": parts,
|
||||
"reason": "already on line %d of this import" % seen_in_file[full]})
|
||||
@@ -2510,7 +2533,7 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
|
||||
result = {
|
||||
"project_id": project_id, "dry_run": bool(body.dry_run),
|
||||
"read": len(rows) + len(rejected),
|
||||
"read": read_total,
|
||||
"created": created, "duplicates": duplicates,
|
||||
"reactivated": reactivated, "rejected": rejected,
|
||||
}
|
||||
@@ -3007,6 +3030,18 @@ def parse_material_rows(text: str):
|
||||
rejected.append({"line": i, "text": raw.strip()[:120],
|
||||
"reason": "more than three columns - description, unit, code is the whole shape"})
|
||||
continue
|
||||
# Same guard as parse_location_rows: reject what Postgres would refuse
|
||||
# (VARCHAR limits, control bytes) with the line number, never a 500.
|
||||
if any(any(ord(ch) < 32 for ch in p) for p in parts):
|
||||
rejected.append({"line": i, "text": raw.strip()[:120],
|
||||
"reason": "contains control characters - re-save the file as plain CSV (UTF-8)"})
|
||||
continue
|
||||
caps = ((300, "description"), (20, "unit"), (80, "code"))
|
||||
long_col = next((("%s is longer than %d characters (%d)" % (label, cap, len(p)))
|
||||
for (cap, label), p in zip(caps, parts) if len(p) > cap), None)
|
||||
if long_col:
|
||||
rejected.append({"line": i, "text": raw.strip()[:120], "reason": long_col})
|
||||
continue
|
||||
rows.append((i, parts))
|
||||
return rows, rejected
|
||||
|
||||
|
||||
@@ -323,6 +323,13 @@ def run(page, base, tok):
|
||||
{"text": "Probe Building One,Probe Level 1,Probe Sector B"})
|
||||
chk("the import reports it as reactivated, not created or duplicate",
|
||||
len(again["body"]["reactivated"]) == 1 and not again["body"]["created"], again["body"])
|
||||
# The 2026-08-23 production 500, pinned (locations side): Postgres-refused
|
||||
# values reject by line, on every dialect, never crash the request.
|
||||
hz = api(page, "POST", "/api/projects/projA/locations/import",
|
||||
{"text": "Probe Building One," + "Y" * 220 + ",S1", "dry_run": True})
|
||||
chk("an over-long name is a line rejection, not a 500",
|
||||
hz["status"] == 200 and hz["body"]["rejected"]
|
||||
and "200 characters" in hz["body"]["rejected"][0]["reason"], hz["body"])
|
||||
same = [n for n in api(page, "GET",
|
||||
"/api/projects/projA/locations?include_inactive=true")["body"]["nodes"]
|
||||
if n["path"] == sec["path"]]
|
||||
|
||||
@@ -54,6 +54,20 @@ def main():
|
||||
chk("locations and materials are both instances of it - not a copy beside it",
|
||||
"locList = WPListImport(" in suite and "matList = WPListImport(" in suite
|
||||
and "function locImport(dryRun){ locList.importText" in suite)
|
||||
# The 2026-08-21 outage, pinned: a Boolean server_default of sa.text('1')
|
||||
# passes on SQLite (which coerces 1) and crash-loops Postgres at deploy
|
||||
# (DatatypeMismatch). Every migration must say sa.true()/sa.false().
|
||||
import re as _re
|
||||
bad = []
|
||||
vdir = os.path.join(ROOT, "server", "alembic", "versions")
|
||||
for fn in sorted(os.listdir(vdir)):
|
||||
if not fn.endswith(".py"):
|
||||
continue
|
||||
for ln in open(os.path.join(vdir, fn), encoding="utf-8"):
|
||||
if "Boolean" in ln and "server_default" in ln and not _re.search(r"server_default=sa\.(true|false)\(\)", ln):
|
||||
bad.append("%s: %s" % (fn, ln.strip()[:90]))
|
||||
chk("no migration gives a Boolean a non-portable server_default "
|
||||
"(sa.true()/sa.false() only)", not bad, ascii_(bad[:3]))
|
||||
model = open(os.path.join(ROOT, "server", "models.py"), encoding="utf-8").read()
|
||||
mat_block = model[model.find("class MaterialItem"):model.find("class WpFile")]
|
||||
cols = re.findall(r"^\s+(\w+): Mapped", mat_block, re.M)
|
||||
@@ -91,6 +105,20 @@ def main():
|
||||
_, listing = api(base, "/api/projects/projA/materials", root)
|
||||
chk("...and nothing was written", listing["items"] == [])
|
||||
|
||||
# The 2026-08-23 production 500, pinned: what Postgres refuses (VARCHAR
|
||||
# overflow, control bytes) must come back as a per-line rejection - on
|
||||
# EVERY dialect - never crash the request.
|
||||
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
|
||||
{"text": "Sample " + "x" * 300 + ",EA", "dry_run": True})
|
||||
chk("an over-long description is a line rejection, not a 500",
|
||||
code == 200 and rep["rejected"] and "300 characters" in rep["rejected"][0]["reason"]
|
||||
and not rep["created"], ascii_(rep))
|
||||
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
|
||||
{"text": "Sample widget\u0000,EA", "dry_run": True})
|
||||
chk("a control byte is a line rejection, not a 500",
|
||||
code == 200 and rep["rejected"]
|
||||
and "control characters" in rep["rejected"][0]["reason"], ascii_(rep))
|
||||
|
||||
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
|
||||
{"text": text, "dry_run": False})
|
||||
_, listing = api(base, "/api/projects/projA/materials", root)
|
||||
|
||||
Reference in New Issue
Block a user