server/ldap_auth.py: simple bind to ldaps://prime.local:636 as sAMAccountName@prime.local, nested-group membership via the LDAP_MATCHING_RULE_IN_CHAIN extensible match, and a selftest() that validates the DC certificate without binding so it can never contribute to a lockout. Verified against the live domain, not just reasoned about: selftest() to prime.local -> ok, "certificate validates" selftest() to 192.168.3.37 -> refused, untrusted (no IP SAN) empty / whitespace password -> empty_input, with Connection nulled out so any call to bind() would have raised missing CA file -> unconfigured, is_config_problem=True Tls.validate -> ssl.CERT_REQUIRED, explicit ca_certs_file Three things here are load-bearing and commented as such at the call site: - The empty-password guard runs BEFORE bind(). An LDAP simple bind with an empty password is an anonymous bind and it SUCCEEDS, so without the guard a blank password authenticates as whatever username was submitted. - No `version=` pin on Tls. An earlier draft of this file pinned PROTOCOL_TLSv1_2, which would have silently downgraded every connection from the TLS 1.3 these DCs actually negotiate. - Retries cover connect failures only. A rejected credential returns immediately, because every failed bind counts against the domain lockout policy and this endpoint must not become a way to lock people out of Windows. The trust anchor is server/certs/prime-ca-chain.pem - PRIME CONTROLS ROOT CA plus ISSUING CA 1, public certificates with no private key, checked in because they are public and long-lived (2051 / 2036). The system trust store is deliberately not used: it currently trusts five other self-signed CAs on this estate. LDAP_CA_FILE overrides the path for a mounted bundle. docker-compose.yml: the `outbound` network is no longer optional. Its comment said to detach it if you were not using the Micron asset picker; doing that now breaks every sign-in, since `internal` has no default gateway and therefore no route to prime.local:636. Not yet verified, and called out rather than assumed: the nested-group case needs a real group with a nested member, and the in-container `openssl s_client -CAfile` check needs the stack. Both are T10.1 done-when boxes still open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
314 lines
16 KiB
Markdown
314 lines
16 KiB
Markdown
# 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.
|
|
|
|
**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
|
|
- [ ] `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
|