From c32888981c67be2492f1e7b4bed66bb1edce0a31 Mon Sep 17 00:00:00 2001
From: Oleg <ogrechko@top-sysops.ru>
Date: Wed, 5 Aug 2026 12:00:52 +0000
Subject: [PATCH] Fix LDAP login hang by routing it through ldapsearch too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reported symptom: logging in with an LDAP account got stuck on
"Входим..." forever. Cause: authenticateLdapUser's user lookup used the
same ldapjs search() that was already proven unreliable against this
AD (see previous commits) — except here the failure mode wasn't an
error, it was the bind/search promise never settling at all (neither
resolve nor reject), so the surrounding try/catch's `return null` never
ran and the login button just spun. Routes the lookup through the same
ldapSearchCli used by searchLdapDirectory, and adds an explicit 15s
execFile timeout so this path — which is now login-critical — can't
hang regardless of what ldapsearch itself does. Also deletes the now
fully-unused ldapjs-based search() helper.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012o9j9RezxbZVKQMrB7oRLY
---
 src/lib/ldap/client.ts | 59 ++++++++----------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/src/lib/ldap/client.ts b/src/lib/ldap/client.ts
index 2280126..f498264 100644
--- a/src/lib/ldap/client.ts
+++ b/src/lib/ldap/client.ts
@@ -58,48 +58,6 @@ function bind(client: ldap.Client, dn: string, password: string): Promise<void>
   });
 }
 
-function search(
-  client: ldap.Client,
-  baseDn: string,
-  filter: string,
-): Promise<{ dn: string; attributes: Record<string, string> }[]> {
-  return new Promise((resolve, reject) => {
-    const results: { dn: string; attributes: Record<string, string> }[] = [];
-    // `attributes` is restricted to just what callers actually read
-    // (mail/cn/displayName) instead of requesting every attribute AD has on
-    // an object. The original failure was @ldapjs/asn1's BER decoder
-    // throwing "Encoding too long" — a parser desync, most likely triggered
-    // while decoding some oversized/exotic AD attribute we never use anyway
-    // (e.g. nTSecurityDescriptor, msDS-ReplAttributeMetaData). Not requesting
-    // them sidesteps that bug entirely.
-    //
-    // Deliberately NOT using RFC 2696 paging here: with the response now
-    // this much smaller, an unpaged single request+response is simpler and
-    // matches what a plain `ldapsearch` against this same AD does — paging
-    // instead caused ldapjs's multiple sequential requests on one connection
-    // to get an ECONNRESET partway through.
-    client.search(
-      baseDn,
-      { filter, scope: "sub", attributes: ["mail", "cn", "displayName"] },
-      (err, res) => {
-        if (err) {
-          reject(describeError(client, err));
-          return;
-        }
-        res.on("searchEntry", (entry) => {
-          const attributes: Record<string, string> = {};
-          for (const attr of entry.pojo.attributes) {
-            attributes[attr.type] = attr.values[0] ?? "";
-          }
-          results.push({ dn: entry.objectName ?? entry.pojo.objectName, attributes });
-        });
-        res.on("error", (searchErr) => reject(describeError(client, searchErr)));
-        res.on("end", () => resolve(results));
-      },
-    );
-  });
-}
-
 function unbindQuietly(client: ldap.Client): void {
   client.unbind(() => {});
 }
@@ -179,7 +137,10 @@ async function ldapSearchCli(
       "cn",
       "displayName",
     ],
-    { maxBuffer: 20 * 1024 * 1024 },
+    // This now sits on the login path too — a hard deadline here backs up
+    // authenticateLdapUser's documented guarantee that a directory outage
+    // must never hang login, regardless of what ldapsearch itself does.
+    { maxBuffer: 20 * 1024 * 1024, timeout: 15_000 },
   );
   return parseLdif(stdout);
 }
@@ -204,16 +165,20 @@ export async function testLdapBind(settings: ConnectionSettings & { bindDn: stri
  * own DN with the supplied password to actually verify it. Every failure
  * mode (not configured, disabled, unreachable, no match, wrong password)
  * normalizes to null — a directory outage must never break local login.
+ *
+ * The lookup uses the same `ldapSearchCli` as `searchLdapDirectory`, not
+ * ldapjs: the ldapjs BER-parser bug that broke the directory browse was
+ * also silently hanging login here — the pending bind/search promise
+ * never resolved *or* rejected, so the outer try/catch's `return null`
+ * never ran and the login button spun forever instead of failing.
  */
 export async function authenticateLdapUser(email: string, password: string): Promise<LdapUserInfo | null> {
   const settings = await getLdapSettings();
   if (!settings) return null;
 
-  const serviceClient = createLdapClient(settings);
   try {
-    await bind(serviceClient, settings.bindDn, settings.bindPassword);
     const filter = settings.userFilter.replace("{{email}}", escapeFilterValue(email));
-    const entries = await search(serviceClient, settings.baseDn, filter);
+    const entries = await ldapSearchCli(settings, settings.baseDn, filter);
 
     const match = entries[0];
     if (!match) return null;
@@ -232,8 +197,6 @@ export async function authenticateLdapUser(email: string, password: string): Pro
     };
   } catch {
     return null;
-  } finally {
-    unbindQuietly(serviceClient);
   }
 }