Просмотр исходного кода

Resolve LDAP groups on lldap and OpenLDAP (#3197)

lldap and OpenLDAP's memberof overlay omit memberOf from "*", so every
lldap login fell through to the default group. Request memberOf by name
when the schema defines it, and on non-AD directories also search the
directory root for groupOfNames/groupOfUniqueNames entries listing the
user, since groups often sit outside the user search base and the
overlay tracks only one group class.

Also: skip ldap3's anonymous schema read after StartTLS, which AD and
Samba AD reject, so StartTLS works there; reword a server's StartTLS
refusal with an LDAPS hint; stop the bundle sanitizer masking part of
an OID as an IP; skip the sync right after auto-provisioning so the
default-group warning logs once.
maziggy 1 день назад
Родитель
Сommit
29e6d28205

+ 1 - 0
CHANGELOG.md

@@ -52,6 +52,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **LDAP group mapping finds groups on lldap and OpenLDAP (#3197, reported by @TOFM)** — Every login from an lldap directory got the Default Group, even for a user who was a member of a mapped group. Bambuddy read membership from the user's `memberOf` attribute but asked the server for "all attributes". lldap, and OpenLDAP's memberof overlay, leave `memberOf` out of "all attributes" and only return it when it is asked for by name. Bambuddy now asks for it by name wherever the directory's schema has it. It also asks the groups themselves which `groupOfNames` and `groupOfUniqueNames` entries list the user, searching from the directory root, because groups usually sit beside the users (`ou=groups` next to `ou=people`) rather than under the search base. That covers OpenLDAP without the memberof overlay, where there is no `memberOf` at all, and OpenLDAP with an overlay that tracks only one group class. Active Directory keeps `memberOf` complete and is not searched the second way. Checked against lldap 0.6.3, OpenLDAP 2.6 with and without the overlay, and Samba AD. Smaller things from the same report: **StartTLS now works with Active Directory and Samba AD**, which it never did: straight after StartTLS, and before logging in, the LDAP library read the server's schema, which those directories only allow once logged in, so every StartTLS connection failed with "operationsError" (LDAPS was unaffected); **Test Connection** against a server that doesn't offer StartTLS (lldap only does LDAPS) now says so and suggests LDAPS, instead of "Unsupported extended operation" and a number; support bundles no longer mask part of a longer dotted number such as that one as an IP address, which had made the message unreadable; and a user's first LDAP login logs the "no mapped groups" warning once instead of twice.
 - **In Spoolman mode, each spool's own size is used, not its filament's (#3194, reported by @worried-networking)** — Spoolman stores how much filament a full spool holds on the spool (**Initial Weight**) and uses the filament's **Weight** only when the spool has none, so one filament can have spools of different sizes. Bambuddy read only the filament, so a 250 g spool of a 1000 g filament showed as 1000 g with the wrong fill level, **Sync Weights from AMS** and AMS-percentage usage tracking charged it four times the real grams, and the AMS hover card's Spoolman fill level was off the same way. All of these, the weigh action and the SpoolBuddy scale now use the spool's own size. Creating a spool writes its **Label Weight** to the spool's Initial Weight, so a 250 g spool of a 1000 g catalogue filament is recorded as 250 g. Editing the Label Weight changes only that spool: it used to change the filament's weight while Spoolman kept the spool's old size, so the two disagreed from then on, and on a filament shared with other spools it linked this spool to a new duplicate filament instead. The spool form's **Cost per kg** is stored as Spoolman's spool **Price**, which Spoolman treats as the price of the whole spool; it is now converted at the spool's size both ways, and print cost divides a spool's own price by the spool's size. A catalogue price on the filament is still divided by the filament's weight. For 1000 g spools nothing changes. Saving a spool without changing its size or price no longer writes either, so a 250.7 g spool stays 250.7 g. Editing a spool that has no weight in Spoolman at all, on the spool or its filament, failed with an error; it now saves, and a Label Weight saved from the spool form becomes the spool's size.
 - **In Spoolman mode, weighing a spool now uses the vendor's empty spool weight (#3195, reported by @worried-networking)** — Spoolman finds a spool's empty weight in three places: the spool's own **Spool Weight**, then the filament's, then the vendor's **Empty Spool Weight**. Its own `/measure` endpoint uses that order. Bambuddy stopped after the filament and used 250 g instead of the vendor's value. So a spool whose tare was set only on its vendor weighed against the wrong core: a Sunlu spool (211.7 g) at 1177 g on the scale was saved as 927 g remaining instead of 965.3 g. The same error was in three places, and each is fixed: the weigh action in the inventory, the SpoolBuddy scale, and the **Empty Spool Weight** shown in the spool form and used for gross weight in the list. All three now share one lookup, so they can't drift apart again. The SpoolBuddy "no spool weight set, using 250 g" warning now appears only when the spool, the filament and the vendor all have no weight. The spool form still sends the empty weight to Spoolman only when you change it, so opening and saving a spool does not copy the vendor's value onto it. Changing a filament's spool weight with **Keep old weight for existing spools** now keeps a vendor-inherited tare too: when the filament had no weight of its own, the spools that inherited the vendor's value get that value, not the new filament weight.
 - **A P1P, P1S or X1E added by discovery is saved as the right printer** — Bambu printers announce themselves with an internal code, and the table that turns it into a model name had the P-series codes shifted: a discovered P1S was saved as a P1P, a P1P as a P1S, and an X1E as a P2S. The same mix-up sat in the backend: a file sliced for a P1P or P1S could say it was sliced for an X1C or X1, the X1E's code checked the P2S firmware line, and the P2S's code was missing. All of them now agree with what the printers and 3MF files actually report. Printers saved under a raw code by an older version also get the right ethernet, rod-type and storage handling. A printer that was already saved with the wrong model keeps it: pick the right one under **Model** in **Edit** from the printer card's menu.

+ 7 - 2
backend/app/api/routes/auth.py

@@ -483,6 +483,7 @@ async def login(raw_request: Request, request: LoginRequest, response: Response,
     user = None
     # Check if LDAP is enabled
     ldap_user = None
+    ldap_user_provisioned = False
     ldap_settings = await _get_ldap_settings(db)
     if ldap_settings:
         try:
@@ -506,10 +507,14 @@ async def login(raw_request: Request, request: LoginRequest, response: Response,
                             # User doesn't exist and auto-provision is off
                             ldap_user = None
                         else:
-                            # Auto-provision LDAP user
+                            # Auto-provision LDAP user. Provisioning already sets the
+                            # email, groups and finance defaults the sync below would,
+                            # so a new user skips it (it also logged the default-group
+                            # warning a second time, #3197).
                             user = await _provision_ldap_user(db, ldap_user, ldap_config)
+                            ldap_user_provisioned = True
 
-                    if user and ldap_user:
+                    if user and ldap_user and not ldap_user_provisioned:
                         # Update email and group mappings on each login
                         await _sync_ldap_user(db, user, ldap_user, ldap_config)
                         # Keep finance defaults idempotently in sync for LDAP users

+ 5 - 2
backend/app/services/diagnostic_snapshot.py

@@ -33,8 +33,11 @@ logger = logging.getLogger(__name__)
 # Mirrors the IPv4 pattern in services.log_reader.sanitize_log_content. Kept as
 # a literal here (not imported) so a refactor of that module's internals can't
 # silently change snapshot sanitization. Skips firmware-version-shaped strings
-# (leading-zero octets like "01.09.01.00") via the [1-9]\d|\d alternations.
-_IPV4_RE = re.compile(r"\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\b")
+# (leading-zero octets like "01.09.01.00") via the [1-9]\d|\d alternations,
+# and four-number runs inside a longer dotted number such as an OID.
+_IPV4_RE = re.compile(
+    r"(?<!\d\.)\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\b(?!\.\d)"
+)
 
 # Per-diagnostic wall-clock cap. Each underlying probe carries its own (smaller)
 # TCP / HTTP timeouts; this is the outer guard so a hung interface or a wedged

+ 144 - 14
backend/app/services/ldap_service.py

@@ -14,7 +14,7 @@ import logging
 from dataclasses import dataclass
 
 from ldap3 import ALL, SUBTREE, Connection, Server, Tls
-from ldap3.core.exceptions import LDAPObjectClassError
+from ldap3.core.exceptions import LDAPException, LDAPObjectClassError, LDAPOperationResult
 
 logger = logging.getLogger(__name__)
 
@@ -102,6 +102,37 @@ def _create_server(config: LDAPConfig) -> Server:
     return Server(config.server_url, use_ssl=use_ssl, tls=tls, get_info=ALL, connect_timeout=10)
 
 
+class LDAPStartTLSRefusedError(Exception):
+    """The server answered the StartTLS request with an error result."""
+
+
+def _start_tls_if_configured(conn: Connection, config: LDAPConfig) -> None:
+    """Upgrade `conn` with StartTLS when the config asks for it.
+
+    A server that doesn't offer StartTLS (lldap, for one, only does LDAPS)
+    rejects the request with a bare "Unsupported extended operation" and the
+    StartTLS OID, which doesn't tell the admin what to change (#3197). Only a
+    rejection by the server is reworded; TLS handshake and certificate failures
+    keep their own messages.
+
+    ldap3 re-reads the server's info and schema straight after StartTLS by
+    default, still unauthenticated. A directory that refuses anonymous
+    searches (Active Directory, Samba AD) answers that read with
+    "operationsError", which failed every StartTLS connection to it and would
+    otherwise be reported as a refused StartTLS here. bind() reads the same
+    info once authenticated, so the early read is skipped.
+    """
+    if config.security != "starttls" or config.server_url.startswith("ldaps://"):
+        return
+    try:
+        conn.start_tls(read_server_info=False)
+    except LDAPOperationResult as e:
+        raise LDAPStartTLSRefusedError(
+            f"the server refused StartTLS ({e.description}). If it only offers LDAPS, "
+            "choose LDAPS and use its ldaps:// URL and port"
+        ) from e
+
+
 def _open_service_connection(config: LDAPConfig, server: Server, *, check_names: bool = True) -> Connection:
     """Open and bind a service-account LDAP connection. Raises on failure.
 
@@ -122,12 +153,107 @@ def _open_service_connection(config: LDAPConfig, server: Server, *, check_names:
         check_names=check_names,
     )
     conn.open()
-    if config.security == "starttls" and not config.server_url.startswith("ldaps://"):
-        conn.start_tls()
+    _start_tls_if_configured(conn, config)
     conn.bind()
     return conn
 
 
+# Group classes that list their members by DN, and the attribute each uses.
+_MEMBER_DN_GROUP_CLASSES = (("groupOfNames", "member"), ("groupOfUniqueNames", "uniqueMember"))
+
+
+def _schema_defines(names, name: str) -> bool | None:
+    """Whether a schema dict (attribute types or object classes) has `name`.
+
+    None when the server published no schema. ldap3 then skips its client-side
+    name checks, so the caller can request the name without it raising.
+    """
+    if not names:
+        return None
+    return name in names
+
+
+def _user_search_attributes(server: Server) -> list[str]:
+    """Attributes to request for a user entry: all user attributes plus memberOf.
+
+    `*` alone does not return memberOf on every directory. OpenLDAP's memberof
+    overlay makes it operational and lldap only returns it when asked by name
+    (#3197), so it has to be listed. But ldap3 rejects a requested name that the
+    server's schema doesn't define before anything is sent, even with
+    check_names off, so it is only listed when the schema has it (or publishes
+    no schema at all).
+    """
+    schema = server.schema
+    has_member_of = _schema_defines(schema.attribute_types if schema else None, "memberOf")
+    return ["*"] if has_member_of is False else ["*", "memberOf"]
+
+
+def _groups_base(server: Server, search_base: str) -> str:
+    """The naming context that contains `search_base`, else `search_base`.
+
+    Groups usually live beside the users rather than under them (ou=groups next
+    to ou=people), so a group search from the user search base finds nothing.
+    """
+    base_lower = search_base.lower()
+    contexts = server.info.naming_contexts if server.info and server.info.naming_contexts else []
+    for context in contexts:
+        context_lower = str(context).lower()
+        if base_lower == context_lower or base_lower.endswith("," + context_lower):
+            return str(context)
+    return search_base
+
+
+# Root DSE capability that Active Directory (and Samba AD) advertises.
+_ACTIVE_DIRECTORY_CAPABILITY = "1.2.840.113556.1.4.800"
+
+
+def _is_active_directory(server: Server) -> bool:
+    info = server.info
+    features = info.supported_features if info and info.supported_features else []
+    return any(feature[0] == _ACTIVE_DIRECTORY_CAPABILITY for feature in features)
+
+
+def _member_dn_groups(service_conn: Connection, config: LDAPConfig, user_dn: str) -> list[str]:
+    """Find groupOfNames / groupOfUniqueNames entries that list `user_dn` as a member.
+
+    memberOf alone is not enough outside Active Directory. Plain OpenLDAP has
+    none without the memberof overlay; with it, the overlay tracks only the
+    group class it was configured for (osixia's image: groupOfUniqueNames, so
+    groupOfNames groups are missing) and only groups changed after it was
+    loaded. The groups themselves are the authority, so they are asked too.
+
+    Active Directory is skipped: it keeps memberOf complete itself, and its
+    groups are objectClass=group, not either class searched here, so the
+    search would cost a subtree walk from the domain root and find nothing.
+
+    Only the classes the schema defines go into the filter, for the same
+    client-side check the POSIX lookup trips over.
+    """
+    schema = service_conn.server.schema
+    object_classes = schema.object_classes if schema else None
+    clauses = [
+        f"(&(objectClass={object_class})({attribute}={_ldap_escape(user_dn)}))"
+        for object_class, attribute in _MEMBER_DN_GROUP_CLASSES
+        if _schema_defines(object_classes, object_class) is not False
+    ]
+    if not clauses:
+        return []
+    search_filter = clauses[0] if len(clauses) == 1 else f"(|{''.join(clauses)})"
+    try:
+        service_conn.search(
+            search_base=_groups_base(service_conn.server, config.search_base),
+            search_filter=search_filter,
+            search_scope=SUBTREE,
+            attributes=["cn"],
+        )
+    except LDAPException as e:
+        # The exception text can carry the user's DN (PII, #2681), so only its
+        # type is logged. The user still logs in, with memberOf and POSIX groups.
+        logger.warning("LDAP group membership lookup failed (%s); mapping without it", type(e).__name__)
+        return []
+    return [str(entry.entry_dn) for entry in service_conn.entries]
+
+
 def _pick_canonical_username(entry, fallback: str) -> str:
     """Prefer sAMAccountName, then uid, then the supplied fallback."""
     if hasattr(entry, "sAMAccountName") and entry.sAMAccountName:
@@ -142,17 +268,23 @@ def _extract_user_info(
 ) -> LDAPUserInfo:
     """Build an LDAPUserInfo from an already-fetched directory entry.
 
-    Collects memberOf groups, POSIX memberUid groups, and the primary
-    gidNumber group; dedups DNs case-insensitively. Uses the supplied
-    service-bound connection to resolve POSIX groups.
+    Collects memberOf groups, the groupOfNames / groupOfUniqueNames entries
+    that list the user (except on Active Directory), POSIX
+    memberUid groups, and the primary gidNumber group; dedups DNs
+    case-insensitively. Uses the supplied service-bound connection for the
+    group searches. The entry must have been fetched with
+    `_user_search_attributes`, or memberOf may be missing from it.
     """
     email = str(user_entry.mail) if hasattr(user_entry, "mail") and user_entry.mail else None
     display_name = (
         str(user_entry.displayName) if hasattr(user_entry, "displayName") and user_entry.displayName else None
     )
 
-    # Collect groups from memberOf attribute (Active Directory / groupOfNames)
+    # Collect groups from the memberOf attribute (Active Directory, lldap,
+    # OpenLDAP with the memberof overlay, 389-DS), then ask the groups too.
     groups = [str(g) for g in user_entry.memberOf] if hasattr(user_entry, "memberOf") and user_entry.memberOf else []
+    if not _is_active_directory(service_conn.server):
+        groups.extend(_member_dn_groups(service_conn, config, str(user_entry.entry_dn)))
 
     canonical_username = _pick_canonical_username(user_entry, fallback_username)
 
@@ -202,7 +334,7 @@ def _extract_user_info(
         # error the operator can or should act on.
         logger.info(
             "Directory publishes no posixGroup object class; skipping POSIX group lookup "
-            "(memberOf groups are unaffected)"
+            "(memberOf and groupOfNames groups are unaffected)"
         )
 
     # Dedupe group DNs (user may be in a group via both memberUid and primary gidNumber).
@@ -250,7 +382,7 @@ def authenticate_ldap_user(config: LDAPConfig, username: str, password: str) ->
             search_base=config.search_base,
             search_filter=search_filter,
             search_scope=SUBTREE,
-            attributes=["*"],
+            attributes=_user_search_attributes(server),
         )
 
         if not service_conn.entries:
@@ -271,8 +403,7 @@ def authenticate_ldap_user(config: LDAPConfig, username: str, password: str) ->
                 read_only=True,
             )
             user_conn.open()
-            if config.security == "starttls" and not config.server_url.startswith("ldaps://"):
-                user_conn.start_tls()
+            _start_tls_if_configured(user_conn, config)
             user_conn.bind()
             user_conn.unbind()
         except Exception as e:
@@ -320,7 +451,7 @@ def lookup_ldap_user(config: LDAPConfig, username: str) -> LDAPUserInfo | None:
             search_base=config.search_base,
             search_filter=search_filter,
             search_scope=SUBTREE,
-            attributes=["*"],
+            attributes=_user_search_attributes(server),
         )
         if not service_conn.entries:
             logger.info("LDAP lookup: user not found: %s", username)
@@ -434,8 +565,7 @@ def test_ldap_connection(config: LDAPConfig) -> tuple[bool, str]:
             read_only=True,
         )
         conn.open()
-        if config.security == "starttls" and not config.server_url.startswith("ldaps://"):
-            conn.start_tls()
+        _start_tls_if_configured(conn, config)
         conn.bind()
 
         # Try a search to verify search base

+ 4 - 2
backend/app/services/log_reader.py

@@ -184,9 +184,11 @@ def sanitize_log_content(content: str, sensitive_strings: dict[str, str] | None
     # Replace Bambu Lab printer serial numbers (format: 00M/01D/01S/01P/03W + alphanumeric, 12-16 chars total)
     content = re.sub(r"\b0[0-3][A-Z0-9][A-Z0-9]{9,13}\b", "[SERIAL]", content, flags=re.IGNORECASE)
 
-    # Replace IPv4 addresses (skip firmware versions like 01.09.01.00 which have leading zeros)
+    # Replace IPv4 addresses (skip firmware versions like 01.09.01.00 which have leading zeros,
+    # and four-number runs inside a longer dotted number, such as the LDAP StartTLS OID
+    # 1.3.6.1.4.1.1466.20037, whose masked "[IP].4.1.1466.20037" hid what failed, #3197)
     content = re.sub(
-        r"\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\b",
+        r"(?<!\d\.)\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\b(?!\.\d)",
         "[IP]",
         content,
     )

+ 70 - 0
backend/tests/integration/test_ldap_provision.py

@@ -461,3 +461,73 @@ class TestLdapLoginFinanceDefaults:
         ).scalar_one_or_none()
         assert membership is not None
         assert membership.can_print is True
+
+
+class TestLdapFirstLoginProvisionsOnce:
+    """Auto-provisioning on login used to be followed straight away by the sync
+    meant for returning users, which repeated what provisioning had just done and
+    logged the default-group warning twice (#3197)."""
+
+    async def _login(
+        self, async_client: AsyncClient, db_session: AsyncSession, groups: list[str], default_group: str = "Viewers"
+    ):
+        await async_client.post(
+            "/api/v1/auth/setup",
+            json={"auth_enabled": True, "admin_username": "ldapadmin", "admin_password": "AdminPass1!"},
+        )
+        await _seed_ldap_settings(
+            db_session,
+            ldap_auto_provision="true",
+            ldap_default_group=default_group,
+            ldap_group_mapping='{"cn=bambuddy-admins,ou=groups,dc=test,dc=com": "Administrators"}',
+        )
+        fake_ldap = LDAPUserInfo(username="tofm", email="tofm@test.com", display_name=None, groups=groups)
+        with patch("backend.app.services.ldap_service.authenticate_ldap_user", return_value=fake_ldap):
+            return await async_client.post("/api/v1/auth/login", json={"username": "tofm", "password": "irrelevant"})
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_default_group_warning_is_logged_once(
+        self, async_client: AsyncClient, db_session: AsyncSession, caplog
+    ):
+        with caplog.at_level("WARNING", logger="backend.app.api.routes.auth"):
+            response = await self._login(async_client, db_session, groups=[])
+
+        assert response.status_code == 200
+        assert {g["name"] for g in response.json()["user"]["groups"]} == {"Viewers"}
+        assert caplog.text.count("has no mapped groups") == 1
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_new_user_gets_mapped_group_and_finance_defaults(
+        self, async_client: AsyncClient, db_session: AsyncSession
+    ):
+        response = await self._login(async_client, db_session, groups=["CN=Bambuddy-Admins,OU=Groups,DC=Test,DC=Com"])
+
+        assert response.status_code == 200
+        body = response.json()["user"]
+        assert body["auth_source"] == "ldap"
+        assert body["email"] == "tofm@test.com"
+        assert {g["name"] for g in body["groups"]} == {"Administrators"}
+        user = (await db_session.execute(select(User).where(User.username == "tofm"))).scalar_one()
+        wallet = (
+            await db_session.execute(select(UserWallet).where(UserWallet.user_id == user.id))
+        ).scalar_one_or_none()
+        assert wallet is not None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_new_user_with_no_groups_at_all_logs_in_twice(
+        self, async_client: AsyncClient, db_session: AsyncSession
+    ):
+        """No mapped group and no default group: the new user has no groups, and
+        the second login (the sync path) still works."""
+        first = await self._login(async_client, db_session, groups=[], default_group="")
+        assert first.status_code == 200
+        assert first.json()["user"]["groups"] == []
+
+        fake_ldap = LDAPUserInfo(username="tofm", email="new@test.com", display_name=None, groups=[])
+        with patch("backend.app.services.ldap_service.authenticate_ldap_user", return_value=fake_ldap):
+            second = await async_client.post("/api/v1/auth/login", json={"username": "tofm", "password": "irrelevant"})
+        assert second.status_code == 200
+        assert second.json()["user"]["email"] == "new@test.com"

+ 353 - 5
backend/tests/unit/services/test_ldap_service.py

@@ -10,8 +10,16 @@ Network-dependent functions (authenticate_ldap_user, test_ldap_connection)
 are not tested here — they require a live LDAP server.
 """
 
+from types import SimpleNamespace
+
 import pytest
-from ldap3.core.exceptions import LDAPObjectClassError
+from ldap3.core.exceptions import (
+    LDAPObjectClassError,
+    LDAPSocketOpenError,
+    LDAPStartTLSError,
+    LDAPUnwillingToPerformResult,
+)
+from ldap3.utils.ciDict import CaseInsensitiveDict
 
 from backend.app.services.ldap_service import (
     LDAPConfig,
@@ -23,6 +31,7 @@ from backend.app.services.ldap_service import (
     parse_ldap_config,
     resolve_group_mapping,
     search_ldap_users,
+    test_ldap_connection as check_ldap_connection,
 )
 
 
@@ -289,6 +298,29 @@ class _MockEntry:
             setattr(self, key, _MockAttr(val))
 
 
+class _MockServer:
+    """Stand-in for ldap3 Server: only the schema and root DSE info the service reads.
+
+    `schema` None is a server that published no schema, where ldap3 checks no
+    names client-side. Otherwise it carries the attribute types and object
+    classes the server defines.
+    """
+
+    def __init__(self, attribute_types=None, object_classes=None, naming_contexts=None, active_directory=False):
+        if attribute_types is None and object_classes is None:
+            self.schema = None
+        else:
+            self.schema = SimpleNamespace(
+                attribute_types=CaseInsensitiveDict(dict.fromkeys(attribute_types or ())),
+                object_classes=CaseInsensitiveDict(dict.fromkeys(object_classes or ())),
+            )
+        features = [("1.2.840.113556.1.4.800", "FEATURE", "Active directory", "MICROSOFT")] if active_directory else []
+        if naming_contexts is None and not active_directory:
+            self.info = None
+        else:
+            self.info = SimpleNamespace(naming_contexts=naming_contexts, supported_features=features)
+
+
 class _MockConnection:
     """Mock ldap3 Connection that returns pre-configured entries based on filter substring match.
 
@@ -304,17 +336,20 @@ class _MockConnection:
     # before the request is ever built (#2769).
     _raise_object_class_error_on: str | None = None
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, server=None, *args, **kwargs):
+        self.server = server
         self.entries: list = []
         self.search_calls: list[str] = []
+        self.search_bases: list[str | None] = []
+        self.search_attrs: list[list | None] = []
         self.last_attrs: list | None = None
         _MockConnection._instances.append(self)
 
     def open(self):
         pass
 
-    def start_tls(self):
-        pass
+    def start_tls(self, read_server_info=True):
+        self.start_tls_read_server_info = read_server_info
 
     def bind(self):
         return True
@@ -325,7 +360,9 @@ class _MockConnection:
     def search(self, search_base=None, search_filter=None, search_scope=None, attributes=None, **kwargs):
         # **kwargs absorbs ldap3 options like size_limit that the real client supports
         self.search_calls.append(search_filter or "")
+        self.search_bases.append(search_base)
         self.last_attrs = list(attributes) if attributes is not None else None
+        self.search_attrs.append(self.last_attrs)
         needle = _MockConnection._raise_object_class_error_on
         if needle and needle in (search_filter or ""):
             raise LDAPObjectClassError(f"invalid class in objectClass attribute: {needle}")
@@ -343,8 +380,11 @@ def mock_ldap(monkeypatch):
     _MockConnection._search_fixture = {}
     _MockConnection._instances = []
     _MockConnection._raise_object_class_error_on = None
+    _MockConnection.server_fixture = _MockServer()
     monkeypatch.setattr("backend.app.services.ldap_service.Connection", _MockConnection)
-    monkeypatch.setattr("backend.app.services.ldap_service._create_server", lambda config: None)
+    monkeypatch.setattr(
+        "backend.app.services.ldap_service._create_server", lambda config: _MockConnection.server_fixture
+    )
     return _MockConnection
 
 
@@ -704,3 +744,311 @@ class TestLookupLdapUser:
 
         with pytest.raises(RuntimeError):
             lookup_ldap_user(_base_config(), "anyone")
+
+
+# ---------------------------------------------------------------------------
+# Group membership on directories where `*` doesn't return memberOf (#3197)
+# ---------------------------------------------------------------------------
+
+_USER_DN = "uid=tofm,ou=people,dc=example,dc=com"
+_ADMINS_DN = "cn=bambuddy-admins,ou=groups,dc=example,dc=com"
+_PEOPLE_BASE = "ou=people,dc=example,dc=com"
+
+
+def _user_search_attrs(conn: _MockConnection) -> list | None:
+    """The attribute list sent with the user search (the first search on the service connection)."""
+    return conn.search_attrs[0]
+
+
+def _member_searches(conn: _MockConnection) -> list[tuple[str, str | None]]:
+    return [
+        (flt, base)
+        for flt, base in zip(conn.search_calls, conn.search_bases, strict=True)
+        if "(member=" in flt or "(uniqueMember=" in flt
+    ]
+
+
+class TestMemberOfIsRequestedByName:
+    """lldap fills in memberOf only when it is asked for by name, and OpenLDAP's
+    memberof overlay makes it operational, so `*` alone returns no groups. The
+    reporter's lldap user was a member of a mapped group and always got the
+    default group instead."""
+
+    def test_login_asks_for_memberof_when_the_schema_has_it(self, mock_ldap):
+        mock_ldap.server_fixture = _MockServer(
+            attribute_types=["uid", "memberOf"], object_classes=["groupOfUniqueNames"]
+        )
+        user_entry = _MockEntry(_USER_DN, uid="tofm", memberOf=[_ADMINS_DN])
+        mock_ldap._search_fixture = {"(uid=tofm)": [user_entry]}
+
+        info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        service_conn = _MockConnection._instances[0]
+        assert _user_search_attrs(service_conn) == ["*", "memberOf"]
+        assert info.groups == [_ADMINS_DN]
+
+    def test_schema_match_is_case_insensitive(self, mock_ldap):
+        """Schemas spell it memberof, memberOf or MemberOf; ldap3's schema dict ignores case."""
+        mock_ldap.server_fixture = _MockServer(attribute_types=["memberof"], object_classes=[])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(), "tofm", "password")
+
+        assert _user_search_attrs(_MockConnection._instances[0]) == ["*", "memberOf"]
+
+    def test_groups_are_asked_as_well_as_memberof(self, mock_ldap):
+        """OpenLDAP's memberof overlay tracks only the group class it was set up
+        for (osixia's image: groupOfUniqueNames), so a groupOfNames group is
+        missing from memberOf even though the schema has the attribute."""
+        mock_ldap.server_fixture = _MockServer(
+            attribute_types=["memberOf"],
+            object_classes=["groupOfNames", "groupOfUniqueNames"],
+            naming_contexts=["dc=example,dc=com"],
+        )
+        operators_dn = "cn=bambuddy-operators,ou=groups,dc=example,dc=com"
+        mock_ldap._search_fixture = {
+            "(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm", memberOf=[operators_dn])],
+            f"(member={_USER_DN})": [_MockEntry(_ADMINS_DN), _MockEntry(operators_dn)],
+        }
+
+        info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        assert info.groups == [operators_dn, _ADMINS_DN]
+
+    def test_no_group_side_search_on_active_directory(self, mock_ldap):
+        """AD keeps memberOf complete, and its groups are objectClass=group, so a
+        subtree search from the domain root would find nothing."""
+        mock_ldap.server_fixture = _MockServer(
+            attribute_types=["memberOf", "member"],
+            object_classes=["group", "groupOfNames"],
+            naming_contexts=["dc=example,dc=com"],
+            active_directory=True,
+        )
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm", memberOf=[_ADMINS_DN])]}
+
+        info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        assert info.groups == [_ADMINS_DN]
+        assert _member_searches(_MockConnection._instances[0]) == []
+
+    def test_admin_lookup_asks_for_memberof_too(self, mock_ldap):
+        mock_ldap.server_fixture = _MockServer(attribute_types=["memberOf"], object_classes=[])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm", memberOf=[_ADMINS_DN])]}
+
+        info = lookup_ldap_user(_base_config(), "tofm")
+
+        assert _user_search_attrs(_MockConnection._instances[0]) == ["*", "memberOf"]
+        assert info.groups == [_ADMINS_DN]
+
+    def test_memberof_not_requested_when_the_schema_lacks_it(self, mock_ldap):
+        """ldap3 rejects a requested attribute the schema doesn't define before
+        sending anything, even with check_names off. Asking anyway would make
+        every login on such a directory fail."""
+        mock_ldap.server_fixture = _MockServer(attribute_types=["uid", "cn"], object_classes=[])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(), "tofm", "password")
+
+        assert _user_search_attrs(_MockConnection._instances[0]) == ["*"]
+
+    def test_memberof_requested_when_the_server_publishes_no_schema(self, mock_ldap):
+        """Without a schema ldap3 checks no names, and the server ignores an
+        attribute it doesn't know."""
+        mock_ldap.server_fixture = _MockServer()
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(), "tofm", "password")
+
+        assert _user_search_attrs(_MockConnection._instances[0]) == ["*", "memberOf"]
+
+
+class TestGroupsListingTheUserByDn:
+    """Group membership asked from the group side. Plain OpenLDAP without the
+    memberof overlay can answer it no other way."""
+
+    def _server(self, object_classes=("groupOfNames", "groupOfUniqueNames"), naming_contexts=("dc=example,dc=com",)):
+        return _MockServer(
+            attribute_types=["uid", "cn", "member", "uniqueMember"],
+            object_classes=list(object_classes),
+            naming_contexts=list(naming_contexts),
+        )
+
+    def test_finds_a_group_outside_the_user_search_base(self, mock_ldap):
+        """The reporter's layout: users under ou=people, groups under ou=groups.
+        The group search starts at the naming context, not the user search base."""
+        mock_ldap.server_fixture = self._server()
+        mock_ldap._search_fixture = {
+            "(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")],
+            f"(member={_USER_DN})": [_MockEntry(_ADMINS_DN)],
+        }
+
+        info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        assert info.groups == [_ADMINS_DN]
+        searches = _member_searches(_MockConnection._instances[0])
+        assert len(searches) == 1
+        flt, base = searches[0]
+        assert base == "dc=example,dc=com"
+        assert flt == (
+            f"(|(&(objectClass=groupOfNames)(member={_USER_DN}))"
+            f"(&(objectClass=groupOfUniqueNames)(uniqueMember={_USER_DN})))"
+        )
+
+    def test_only_classes_the_schema_defines_go_into_the_filter(self, mock_ldap):
+        """Naming an undefined class raises client-side, the #2769 failure."""
+        mock_ldap.server_fixture = self._server(object_classes=["groupOfUniqueNames"])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        (flt, _base) = _member_searches(_MockConnection._instances[0])[0]
+        assert flt == f"(&(objectClass=groupOfUniqueNames)(uniqueMember={_USER_DN}))"
+
+    def test_no_search_when_the_schema_has_neither_class(self, mock_ldap):
+        mock_ldap.server_fixture = self._server(object_classes=["posixGroup"])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        info = authenticate_ldap_user(_base_config(), "tofm", "password")
+
+        assert info.groups == []
+        assert _member_searches(_MockConnection._instances[0]) == []
+
+    def test_dn_is_escaped_in_the_filter(self, mock_ldap):
+        """A DN may carry filter metacharacters (an escaped comma, a parenthesis)."""
+        dn = r"cn=Doe\, John (ops),ou=people,dc=example,dc=com"
+        mock_ldap.server_fixture = self._server(object_classes=["groupOfNames"])
+        mock_ldap._search_fixture = {"(uid=jdoe)": [_MockEntry(dn, uid="jdoe")]}
+
+        authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "jdoe", "password")
+
+        (flt, _base) = _member_searches(_MockConnection._instances[0])[0]
+        assert flt == r"(&(objectClass=groupOfNames)(member=cn=Doe\5c, John \28ops\29,ou=people,dc=example,dc=com))"
+
+    def test_search_base_is_used_when_no_naming_context_contains_it(self, mock_ldap):
+        mock_ldap.server_fixture = self._server(naming_contexts=["dc=other,dc=org"])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        (_flt, base) = _member_searches(_MockConnection._instances[0])[0]
+        assert base == _PEOPLE_BASE
+
+    def test_naming_context_match_is_on_whole_components(self, mock_ldap):
+        """dc=ample,dc=com is not a suffix of ou=people,dc=example,dc=com."""
+        mock_ldap.server_fixture = self._server(naming_contexts=["dc=ample,dc=com", "DC=Example,DC=Com"])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+
+        authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        (_flt, base) = _member_searches(_MockConnection._instances[0])[0]
+        assert base == "DC=Example,DC=Com"
+
+    def test_dedupes_against_posix_groups(self, mock_ldap):
+        """A group can be both a groupOfNames and a posixGroup (OpenLDAP rfc2307bis)."""
+        mock_ldap.server_fixture = self._server(object_classes=["groupOfNames", "posixGroup"])
+        mock_ldap._search_fixture = {
+            "(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")],
+            f"(member={_USER_DN})": [_MockEntry(_ADMINS_DN)],
+            "memberUid=tofm": [_MockEntry(_ADMINS_DN.upper())],
+        }
+
+        info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+
+        assert info.groups == [_ADMINS_DN]
+
+    def test_a_failed_group_search_still_logs_the_user_in(self, mock_ldap, caplog):
+        """The user gets the groups found by other means, and the log names the
+        failure without the DN it may contain (#2681)."""
+
+        class _GroupSearchFails(_MockConnection):
+            def search(self, search_base=None, search_filter=None, **kwargs):
+                if "(member=" in (search_filter or ""):
+                    raise LDAPObjectClassError(f"size limit on {_USER_DN}")
+                return super().search(search_base=search_base, search_filter=search_filter, **kwargs)
+
+        import backend.app.services.ldap_service as ldap_service
+
+        mock_ldap.server_fixture = self._server(object_classes=["groupOfNames"])
+        mock_ldap._search_fixture = {"(uid=tofm)": [_MockEntry(_USER_DN, uid="tofm")]}
+        original = ldap_service.Connection
+        ldap_service.Connection = _GroupSearchFails
+        try:
+            with caplog.at_level("WARNING", logger="backend.app.services.ldap_service"):
+                info = authenticate_ldap_user(_base_config(search_base=_PEOPLE_BASE), "tofm", "password")
+        finally:
+            ldap_service.Connection = original
+
+        assert info is not None
+        assert info.groups == []
+        assert "LDAP group membership lookup failed (LDAPObjectClassError)" in caplog.text
+        assert _USER_DN not in caplog.text
+
+
+class TestStartTlsRefused:
+    """lldap offers LDAPS only. Its answer to StartTLS is "Unsupported extended
+    operation" plus the StartTLS OID, which the reporter had to decode."""
+
+    def _refusing(self, error):
+        class _Conn(_MockConnection):
+            def start_tls(self, read_server_info=True):
+                raise error
+
+        return _Conn
+
+    def test_connection_test_says_what_to_change(self, mock_ldap, monkeypatch):
+        refusal = LDAPUnwillingToPerformResult(
+            result=53,
+            description="unwillingToPerform",
+            message="Unsupported extended operation: 1.3.6.1.4.1.1466.20037",
+            response_type="extendedResp",
+        )
+        monkeypatch.setattr("backend.app.services.ldap_service.Connection", self._refusing(refusal))
+
+        ok, message = check_ldap_connection(_base_config(server_url="ldap://lldap:3890", security="starttls"))
+
+        assert ok is False
+        assert message == (
+            "LDAP connection failed: the server refused StartTLS (unwillingToPerform). "
+            "If it only offers LDAPS, choose LDAPS and use its ldaps:// URL and port"
+        )
+
+    def test_login_with_refused_starttls_fails_cleanly(self, mock_ldap, monkeypatch):
+        refusal = LDAPUnwillingToPerformResult(result=53, description="unwillingToPerform")
+        monkeypatch.setattr("backend.app.services.ldap_service.Connection", self._refusing(refusal))
+
+        assert authenticate_ldap_user(_base_config(server_url="ldap://x", security="starttls"), "u", "p") is None
+
+    @pytest.mark.parametrize(
+        "error",
+        [LDAPStartTLSError("wrap socket error: certificate verify failed"), LDAPSocketOpenError("reset")],
+    )
+    def test_tls_failures_keep_their_own_message(self, mock_ldap, monkeypatch, error):
+        """Only a refusal by the server is reworded; a certificate problem is not a missing feature."""
+        monkeypatch.setattr("backend.app.services.ldap_service.Connection", self._refusing(error))
+
+        ok, message = check_ldap_connection(_base_config(server_url="ldap://x", security="starttls"))
+
+        assert ok is False
+        assert message == f"LDAP connection failed: {error}"
+        assert "refused StartTLS" not in message
+
+    def test_ldaps_never_sends_starttls(self, mock_ldap, monkeypatch):
+        refusal = LDAPUnwillingToPerformResult(result=53, description="unwillingToPerform")
+        monkeypatch.setattr("backend.app.services.ldap_service.Connection", self._refusing(refusal))
+
+        ok, _message = check_ldap_connection(_base_config(server_url="ldaps://x:636", security="starttls"))
+
+        assert ok is True
+
+    def test_server_info_is_not_read_before_the_bind(self, mock_ldap):
+        """ldap3's default re-reads the schema right after StartTLS, before any
+        bind; Active Directory and Samba AD refuse that anonymous read with
+        operationsError, so StartTLS never worked against them. bind() reads it
+        once authenticated."""
+        mock_ldap._search_fixture = {"(uid=u)": [_MockEntry("uid=u,dc=test,dc=com", uid="u")]}
+
+        authenticate_ldap_user(_base_config(server_url="ldap://ad:389", security="starttls"), "u", "p")
+
+        service_conn, user_conn = _MockConnection._instances
+        assert service_conn.start_tls_read_server_info is False
+        assert user_conn.start_tls_read_server_info is False

+ 38 - 0
backend/tests/unit/test_support_helpers.py

@@ -269,6 +269,44 @@ class TestSanitizeLogContent:
         assert "01.07.02.00" in result
         assert "[IP] running firmware 01.07.02.00" in result
 
+    def test_oid_is_not_masked_as_an_ip(self):
+        """A longer dotted number is not an address. Masking its first four parts
+        turned lldap's StartTLS refusal into "[IP].4.1.1466.20037" (#3197)."""
+        from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content
+
+        content = "Unsupported extended operation: 1.3.6.1.4.1.1466.20037 - extendedResp"
+        assert _sanitize_log_content(content) == content
+
+    @pytest.mark.parametrize(
+        ("content", "expected"),
+        [
+            ("Connected to 192.168.1.5.", "Connected to [IP]."),
+            ("(10.0.0.1)", "([IP])"),
+            ("ftp://10.0.0.1:990/cache", "ftp://[IP]:990/cache"),
+            ("peers 10.0.0.1,10.0.0.2", "peers [IP],[IP]"),
+            ("[IP]:54054 via 172.16.0.9:443", "[IP]:54054 via [IP]:443"),
+        ],
+    )
+    def test_ips_are_still_masked_next_to_punctuation(self, content, expected):
+        from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content
+
+        assert _sanitize_log_content(content) == expected
+
+    @pytest.mark.parametrize(
+        "content",
+        [
+            "at 192.168.1.5 and 1.3.6.1.4.1.1466.20037",
+            "firmware 01.07.02.00 on 10.1.2.3.",
+            "Connected to 192.168.1.5.",
+        ],
+    )
+    def test_diagnostic_snapshot_masks_the_same_way(self, content):
+        """diagnostic_snapshot keeps its own copy of the pattern; the two must agree."""
+        from backend.app.services.diagnostic_snapshot import _IPV4_RE
+        from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content
+
+        assert _IPV4_RE.sub("[IP]", content) == _sanitize_log_content(content)
+
     def test_printer_ip_from_sensitive_strings(self):
         """Printer IPs in sensitive_strings are replaced before regex pass."""
         from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content