Reported by a user who is a direct member of the configured group and was being
refused with "Invalid username or password". Get-ADUser confirmed the
membership, so the fault was here.
The connection is built with raise_exceptions=False. A search that FAILS
therefore returns False and leaves conn.entries empty - which is
indistinguishable from "no match" if you only inspect conn.entries, which is
all member_of did. Every possible failure of the extensible-match filter
presented to the user as "you are not in the group" while they plainly were,
and produced no log line saying otherwise.
Now:
- conn.search()'s return value is checked. A failed search raises
LookupError(GROUP_CHECK_FAILED) and logs conn.result together with the filter
that produced it. GROUP_CHECK_FAILED counts as a config problem, so login()
answers 503 rather than 401 - our fault, not the user's, and reported as such.
- If the transitive query matches nothing, a plain memberOf equality check runs
for DIRECT membership. If THAT matches, the person is a member and is let in:
refusing a real member is the worse error. It logs a WARNING naming the
matching rule, because that outcome means nested membership is silently not
working on this connection and needs a human.
The failure mode this replaces is the one that hurts most: correct
configuration, correct credential, real membership, and a refusal that blames
the password. Same shape as the two logging faults fixed just before it - the
information existed and could not be read.
ldap_auth_check still 32/32; the fake directory exercises both branches.
A step-by-step diagnostic (bind, account lookup, memberOf dump, the nested
query, the direct query, group resolution) is in the session scratchpad rather
than the repo - it is a one-off aid, not a deliverable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reason a sign-in was refused was logged at INFO, in app.py and in
ldap_auth. Nothing in this app configures the root logger, and uvicorn
configures only its own - so an INFO record from wpsuite.* reaches no handler
and is discarded. The message existed and could not be read, in exactly the
situation it was written for: someone cannot sign in and the operator needs to
know whether the credential was wrong, the account is outside the required
group, or the group does not resolve.
Raised to WARNING on the three refusal paths:
app.py "sign-in refused for 'x' (not_in_group: not in CN=...)"
ldap_auth "bind refused for 'x': 52e (bad password)"
ldap_auth "bind succeeded for 'x' but the account is NOT in 'CN=...'"
Left at INFO: provisioning an account, and normalising an address to a
sAMAccountName. Those are narrative, not diagnostic.
Config faults were already ERROR and were always visible, which is why the
503 path could be diagnosed and the 401 path could not.
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>
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>
