Files
Project-SDE-WP-Suite/docs/waves/wave-10.md
Cody Schaefer d261024139 T10.6 - before/after login screenshots at 390px and 1440px
Closes the last CLAUDE.md verification box on T10.6, which requires any task
touching the frontend to be exercised at both widths with screenshots in the PR.

Captured with the repo's own tests/baseline_shots.py rather than by hand; it
already treats login as the one page whose real state is signed out. "Before"
was shot from a detached git worktree at main (a8e28bf) so each half ran against
its own server - the old login page needs the forgot-password endpoints that no
longer exist here. The worktree is removed again; the working checkout was never
switched.

What the diff shows, at both widths:

  the two reset views are gone
  the password field gained a hint saying WHICH password to type
  "Forgot password?" stays, now pointing at Okta

The hint uses the .hint class login.html already had, so no CSS was added and
no literal with it. It wraps to two lines at both widths, sits between the
password field and the button, and nothing overflows or shifts the card.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 09:35:20 -05:00

22 KiB

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
  • 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 — CLOSED, NOT BUILT (Aug 24 2026): the required group stays an env var

  • Items: D13 (3)
  • Status: won't build. LDAP_REQUIRED_GROUP in the environment is the answer.

Do not build this later by reading the original task and assuming it was skipped. It was proposed, examined and rejected on purpose, and the reasoning is below.

What it was going to be: the required group moved out of the environment into an Admin console setting, with a validate-on-save guard that resolved the group in the directory and confirmed the saving admin was a member.

Why it is not being built:

  1. The console requirement was invented here, not asked for. D13 criterion 3 says "An AD group is configured" — not "configurable from the console". The env var satisfies the criterion as written.
  2. The validate-on-save guard existed only to defend against a risk the console itself introduced. A feature whose complexity exists to defend against itself is usually the wrong feature.
  3. The lockout scenario it defended against is already handled. A group that does not resolve raises LookupError in member_of, which verify() maps to GROUP_NOT_FOUND, which is_config_problem classifies as ours — so login() answers 503, not 401, and the log says "required group 'X' does not resolve in DC=prime,DC=local — refusing the sign-in. This is a configuration fault, not a bad password." A genuine non-member still gets 401. The two are already distinguishable in both the log and the response.
  4. A redeploy is deliberate and reviewable; a text box is not. The group is set once and effectively never changes — it is not SMTP configuration.
  5. The console version creates a circular failure. Fixing a lockout would require the console the lockout prevents you from reaching. Editing the env var does not.

What is genuinely lost, and accepted:

  • Discoverability. An app admin cannot see which group is required without Portainer or shell access. A read-only line in the Admin console diagnostics would give the useful half without the dangerous half; it was offered and declined on Aug 24 as not needed.
  • Deploy-time validation. Nothing confirms the group resolves until the first sign-in attempt. This is not fixable: resolving a group needs an authenticated search, anonymous bind is disabled on this estate, and there is no service account by design. The first-attempt 503 is the earliest possible detection.

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 — docs/reference/baseline/before-wave10/ and after-wave10/, captured Aug 24 with tests/baseline_shots.py. "Before" comes from a detached worktree at main (a8e28bf) so each half was shot against its own server.
  • 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 stub backend cannot be selected when a non-SQLite DATABASE_URL is set — asserted by a test
  • tests/ldap_auth_check.py covers the cases above — 20/20
  • the empty-password case is proved by nulling Connection, so any call to bind() would raise — it asserts the guard returns before the transport, not merely that the result is a failure
  • smoketest.py and seed_demo.py document which credentials they now need (T10.8)
  • the removed password-reset checks are called out, not silently dropped — console_dialogs_check.py's docstring records the coverage loss and where the prompt kit is still covered
  • the full tests/ suite passes with no DC reachable — 1284/1288 checks across 40 files, Aug 24. Two files fail, both proven pre-existing and unrelated (BL-028 assets_check, BL-029 generalinfo_check): git diff main...HEAD shows this branch touches neither file. token_check.py is not a member of the suite — it is a capture/diff tool that requires --out or --compare, and a sweep script that runs it bare gets a usage message and exit 2.

Scope note. This task was estimated as far larger than it turned out to be. The premise was that four checks sign in and would all need the seam; in fact seed() mints a token with auth.create_token() and sets the cookie directly, so no browser check signs in except url_state_check's deep-link case. browser_check and launcher_check needed one kwarg deleted each — and since 39 files import seed/start_server from browser_check, that single line unblocked nearly the whole suite.


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 — thumbprints and a Get-ChildItem one-liner in DEPLOYMENT.md
  • 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
  • the DC-unreachable behaviour is stated explicitly, with the diagnostic commands
  • IMPLEMENTATION.md section 4 lists wave 10
  • no doc still claims passwords are stored as bcrypt hashes (swept; remaining matches all say the opposite)
  • 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:

  • create-admin, create and reset-password are rejected as invalid choices
  • list works with no credential and with no LDAP configured
  • a state-changing command with LDAP misconfigured refuses instead of proceeding
  • there is no --password flag on any command
  • promote raises a role; demote returns an account to project_user
  • promoting YOURSELF is allowed and recorded with self: true
  • the last active admin cannot be demoted
  • an unknown account gives an error that says accounts are made on first sign-in
  • every change writes an AuditLog row naming the operator
  • verified against a real domain bind — promote and demote confirmed working Aug 24 2026