member_of could not tell a failed search from a non-member

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>

server/ldap_auth.py

@@ -117,6 +117,7 @@ BAD_CREDENTIALS = "bad_credentials"
 NOT_IN_GROUP = "not_in_group"
 NO_DIRECTORY_ENTRY = "no_directory_entry"
 GROUP_NOT_FOUND = "group_not_found"
+GROUP_CHECK_FAILED = "group_check_failed"
 
 # AD returns these as `data <code>` inside an error-49 message. Kept for the log
 # only: telling an unauthenticated caller "your password expired" confirms the
@@ -156,7 +157,8 @@ class LdapResult:
         """True when the failure is ours, not the user's. With no local password
         fallback (D13) these must be logged as errors and surfaced to an operator —
         otherwise a broken deploy is indistinguishable from a forgotten password."""
-        return self.reason in (UNCONFIGURED, UNREACHABLE, UNTRUSTED, GROUP_NOT_FOUND)
+        return self.reason in (UNCONFIGURED, UNREACHABLE, UNTRUSTED, GROUP_NOT_FOUND,
+                               GROUP_CHECK_FAILED)
 
 
 def base_dn(domain: Optional[str] = None) -> str:
@@ -268,18 +270,58 @@ def _resolve_group_dn(conn, group: str) -> Optional[str]:
 
 
 def member_of(conn, sam: str, group: str) -> bool:
-    """Nested-group-aware membership test for an already-bound connection."""
+    """Is `sam` in `group`, counting nested membership?
+
+    THE RETURN VALUE OF conn.search() IS NOT OPTIONAL READING. The connection is
+    built with raise_exceptions=False, so a search that FAILS returns False and
+    leaves conn.entries empty — which is byte-for-byte indistinguishable from "no
+    match" if you only look at conn.entries. An earlier version of this function did
+    exactly that, and every failure of the extensible-match filter presented to the
+    user as "you are not in the group" while they plainly were.
+
+    Two searches, in order, and the second one exists to catch the first being wrong:
+
+      1. AD's transitive matching rule (LDAP_MATCHING_RULE_IN_CHAIN). This is the
+         correct query — it walks nested groups, which plain memberOf does not.
+      2. If that matches nothing, a plain memberOf equality check for DIRECT
+         membership.
+
+    If (2) matches after (1) did not, the person IS a member and is let in — refusing
+    a real member is the worse error — but it is logged as a WARNING, because it means
+    the transitive rule is returning nothing and NESTED membership is silently not
+    working on this connection. That needs a human; it must not pass unnoticed.
+    """
     dn = _resolve_group_dn(conn, group)
     if not dn:
         log.error("required group %r does not resolve in %s — refusing the sign-in. "
                   "This is a configuration fault, not a bad password.", group, base_dn())
         raise LookupError(GROUP_NOT_FOUND)
-    filt = (f"(&(sAMAccountName={escape_filter_chars(sam)})"
-            f"(memberOf:{NESTED_MEMBER_RULE}:={escape_filter_chars(dn)}))")
-    conn.search(base_dn(), filt, search_scope=SUBTREE,
+
+    esc_sam, esc_dn = escape_filter_chars(sam), escape_filter_chars(dn)
+
+    def _search(filt: str, label: str):
+        ok = conn.search(base_dn(), filt, search_scope=SUBTREE,
                          attributes=["sAMAccountName"], size_limit=1)
+        if not ok:
+            log.error("the %s membership search FAILED (not 'no match') for %r: %s | "
+                      "filter=%s", label, sam, conn.result, filt)
+            raise LookupError(GROUP_CHECK_FAILED)
         return bool(conn.entries)
 
+    if _search(f"(&(sAMAccountName={esc_sam})(memberOf:{NESTED_MEMBER_RULE}:={esc_dn}))",
+               "nested"):
+        return True
+
+    if _search(f"(&(sAMAccountName={esc_sam})(memberOf={esc_dn}))", "direct"):
+        log.warning(
+            "%r IS a direct member of %r, but AD's transitive matching rule "
+            "(%s) returned nothing for them. Allowing the sign-in — refusing a real "
+            "member is worse — but NESTED group membership is not working on this "
+            "connection and needs investigating.", sam, dn, NESTED_MEMBER_RULE)
+        return True
+
+    return False
+
 
 def verify(username: str, password: str, required_group: Optional[str] = None) -> LdapResult:
     """Authenticate against the domain. The only entry point the app should call.
@@ -354,8 +396,9 @@ def verify(username: str, password: str, required_group: Optional[str] = None) -
                         log.warning("bind succeeded for %r but the account is NOT in %r",
                                     sam, group)
                         return LdapResult(False, NOT_IN_GROUP, f"not in {group}")
-                except LookupError:
-                    return LdapResult(False, GROUP_NOT_FOUND, f"group {group!r} not found")
+                except LookupError as exc:
+                    reason = str(exc) or GROUP_NOT_FOUND
+                    return LdapResult(False, reason, f"group {group!r}: {reason}")
 
             return LdapResult(
                 True, OK,