n.siegfried/Project-SDE-WP-Suite
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

T10.7 - cover the throttle and local is_active; tick what was verified

Browse Source 
Two things, both found by checking the wave file before pushing rather than
assuming it was current.

First: most done-when boxes were still open even where the work had been
verified, which would have told a reviewer that almost nothing was checked.
Ticked the ones genuinely verified, each with what verified it, and left eight
open that are not. One box was not merely unticked but WRONG - it asked that
`create-admin` create an admin with no password prompt, and T10.9 removed that
command outright; restated as what now has to be true.

Second: two of the open boxes were safety-relevant and cheap to close, so
ldap_auth_check now covers them (24/24):

  a disabled local account is refused 403 even though its bind succeeds -
  local is_active overrides the directory, which is how access to THIS app is
  revoked without touching the domain account

  the throttle stops CALLING the directory, not merely refusing. Proved by
  spending the attempt budget on wrong passwords and then presenting the
  CORRECT one: a 429 for a credential that would otherwise succeed is only
  possible if the check runs before the directory is consulted. Also asserts
  the budget is per-username, so throttling one account does not throttle
  everyone.

That was the last untested safety-critical behaviour on the branch. It is the
thing standing between an unauthenticated caller and locking colleagues out of
Windows, and until now nothing exercised it.

Also recorded a deployment fact at the top of wave-10 where it cannot be missed:
LDAP_REQUIRED_GROUP is empty, and empty means no group gate - every account in
prime.local may sign in. The live sign-in that confirmed this branch works was
made without it, so it proved the bind, the certificate chain and provisioning,
but not the group check. That path has still never run against the real
directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cody Schaefer
2026-08-24 09:42:45 -05:00
parent d261024139
commit 24fff382c8
2 changed files with 91 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
docs/waves/wave-10.md
View File

@@ -12,6 +12,14 @@ with their Windows password instead of an app password, the "Forgot password?" l
disappears, and admins stop issuing passwords. Everything else is invisible — which means
the done-when checks are the only evidence the wave worked.
> **BEFORE PRODUCTION: `LDAP_REQUIRED_GROUP` must be set.** It is empty by default,
> and empty means *no group gate* — every account in `prime.local` may sign in. The
> successful live sign-in on Aug 24 was made without it, so it proved the bind, the
> certificate chain and JIT provisioning, but **not** the group check: that path never
> executed. The group gate is covered against the fake directory in `ldap_auth_check`
> and has never run against the real directory. The startup line says which state you
> are in — `required group: (none configured — every domain account may sign in)`.
**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.
@@ -51,15 +59,15 @@ across resolved addresses, because round-robin will hand out a rebooting DC's ad
**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
- [x] `ldap3` is pinned to an exact version in `requirements.txt`
- [x] an empty or whitespace-only password returns failure **without calling `bind()`** — proven with `Connection` nulled, so any call to `bind()` would raise
- [x] an empty username returns failure without calling `bind()`
- [x] `Tls` is constructed with `validate=ssl.CERT_REQUIRED` and an explicit `ca_certs_file`
- [x] no code path sets `CERT_NONE`, and none falls back to the system trust store — asserted by an AST walk in `ldap_auth_check`, not a grep: the docstring names it to explain the ban
- [ ] `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
- [x] a bind against `192.168.3.37` (raw IP) fails hostname validation rather than silently passing — `selftest()` to the raw IP returns `untrusted`; to `prime.local`, `0 (ok)`
- [ ] `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`)
- [x] the module imports cleanly with no LDAP env set (unconfigured is a first-class state, as with `MICRON_DB_URL`)
---
@@ -89,13 +97,13 @@ misconfigured deploy is indistinguishable from a forgotten password at the login
**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"
- [x] a correct domain password signs in and sets the session cookie — confirmed against the live domain Aug 24, and in `ldap_auth_check`
- [x] a wrong password is refused with the generic message
- [x] a blank password is refused (guards `T10.1` from the caller's side too)
- [x] a user not in the required group is refused even though the bind succeeded
- [x] `is_active = false` locally still refuses, independent of the directory — a disabled account is refused 403 even though the bind succeeds
- [x] the local throttle trips below the domain lockout threshold and stops calling the DC — proved by presenting a CORRECT password once the budget is spent: a 429 for a credential that would otherwise work is only possible if the throttle runs before the directory is consulted
- [x] no response body distinguishes "no such user" from "wrong password"
- [ ] error-49 sub-codes appear in the log and nowhere in any response
---
@@ -131,12 +139,12 @@ 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] `grep -rn "password_hash\|hash_password\|verify_password\|password_problem" server/` returns nothing outside the migration — only the migration and one docstring naming the dropped column
- [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
- [x] the migration's `downgrade()` recreates the column nullable, not `NOT NULL` — there are no hashes to put back — verified on SQLite and Postgres
- [ ] `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
- [x] `manage_users.py list`, `disable`, `enable` still work; `reset-password` is gone
- [x] no CLI command prompts for a password — superseded by `T10.9`, which removed `create-admin` and `create` outright; the first admin is now bootstrapped by signing in and then `promote`
---
@@ -176,15 +184,15 @@ 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
- [x] an unknown username with a valid bind and group membership gets a `users` row at `project_user`
- [x] `full_name` and `email` are populated from the directory on creation
- [x] an existing `admin` signing in is still `admin` afterwards — asserted, not assumed — asserted in `ldap_auth_check` against a real server
- [x] an existing account with a locally-set `full_name` does not have it overwritten
- [x] 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
- [x] each JIT creation writes an `AuditLog` row
- [x] a failed bind creates **no** row
- [x] a bind that succeeds but fails the group check creates **no** row
---
@@ -267,16 +275,16 @@ 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
- [x] `grep -rn "forgot\|reset-password\|new-password" html/` returns nothing but prose
- [x] "Forgot password?" opens `https://primecontrols.okta.com/` in a new tab — by inspection of the markup: `target="_blank"` with `rel="noopener noreferrer"`
- [x] `#forgot-link` has NO click handler (a `preventDefault()` would swallow the navigation)
- [x] a 503 from the login endpoint says sign-in is unavailable, not that the password is wrong
- [x] the sign-in form still submits, and a failure still announces through `role="alert"` (`login.html` already does this correctly — do not regress it) — `url_state_check` drives the real form end to end; the `role="alert"` region is untouched
- [x] the password field says which password to enter — see the screenshots
- [x] no dead `<a href="#">` or handler remains for a removed view
- [ ] granting admin to an existing user still works from the console
- [x] 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)
- [x] no raw hex added to any stylesheet (the token rule) — no CSS was added at all — the hint reuses the `.hint` class the page already had
---

49
tests/ldap_auth_check.py
View File

@@ -214,6 +214,55 @@ def main():
except subprocess.TimeoutExpired:
pass
print("")
print("4. local state overrides the directory, and the throttle protects it")
db_fd3, db3 = tempfile.mkstemp(suffix=".db"); os.close(db_fd3)
import sqlalchemy as _sa0
from server.db import Base as _B0
from server import models as _m0 # noqa: F401
eng0 = _sa0.create_engine("sqlite:///" + db3.replace("\\", "/"))
_B0.metadata.create_all(bind=eng0)
with eng0.begin() as con:
con.execute(_sa0.text(
"insert into users (id,username,email,full_name,role,is_active,"
"failed_attempts,token_version,project_role,locale,timezone,"
"auto_add_projects,auto_add_role,created_at,updated_at) values "
"('user_root','root','','Root','project_user',0,0,0,'','','',0,'',"
"datetime('now'),datetime('now'))")) # is_active = 0
eng0.dispose()
port3 = free_port()
server3 = start(port3, db3, fake, required_group=GROUP)
if server3 is None:
print("the third test server would not start.")
return 2
b3 = f"http://127.0.0.1:{port3}"
try:
st, _ = post(b3, "/api/auth/login", {"username": "root", "password": PW})
chk("a disabled local account is refused even though the bind succeeds",
st == 403, st)
# The throttle. AUTH_MAX_ATTEMPTS is 2, and its whole purpose is that failures
# are real domain binds counting against the AD lockout policy — so it has to
# stop CALLING the directory, not merely refuse. Proving that: burn the budget
# with wrong passwords, then present the CORRECT one. A 429 for a credential
# that would otherwise succeed is only possible if the throttle runs before the
# directory is consulted.
codes = [post(b3, "/api/auth/login",
{"username": "outsider", "password": "wrong"})[0] for _ in range(3)]
chk("the attempt budget is spent and the next try is throttled",
codes[-1] == 429, codes)
st, _ = post(b3, "/api/auth/login", {"username": "outsider", "password": PW})
chk("...and a CORRECT password is still refused while throttled, proving the "
"directory is never reached", st == 429, st)
chk("...while another account is unaffected (the budget is per-username)",
post(b3, "/api/auth/login", {"username": "root", "password": PW})[0] == 403, "")
finally:
server3.kill()
try:
server3.wait(timeout=10)
except subprocess.TimeoutExpired:
pass
print("\n3. an existing admin survives the switch")
db_fd2, db2 = tempfile.mkstemp(suffix=".db"); os.close(db_fd2)
# Build the schema with a NEW engine bound to this file — see users_in().