DEPLOY-login-portal.md was the most wrong and is rewritten. It described taking a username/password portal live - bcrypt, and a first admin created with `create-admin --password`. Every command in it now fails. It keeps its filename and carries a note saying what it replaced, because an admin holding the old copy needs to know why the steps stopped working rather than concluding the deploy is broken. New content leads with the warning that there is no break-glass, and puts verification BEFORE announcing the deploy - the log line, the certificate check that binds nothing, then a real sign-in. DEPLOYMENT.md: AUTH_RESET_* replaced with the LDAP variables; the users table row no longer claims a password_hash column; "Self-service password reset" replaced by a section saying there isn't one and pointing at Okta. New "Domain authentication" section covering the three things that are not obvious - why prime.local and never a DC or an IP, why the CA bundle is not a certificate issued to this app (with the thumbprints and a Get-ChildItem line to rebuild it), and why the outbound network stopped being optional - plus the lockout arithmetic written out so the next person to raise AUTH_MAX_ATTEMPTS sees the constraint rather than a magic 2. server/README.md: endpoint table drops /api/auth/password and gains the role route; the login-portal section becomes domain authentication; create-admin becomes the two-step bootstrap (sign in, then promote). CLAUDE.md: a new "authentication rules" section beside the token rule, for the same reason that one exists - four things that look like tidying-up if you do not know why. The empty-password guard that must run before bind(), CERT_REQUIRED with an explicit CA file, AUTH_MAX_ATTEMPTS being arithmetic rather than taste, and connecting to the domain name rather than a DC. Plus: no break-glass, and roles are local - never read a role from AD. Closed three done-when boxes that were open rather than ticked: T10.8 all of them T10.9 promote/demote verified against a real bind (Aug 24), not a stub T10.3 the Postgres round trip, on postgres:16-alpine Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
374 lines
20 KiB
Markdown
374 lines
20 KiB
Markdown
# Wave 10 — Domain authentication over LDAPS
|
|
|
|
**Items:** `D13`, `D14`
|
|
**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.
|
|
|
|
**Break-glass: none — decided Aug 21, see the decision doc.** LDAPS is the only way in, so
|
|
this task adds no fallback path. What it must add instead is *visibility*: a startup log line
|
|
stating whether LDAP is configured and whether the DC answered, because with no fallback a
|
|
misconfigured deploy is indistinguishable from a forgotten password at the login box.
|
|
|
|
**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
|
|
- [x] `alembic upgrade head` then `downgrade -1` round-trips on SQLite and on Postgres (16.15, the compose image — Aug 24; needed a pre-existing T8.6 migration bug fixed first, see `495d87d`)
|
|
- [ ] 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.
|
|
|
|
**A JIT account gets NO project access, and that is deliberate.** An earlier draft of this
|
|
task said to honour the `auto_add_projects` machinery so a new account "lands in the right
|
|
projects" — that was wrong about how the flag works. `auto_add_projects` is evaluated when a
|
|
**project** is created (`app.py:245`), marking accounts that should join every *new* job; it
|
|
cannot retroactively add a new account to existing ones. There is no correct default, so
|
|
least privilege applies: the account exists, can sign in, and sees nothing until someone
|
|
grants access. That is a real UX cliff — a successful sign-in into an empty app — so it has
|
|
to be visible to admins rather than silent, which is what the `AuditLog` row is for.
|
|
|
|
Note the flush-order warning in the `models.py` docstring: `create_user` in `app.py` handles
|
|
account-then-membership correctly in one flush — follow it if you add rows.
|
|
|
|
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 has NO `ProjectMember` rows and sees no projects
|
|
- [ ] the new account appears in the Admin console user list so access can be granted
|
|
- [ ] 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.
|
|
|
|
**Decided Aug 21: "Forgot password?" is KEPT and repointed at
|
|
`https://primecontrols.okta.com/`.** An earlier draft of this task removed the link, and a
|
|
plain sentence saying "contact IT" was proposed instead. Okta is the better answer — it is a
|
|
real self-service path, and with no app password and no break-glass it is the only recovery
|
|
route that exists. Note this is the first sign of an Okta tenancy on this estate; see the
|
|
new backlog entry.
|
|
|
|
Mechanics that matter: it is a plain `<a href>`, not a form post, so the `form-action 'self'`
|
|
in the CSP does not apply and no `navigate-to` directive is set — off-origin link navigation
|
|
is allowed as-is. `target="_blank"` needs `rel="noopener noreferrer"`, and there must be NO
|
|
click handler on `#forgot-link`: the old one called `preventDefault()` to swap views, and
|
|
leaving it would silently swallow the navigation.
|
|
|
|
**Do:** Remove the `#view-forgot` and `#view-reset` sections and the reset-token handling in
|
|
`login.js`, and repoint `#forgot-link` as above. 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 prose
|
|
- [ ] "Forgot password?" opens `https://primecontrols.okta.com/` in a new tab
|
|
- [ ] `#forgot-link` has NO click handler (a `preventDefault()` would swallow the navigation)
|
|
- [ ] a 503 from the login endpoint says sign-in is unavailable, not that the password is wrong
|
|
- [ ] 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:**
|
|
|
|
- [x] every new env var is documented in `server/.env.example` and `DEPLOYMENT.md`
|
|
- [x] the CA bundle procedure is reproducible by an admin who has not read this thread — thumbprints and a `Get-ChildItem` one-liner in `DEPLOYMENT.md`
|
|
- [x] `DEPLOY-login-portal.md` no longer instructs anyone to set a password — rewritten, with a note saying what it replaced so an admin holding the old copy is not misled
|
|
- [x] the DC-unreachable behaviour is stated explicitly, with the diagnostic commands
|
|
- [x] `IMPLEMENTATION.md` section 4 lists wave 10
|
|
- [x] no doc still claims passwords are stored as bcrypt hashes (swept; remaining matches all say the opposite)
|
|
- [x] `CLAUDE.md` carries the four load-bearing auth rules, next to the token rule
|
|
|
|
---
|
|
|
|
### T10.9 — D14: the CLI authenticates, and stops creating accounts
|
|
|
|
- **Items:** `D14`
|
|
- **Depends on:** T10.3
|
|
- **Blocks:** T10.8
|
|
- **Surface:** `server/`
|
|
- **Files:** `server/manage_users.py`
|
|
|
|
**Problem:** `manage_users.py` writes to the `users` table with no authentication at all.
|
|
It also still offers `create-admin` / `create`, which are redundant now that accounts
|
|
provision themselves — and worse than redundant, because a hand-typed username can end up
|
|
matching no directory identity.
|
|
|
|
**Do:** As stated in `D14`. Remove the two create commands, add `promote` / `demote`, gate
|
|
every state-changing command on a prompted domain bind, and write an `AuditLog` row naming
|
|
the operator. Write the audit row by hand rather than importing `log_event` from `app.py` —
|
|
that would pull FastAPI and the whole application into a CLI startup for one INSERT.
|
|
|
|
**Done when:**
|
|
|
|
- [x] `create-admin`, `create` and `reset-password` are rejected as invalid choices
|
|
- [x] `list` works with no credential and with no LDAP configured
|
|
- [x] a state-changing command with LDAP misconfigured refuses instead of proceeding
|
|
- [x] there is no `--password` flag on any command
|
|
- [x] `promote` raises a role; `demote` returns an account to `project_user`
|
|
- [x] promoting YOURSELF is allowed and recorded with `self: true`
|
|
- [x] the last active admin cannot be demoted
|
|
- [x] an unknown account gives an error that says accounts are made on first sign-in
|
|
- [x] every change writes an `AuditLog` row naming the operator
|
|
- [x] verified against a real domain bind — `promote` and `demote` confirmed working Aug 24 2026
|