D13 / wave 10 - the decision and the task file, before any code

Authentication moves to the domain over LDAPS. Nothing in the spec covered
auth against a directory, so per CLAUDE.md this is new scope with a new `D`
id rather than a widened old one. D1-D12 were taken.

Records what was verified against the live environment on Aug 21 rather than
assumed: LDAPS on 636 with TLS 1.3, the DC cert chain up through PRIME
CONTROLS ISSUING CA 1 to a root valid until 2051, six DCs in the SRV record,
and `prime.local` in every DC cert's SAN - which is why the client connects
to the domain name and not to a DC or an IP.

Also records why the certificate already serving the site is not usable for
this: it is a Let's Encrypt DV cert held by an OpenResty host outside this
repo, and it carries no relationship to prime.local. The question was asked
directly and the answer is not obvious, so it belongs in the record.

Eight tasks, T10.1 through T10.8. Two questions are left open in the decision
doc to be answered rather than guessed - break-glass access when the DC is
unreachable, and whether existing usernames match sAMAccountName.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-21 13:54:01 -05:00
parent a8e28bf3ab
commit 3c9343dfc8
3 changed files with 445 additions and 0 deletions

View File

@@ -0,0 +1,129 @@
# Decisions — August 21, 2026
One item, and it is the largest single change to the auth model since the login portal
shipped. Like the August 18 and August 20 sets it is a **new item** with its own `D` id,
not a reinterpretation of an existing one. `D1``D12` are taken; this is `D13`.
Raised by Cody Schaefer on Aug 21 2026 while asking how the suite handles HTTPS. Nothing in
`IMPLEMENTATION.md`, in `CR`/`F`/`S`/`A`/`B`/`C`, or in `docs/waves/backlog.md` covers
authentication against the domain — so this is new scope, and it gets a new ID rather than
being folded into the login-portal work that produced `server/auth.py`.
---
## D13 — Authentication moves to the domain over LDAPS
- **Amends:** the authentication model shipped in `DEPLOY-login-portal.md` (bcrypt hashes in
`users.password_hash`, verified in-process). That document describes what is being
replaced, not what is wrong — it was correct for a suite with no directory behind it.
- **Surface:** `server/` (`auth.py`, `app.py`, `models.py`, `manage_users.py`, `notify.py`,
a new `ldap_auth.py`, a new migration), `html/` (`login.html`, `login.js`, `admin.js`,
`users.js`), `requirements.txt`, `docker-compose.yml`, `Dockerfile`, deployment docs.
- **Wave:** 10 (`docs/waves/wave-10.md`). Depends on wave 9 merged, which it is.
### The decision
The suite stops storing passwords. A sign-in becomes a **simple bind to
`ldaps://prime.local:636`** as `<username>@prime.local` using the password the person typed.
A successful bind is the authentication. `users.password_hash` is dropped from the schema.
Four parts, all four required for the item to be done:
1. **LDAPS bind replaces local password verification.** `password_hash` is removed from the
model and from the database by migration. No password is stored, hashed or otherwise.
2. **Accounts are provisioned just-in-time.** A successful bind for a username with no
`users` row creates one, at the default role, with `full_name`/`email` read from the
directory.
3. **A required group gates login.** An AD group is configured; a bind that succeeds but
whose account is not in that group is refused. Membership is evaluated including nested
groups.
4. **Existing accounts keep their roles, and roles stay local.** An existing `admin` stays
an admin on first directory login. Granting admin to an existing account continues to
work from the Admin console. The directory supplies *identity*; this app supplies
*authorization*.
### Why LDAPS and not the certificate already in play
Recorded because the question was asked directly and the answer is not obvious.
The site's serving certificate is a **Let's Encrypt** cert (`CN=wp.controls.dev`, issued by
`Let's Encrypt YE2`, expiring 2026-11-08) held by an **OpenResty** instance at
`192.168.3.56` that is not part of this repo. It is a public domain-validated certificate.
It attests that whoever presented it controls DNS for `wp.controls.dev`; it carries no user
identity and no relationship to `prime.local`. There is no configuration that turns it into
a domain credential, so cert-based auth was never available "for free".
Client-certificate auth (mTLS) was considered and rejected for this wave: TLS terminates two
hops upstream at OpenResty, so the API never sees the handshake, and doing it in-app would
mean bypassing the proxy and losing the CSP/HSTS headers and static serving with it.
### What was verified before writing this (Aug 21 2026)
| Fact | Value |
|---|---|
| LDAPS reachable | `192.168.3.37:636` open, TLS 1.3, `TLS_AES_256_GCM_SHA384` |
| DC cert issuer | `CN=PRIME CONTROLS ISSUING CA 1, DC=prime, DC=local` |
| Root of that chain | `CN=PRIME CONTROLS ROOT CA` (self-signed, expires 2051-09-09) |
| Issuing CA expiry | 2036-09-09 |
| DC cert SAN | `DR-DC10Core.prime.local`, `prime.local`, `PRIME` |
| DCs published in `_ldap._tcp.prime.local` | six — `nla-dc10`, `lew-dc20`, `dr-dc30-core`, `lew-dc40`, `SABINEDC`, `dr-dc10core` |
| Chain validates against root+issuing bundle | yes — `Verify return code: 0 (ok)` |
Two consequences of that table, both binding on the build:
- **Connect to `prime.local`, not to a DC name or an IP.** Every DC's certificate carries
`prime.local` in its SAN, so the domain name both passes hostname validation and
round-robins across all six DCs. Verified: `prime.local` gives `0 (ok)`; the raw IP
`192.168.3.37` gives `62 (hostname mismatch)`, because there is no IP SAN.
- **The trust anchor is a CA certificate, not a certificate issued to this app.** The API is
the TLS *client*; clients present nothing. It needs `PRIME CONTROLS ROOT CA` plus
`PRIME CONTROLS ISSUING CA 1` as a PEM bundle, which is public information. No CSR, no
enrollment, no private key, nothing to request from IT.
### Non-negotiables
These are the ways this change goes wrong, and each has a done-when check in wave 10.
- **An empty password must be rejected before `bind()` is called.** In LDAP a simple bind
with an empty password is an *anonymous* bind and it **succeeds**. Without an explicit
guard, a blank password authenticates as any username submitted. This is the single
highest-severity failure mode in the item and it gets its own test.
- **`validate=ssl.CERT_REQUIRED` with an explicit CA file.** Not `CERT_NONE`, and not the
system trust store. `CERT_NONE` still encrypts, so it fails silently — what it loses is
the ability to distinguish the real DC from an attacker who terminates the TLS session,
harvests the domain password and relays the bind onward. Since domain credentials now
cross that channel, a compromise escalates from "this app" to Windows, mail and file
shares. The system store is refused separately because it currently trusts five other
self-signed CAs (`prime-DR-CAPRIME-CA`, `prime-DR-CA_PRIME-CA`, `prime-DR-DC20-CA`,
`PRIME CONTROLS ISSUING CA 2`, and a stray `L55401TDKLY3.prime.local` machine cert in
Trusted Root).
- **The app's lockout must trip below the domain's.** `LOGIN_MAX_ATTEMPTS` currently writes
to the local `users` row. Once failures are binds, they count against the **AD** lockout
policy, so an unauthenticated caller hammering `/api/auth/login` can lock real domain
accounts out of Windows. The local throttle must stop calling the DC before the domain
threshold is reached.
- **Never leak which usernames exist.** `login()` today equalises response timing on purpose
so a caller cannot enumerate accounts. Directory error 49 sub-codes (`52e` bad password,
`532` password expired, `533` disabled, `775` locked) are useful in the log and must not
reach the response body.
### Open, to confirm in the PR rather than decide alone
- **Break-glass.** Removing `password_hash` means an unreachable DC locks out *everyone*,
admins included. See `T10.2`.
- **Username ↔ `sAMAccountName` mapping.** Existing accounts were created by
`manage_users.py` with hand-typed usernames. Criterion 4 holds only where those match the
directory's `sAMAccountName`. The production account list must be checked against AD
before this deploys; a mismatch means an existing admin gets a *second*, JIT-provisioned
account at the default role instead of keeping their admin. `auth.find_user` already
matches on username **or** email case-insensitively, which covers some of the gap but not
all of it.
### Explicitly out of scope
- mTLS / client-certificate authentication (see above).
- Kerberos / SPNEGO single sign-on. It is the better long-term answer for domain-joined
desktops and needs a keytab, an SPN and browser trust configuration; it is not this item.
- Group-to-role mapping (e.g. an AD group that confers `project_admin`). Criterion 4 keeps
authorization local on purpose. Worth its own item later; logged in `backlog.md`.
- Replacing the Let's Encrypt certificate or changing anything on the OpenResty host.

311
docs/waves/wave-10.md Normal file
View File

@@ -0,0 +1,311 @@
# Wave 10 — Domain authentication over LDAPS
**Items:** `D13`
**Depends on:** wave 9 merged (it is — `a8e28bf`)
One item, eight tasks. The item is stated in `docs/waves/decisions-2026-08-21.md`; read it
before starting, particularly the **Non-negotiables** section, which is where this change
goes wrong if it goes wrong.
**This wave is field-visible in one direction only.** Nobody gains a screen. People sign in
with their Windows password instead of an app password, the "Forgot password?" link
disappears, and admins stop issuing passwords. Everything else is invisible — which means
the done-when checks are the only evidence the wave worked.
**Build order is task order** for once. `T10.1` is a standalone module with no callers and
should merge first; nothing else can be tested until it exists.
---
### T10.1 — The LDAPS client
- **Items:** `D13` (1)
- **Depends on:** nothing
- **Blocks:** T10.2, T10.4, T10.5, T10.7
- **Surface:** `server/`
- **Files:** new `server/ldap_auth.py`, `server/requirements.txt`, `docker-compose.yml`,
`Dockerfile`, `server/.env.example`
**Problem:** There is no directory client in the repo. `ldap3` is not a dependency, the API
container has no CA bundle, and the `api` service sits on the `internal` network, which has
no default gateway and therefore no route to `192.168.3.x` at all.
**Do:** Add `ldap3` (pinned, per the convention at the top of `requirements.txt`). Write
`server/ldap_auth.py` exposing two functions and nothing else:
- `verify(username, password) -> LdapResult | None` — simple bind as
`f"{username}@{domain}"` against `ldaps://{host}:636`.
- `member_of(conn, group) -> bool` — nested-group aware, via
`LDAP_MATCHING_RULE_IN_CHAIN` (`1.2.840.113556.1.4.1941`). `memberOf` alone is direct
membership only and will wrongly refuse anyone in a nested group.
Configuration by environment: `LDAP_HOST` (default `prime.local`), `LDAP_DOMAIN` (default
`prime.local`), `LDAP_CA_FILE`, `LDAP_REQUIRED_GROUP`, `LDAP_TIMEOUT_SECONDS`. Attach the
`outbound` network to `api` in `docker-compose.yml` — the same reason `assets_db.py` needed
it, and the comment there already explains the gateway-less `internal` network. Mount or
`COPY` the PEM bundle and point `LDAP_CA_FILE` at it.
Connect to **`prime.local`**, never a DC hostname and never an IP. See the decision doc for
why: SAN coverage plus round-robin across six DCs in one move. Wrap the bind in a retry
across resolved addresses, because round-robin will hand out a rebooting DC's address.
**Done when:**
- [ ] `ldap3` is pinned to an exact version in `requirements.txt`
- [ ] an empty or whitespace-only password returns failure **without calling `bind()`**
- [ ] an empty username returns failure without calling `bind()`
- [ ] `Tls` is constructed with `validate=ssl.CERT_REQUIRED` and an explicit `ca_certs_file`
- [ ] no code path sets `CERT_NONE`, and none falls back to the system trust store
- [ ] `member_of` returns true for an account in a **nested** child of the required group
- [ ] a bind against `192.168.3.37` (raw IP) fails hostname validation rather than silently passing
- [ ] `docker compose exec api openssl s_client -connect prime.local:636 -CAfile $LDAP_CA_FILE` reports `Verify return code: 0 (ok)`
- [ ] the module imports cleanly with no LDAP env set (unconfigured is a first-class state, as with `MICRON_DB_URL`)
---
### T10.2 — The login path binds instead of hashing
- **Items:** `D13` (1)
- **Depends on:** T10.1
- **Blocks:** T10.3, T10.4
- **Surface:** `server/`
- **Files:** `server/app.py` (`login`), `server/auth.py`
**Problem:** `login()` at `server/app.py:681` calls `auth.verify_password` against
`user.password_hash`. The lockout counter it maintains is about to start counting *domain*
bind failures, which changes what that counter is for.
**Do:** Replace the credential check with `ldap_auth.verify`. Keep the surrounding shape —
the deliberate timing equalisation, the generic `401`, the `403` for a disabled local
account, `last_login_at`, the session cookie. Rework the throttle so the local counter trips
**before** the domain policy is reached and short-circuits without calling the DC: the
failure mode to avoid is this endpoint being usable to lock domain accounts out of Windows.
Log directory error-49 sub-codes for diagnosis; return the same generic message regardless.
Decide the break-glass question here (see **Open** in the decision doc) and record the answer
in this file before implementing, because it changes this task's shape.
**Done when:**
- [ ] a correct domain password signs in and sets the session cookie
- [ ] a wrong password is refused with the generic message
- [ ] a blank password is refused (guards `T10.1` from the caller's side too)
- [ ] a user not in the required group is refused even though the bind succeeded
- [ ] `is_active = false` locally still refuses, independent of the directory
- [ ] the local throttle trips below the domain lockout threshold and stops calling the DC
- [ ] no response body distinguishes "no such user" from "wrong password"
- [ ] error-49 sub-codes appear in the log and nowhere in any response
---
### T10.3 — Drop `password_hash`
- **Items:** `D13` (1)
- **Depends on:** T10.2
- **Blocks:** T10.6, T10.8
- **Surface:** `server/`
- **Files:** `server/models.py`, new migration, `server/app.py`, `server/auth.py`,
`server/manage_users.py`
**Problem:** With binds doing the work, every password code path is dead weight and a
liability. It is also the criterion that makes this item irreversible, so it lands on its own
commit.
**Do:** Remove `password_hash` from `models.User` and drop the column in a new Alembic
revision with `down_revision = 'a1b8c6d4e2f9'` (the current head — confirm with
`alembic heads` rather than trusting this line). Remove from `auth.py`: `hash_password`,
`verify_password`, `password_problem`, `MIN_PASSWORD_LEN`, `_COMMON_PASSWORDS`,
`create_reset_token`, `decode_reset_token`, `RESET_MINUTES`. Remove from `app.py`:
`/api/auth/forgot-password`, `/api/auth/reset-password`, `/api/auth/reset-available`,
`/api/auth/password`, `/api/auth/users/{user_id}/password`, and the `_reset_last` throttle
with `reset_body`. Remove `reset-password` from `manage_users.py` and the password prompt
from `create` / `create-admin`.
`token_version` **stays.** It is still the session-revocation mechanism — role changes and
deactivation should bump it even though password changes no longer exist.
**Do not** remove the account-management endpoints themselves. `/api/auth/users/{id}/role`
is criterion 4 and must keep working.
**Done when:**
- [ ] `grep -rn "password_hash\|hash_password\|verify_password\|password_problem" server/` returns nothing outside the migration
- [ ] `alembic upgrade head` then `downgrade -1` round-trips on SQLite and on Postgres
- [ ] the migration's `downgrade()` recreates the column nullable, not `NOT NULL` — there are no hashes to put back
- [ ] `token_version` still invalidates sessions, exercised by a role change
- [ ] `manage_users.py list`, `disable`, `enable` still work; `reset-password` is gone
- [ ] `python -m server.manage_users create-admin <u>` creates an admin with no password prompt
---
### T10.4 — Just-in-time provisioning, without trampling existing accounts
- **Items:** `D13` (2, 4)
- **Depends on:** T10.2
- **Blocks:** T10.7
- **Surface:** `server/`
- **Files:** `server/app.py` (`login`), `server/auth.py`
**Problem:** Criteria 2 and 4 pull in opposite directions. Provisioning on first login must
create accounts that do not exist, and must not touch the role of accounts that do — an
existing `admin` signing in for the first time after this wave must still be an admin
afterwards.
**Do:** On a successful bind that passes the group check, look the account up with
`auth.find_user` (already case-insensitive across username **and** email). If it exists,
update only `last_login_at` and — if empty locally — `full_name` and `email` from the
directory. **Never write `role`.** If it does not exist, create it at
`ROLE_PROJECT_USER` with `full_name`/`email` from the directory, honouring the existing
`auto_add_projects` machinery so a JIT account lands in the right projects. Note the
flush-order warning in the `models.py` docstring: `create_user` in `app.py` already handles
this correctly for account + `ProjectMember` rows in one flush — follow it.
Write an `AuditLog` row for each JIT creation. An account appearing without an administrator
creating it is exactly the kind of event that record exists for.
**Done when:**
- [ ] an unknown username with a valid bind and group membership gets a `users` row at `project_user`
- [ ] `full_name` and `email` are populated from the directory on creation
- [ ] an existing `admin` signing in is still `admin` afterwards — asserted, not assumed
- [ ] an existing account with a locally-set `full_name` does not have it overwritten
- [ ] a JIT account with `auto_add_projects` peers gets its `ProjectMember` rows
- [ ] each JIT creation writes an `AuditLog` row
- [ ] a failed bind creates **no** row
- [ ] a bind that succeeds but fails the group check creates **no** row
---
### T10.5 — The required group is configurable
- **Items:** `D13` (3)
- **Depends on:** T10.1
- **Blocks:** T10.8
- **Surface:** `server/` + `html/`
- **Files:** `server/notify.py` (`DEFAULTS`), `server/app.py` (`SettingsIn`, `/api/settings`),
`html/admin.js`
**Problem:** The group has to be settable without a redeploy, but it gates login — a typo
in a text box locks the whole company out, including whoever typed it.
**Do:** Add `ldap_required_group` to `notify.DEFAULTS` and `SettingsIn`, and a field in the
Admin console's settings card next to the notification block. `LDAP_REQUIRED_GROUP` from the
environment is the initial value and the fallback when the setting is empty.
**Validate on save, not on login.** Before persisting, resolve the group in the directory and
confirm the saving admin is themselves a member. Refuse the save with a specific message if
either check fails. That single guard is what stops the lockout scenario, so it is not
optional. Do not add it to `notify.PUBLIC_KEYS` — an unauthenticated caller must not be able
to read the group name off the login page.
**Done when:**
- [ ] the field appears in the Admin console and persists across a restart
- [ ] saving a group that does not exist is refused, with a message naming the group
- [ ] saving a group the current admin is not in is refused
- [ ] an empty setting falls back to `LDAP_REQUIRED_GROUP`, and empty-with-no-env means no group gate (documented, logged at startup)
- [ ] the group name is absent from `/api/settings` for a non-admin and from any pre-login response
- [ ] the field is reachable by keyboard, labelled, and announces its save state through the existing `aria-live` region (`C1`)
---
### T10.6 — Strip the password UI
- **Items:** `D13` (1)
- **Depends on:** T10.3
- **Blocks:** nothing
- **Surface:** `html/`
- **Files:** `html/login.html`, `html/login.js`, `html/admin.js`, `html/users.js`,
`html/auth-guard.js`
**Problem:** `login.html` has three views — sign-in, forgot-password, set-new-password — and
two of them now point at endpoints that no longer exist. `auth-guard.js` has a
change-password dialog, and the user-admin UI has a "reset password" action per row.
**Do:** Remove the `#view-forgot` and `#view-reset` sections, the `#forgot-link`, and the
reset-token handling in `login.js`. Remove the change-password dialog from `auth-guard.js`
(`backlog.md:180` refers to it) and the per-row password reset from the users UI. Keep the
sign-in form, and relabel the password field's hint to say it is the Windows/domain password
— people need to know which password to type.
Keep the role-granting controls exactly as they are. That is criterion 4.
**Done when:**
- [ ] `grep -rn "forgot\|reset-password\|new-password" html/` returns nothing but unrelated matches
- [ ] the sign-in form still submits, and a failure still announces through `role="alert"` (`login.html` already does this correctly — do not regress it)
- [ ] the password field says which password to enter
- [ ] no dead `<a href="#">` or handler remains for a removed view
- [ ] granting admin to an existing user still works from the console
- [ ] exercised at 390px and at 1440px, screenshots in the PR
- [ ] no raw hex added to any stylesheet (the token rule)
---
### T10.7 — Make the suite testable without a domain controller
- **Items:** `D13` (1, 2, 3)
- **Depends on:** T10.1, T10.4
- **Blocks:** T10.8
- **Surface:** `tests/` + `server/`
- **Files:** `server/ldap_auth.py`, `tests/browser_check.py`, `tests/launcher_check.py`,
`tests/console_dialogs_check.py`, `tests/url_state_check.py`, `server/smoketest.py`,
`server/seed_demo.py`, new `tests/ldap_auth_check.py`
**Problem:** This is the task most likely to be underestimated. Four existing checks build
users with `password_hash=auth.hash_password(PW)` and sign in over HTTP;
`console_dialogs_check.py` drives the password-reset prompt specifically. `smoketest.py` and
`seed_demo.py` both sign in. None of them can reach a DC, and CI has no domain.
**Do:** Make the bind injectable — a module-level seam in `ldap_auth.py` that a test can
substitute (a fake directory: usernames, passwords, groups, full names), selected by an env
var that is refused when a real `DATABASE_URL` is configured, mirroring how
`auth._load_secret` refuses an ephemeral key in production. Port the four checks onto it.
Delete the password-reset half of `console_dialogs_check.py` — the flow it covers no longer
exists — and say so in the PR rather than leaving a skipped test.
Add `tests/ldap_auth_check.py` covering the non-negotiables from the decision doc: empty
password, empty username, `CERT_REQUIRED` asserted by inspecting the constructed `Tls`,
nested group membership, group-check refusal creating no user, and existing-admin role
preservation.
**Done when:**
- [ ] the full `tests/` suite passes with no DC reachable
- [ ] the stub backend cannot be selected when a non-SQLite `DATABASE_URL` is set — asserted by a test
- [ ] `tests/ldap_auth_check.py` covers all six cases above
- [ ] the empty-password case fails loudly if the guard is removed (verified by removing it)
- [ ] `smoketest.py` and `seed_demo.py` document which credentials they now need
- [ ] the removed password-reset checks are called out in the PR, not silently dropped
---
### T10.8 — Documentation and the deploy runbook
- **Items:** `D13`
- **Depends on:** T10.3, T10.5, T10.7
- **Blocks:** nothing
- **Surface:** docs
- **Files:** `DEPLOYMENT.md`, `server/README.md`, `DEPLOY-login-portal.md`, `CLAUDE.md`,
`IMPLEMENTATION.md`
**Problem:** `DEPLOY-login-portal.md` documents creating the first admin with a password and
is the page an admin will reach for. `DEPLOYMENT.md` describes `AUTH_SECRET_KEY` and SMTP but
knows nothing about a directory. `CLAUDE.md`'s verification section tells anyone touching the
frontend to run the smoke test, which changes here.
**Do:** Document the LDAP variables, how to produce the CA bundle from the two thumbprints
in the decision doc, and the `prime.local`-not-an-IP rule with the reason. Add the
`openssl s_client -CAfile` check as the first-line diagnostic. Rewrite the
`DEPLOY-login-portal.md` bootstrap step: the first admin is now an existing directory account
promoted with `manage_users.py`, not an account created with a password. State plainly what
happens when the DC is unreachable, whatever `T10.2` decides.
**Done when:**
- [ ] every new env var is documented in `server/.env.example` and `DEPLOYMENT.md`
- [ ] the CA bundle procedure is reproducible by an admin who has not read this thread
- [ ] `DEPLOY-login-portal.md` no longer instructs anyone to set a password
- [ ] the DC-unreachable behaviour is stated explicitly
- [ ] `IMPLEMENTATION.md` section 4 lists wave 10
- [ ] no doc still claims passwords are stored as bcrypt hashes