Fix LDAP login hang by routing it through ldapsearch too
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
This commit is contained in:
1 parent
b8ca595d34
commit
c32888981c
1 file changed
+11
-48
+11
-48
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user