Caught the moment a real LDAP_REQUIRED_GROUP landed in .env: ldap_auth_check's
"a just-provisioned account appears in the Admin console list" started failing,
consistently, with no code change that could explain it.
start() removed LDAP_REQUIRED_GROUP from the subprocess environment so the test
could run without a group gate. But the server imports server/db.py, which
calls load_dotenv(), and python-dotenv skips only keys ALREADY PRESENT in
os.environ - so a popped variable is helpfully restored from the developer's
.env inside the child process. The test was quietly running against the real
required group, the fake "outsider" account was refused by it, and the account
under test was never provisioned.
Setting the variable to an empty string fixes it: empty still counts as
present, so load_dotenv leaves it alone.
Fixed in both harnesses - ldap_auth_check and browser_check, the latter shared
by 39 files - and corrected the fix suggested in BL-028, which said to pop
MICRON_DB_URL and would therefore not have worked either.
Worth stating as a rule: in this repo a test cannot assume an environment
variable is ABSENT. .env re-supplies it in any subprocess. Force the value you
want; never remove it.
ldap_auth_check 32/32, browser_check re-run green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I had said all six remaining boxes needed the live environment. Four did not,
and saying so was lazy scoping. ldap_auth_check now covers them (32/32):
a just-provisioned account appears in GET /api/auth/users, the request the
Admin console makes - without which nobody could grant a JIT account access
granting admin through POST /api/auth/users/{id}/role, the request users.js
sends, with the role verified to have actually changed. D13 criterion 4 was
until now only asserted for role PRESERVATION, never for role GRANTING.
no AD error-49 sub-code appears in any response body, while _err49 does parse
one out of a real AD message - the log gets the detail, the caller does not
token_version invalidates a cookie already issued, and leaves other sessions
alone
That last box was wrong as written. It asked to exercise token_version "by a
role change", and nothing in app.py bumps it on a role change - or on a
deactivation. Neither needs to: get_current_user re-reads the account from the
database every request, so both take effect on the next request regardless.
T10.3's note that "role changes and deactivation should bump it" described an
intention rather than the code, and I repeated it without checking.
What token_version actually is now: a mechanism whose only trigger is the bump
manage_users makes on disable, which is belt-and-braces since is_active already
refuses the request. It works, and it is tested - but nothing much triggers it.
Logged as BL-030 rather than resolved here, because whether to wire it to
something (a "sign out everywhere" control is the usual shape) or remove it is
a session-handling design question, not an auth-wave bug.
Two boxes remain open, and both genuinely need your environment: member_of
against a real NESTED group, and the in-container openssl certificate check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Far smaller than estimated, because the premise was wrong. I had said four
checks sign in and would each need a fake directory. They do not: seed() mints
a session token with auth.create_token() and sets the cookie directly -
browser_check's own docstring says so - and the only breakage was a leftover
password_hash= kwarg on a model that no longer has the column. Deleting that one
line in browser_check.seed() unblocked 39 files that import seed/start_server
from it. launcher_check needed the same.
console_dialogs_check's password-reset half is deleted rather than ported. Its
docstring now records what went and where the prompt kit is still covered
(wpPromptDialog has five callers left in wp-creation-app.js; creator_dialogs_check
exercises them, validation included - verified, 20/20). Nothing was left skipped
in place of the removed section.
Exactly one check genuinely needed a seam: url_state_check drives the real login
form to prove a deep link's ?next= survives authentication. That cannot be faked
by minting a cookie, because the login round trip is the thing under test.
The seam is env-driven because it has to be: start_server launches the app as a
SUBPROCESS, so a monkeypatch in the test process would never reach the code doing
the authenticating. server/ldap_fake.py reads WP_LDAP_FAKE_DIRECTORY and
ldap_auth dispatches to it AFTER the empty-input guard, so the anonymous-bind
guard covers the fake path too - a fake that reimplemented it would let the real
one rot unnoticed.
The production guard is the point of that module. An env var that makes any
password work is exactly the kind of thing that escapes into production, and D13
left no other way in. is_active() refuses whenever a non-SQLite DATABASE_URL is
configured - the same test auth._load_secret uses - and describe() shouts in
capitals so a fake run can never be mistaken for a real one in the startup log.
Two things found on the way, neither of them the app's fault:
- url_state_check's "signing in continues to the requested page" asserted
`"wp-creation-index.html" in location.href`. That string is in the ?next=
parameter too, so it passed while sitting on login.html with the sign-in
rejected. It would have passed with login entirely broken. Tightened to assert
we actually left the login page.
- Two assertions in my own new ldap_auth_check read the WRONG database:
server/db.py binds its engine from DATABASE_URL at import, so setting the env
var afterwards keeps reading whichever file was configured first. users_in()
now opens the file it is asked about with sqlite3. The CERT_NONE check also had
to become an AST walk - the module docstring names validate=ssl.CERT_NONE in
order to explain why it is banned, and a text search cannot tell that apart
from a real call.
tests/ldap_auth_check.py is new coverage rather than repair: the anonymous-bind
guard, CERT_REQUIRED by AST, the nested matching rule in the filter, the
production refusal, a refused sign-in creating no account, and an existing admin
still being an admin with their locally-set name intact. 20/20.
Run so far, all green: browser_check 71/71, launcher_check 58/58,
console_dialogs 12/12, url_state 23/23, qa_gate 41/41, critical_reopen 11/11,
creator_dialogs 20/20, a11y 22/22, kitting_notify 17/17, ldap_auth 20/20.
A full sweep of the remaining ~30 is running; its box stays unticked until it
reports.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
