fix(Ldap Node): Fix issue with connections not closing correctly (#7432)

n8n-io · Oct 13, 2023 · c3f0be8 · c3f0be8
1 parent bd5ea45
commit c3f0be8
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 6 deletions.
diff --git a/packages/nodes-base/credentials/Ldap.credentials.ts b/packages/nodes-base/credentials/Ldap.credentials.ts
@@ -31,7 +31,6 @@ export class Ldap implements ICredentialType {
 			type: 'string',
 			default: '',
 			description: 'Distinguished Name of the user to connect as',
-			required: true,
 		},
 		{
 			displayName: 'Binding Password',
@@ -42,7 +41,6 @@ export class Ldap implements ICredentialType {
 			},
 			default: '',
 			description: 'Password of the user provided in the Binding DN field above',
-			required: true,
 		},
 		{
 			displayName: 'Connection Security',
@@ -90,5 +88,12 @@ export class Ldap implements ICredentialType {
 			type: 'string',
 			default: '',
 		},
+		{
+			displayName: 'Timeout',
+			description: 'Optional connection timeout in seconds',
+			name: 'timeout',
+			type: 'number',
+			default: 300,
+		},
 	];
 }
diff --git a/packages/nodes-base/nodes/Ldap/Helpers.ts b/packages/nodes-base/nodes/Ldap/Helpers.ts
@@ -39,6 +39,11 @@ export async function createLdapClient(
 		}
 	}
 
+	if (credentials.timeout) {
+		// Convert seconds to milliseconds
+		ldapOptions.timeout = (credentials.timeout as number) * 1000;
+	}
+
 	if (nodeDebug) {
 		Logger.info(
 			`[${nodeType} | ${nodeName}] - LDAP Options: ${JSON.stringify(ldapOptions, null, 2)}`,

diff --git a/packages/nodes-base/nodes/Ldap/Ldap.node.ts b/packages/nodes-base/nodes/Ldap/Ldap.node.ts
@@ -103,14 +103,16 @@ export class Ldap implements INodeType {
 				credential: ICredentialsDecrypted,
 			): Promise<INodeCredentialTestResult> {
 				const credentials = credential.data as ICredentialDataDecryptedObject;
+				const client = await createLdapClient(credentials);
 				try {
-					const client = await createLdapClient(credentials);
 					await client.bind(credentials.bindDN as string, credentials.bindPassword as string);
 				} catch (error) {
 					return {
 						status: 'Error',
 						message: error.message,
 					};
+				} finally {
+					await client.unbind();
 				}
 				return {
 					status: 'OK',
@@ -126,11 +128,21 @@ export class Ldap implements INodeType {
 				try {
 					await client.bind(credentials.bindDN as string, credentials.bindPassword as string);
 				} catch (error) {
+					await client.unbind();
 					console.log(error);
+					return [];
 				}
 
+				let results;
 				const baseDN = this.getNodeParameter('baseDN', 0) as string;
-				const results = await client.search(baseDN, { sizeLimit: 200, paged: false }); // should this size limit be set in credentials?
+				try {
+					results = await client.search(baseDN, { sizeLimit: 200, paged: false }); // should this size limit be set in credentials?
+				} catch (error) {
+					console.log(error);
+					return [];
+				} finally {
+					await client.unbind();
+				}
 
 				const unique = Object.keys(Object.assign({}, ...results.searchEntries));
 				return unique.map((x) => ({
@@ -145,11 +157,23 @@ export class Ldap implements INodeType {
 				try {
 					await client.bind(credentials.bindDN as string, credentials.bindPassword as string);
 				} catch (error) {
+					await client.unbind();
 					console.log(error);
+					return [];
 				}
 
 				const baseDN = this.getNodeParameter('baseDN', 0) as string;
-				const results = await client.search(baseDN, { sizeLimit: 10, paged: false }); // should this size limit be set in credentials?
+
+				let results;
+				try {
+					results = await client.search(baseDN, { sizeLimit: 10, paged: false }); // should this size limit be set in credentials?
+				} catch (error) {
+					console.log(error);
+					return [];
+				} finally {
+					await client.unbind();
+				}
+
 				const objects = [];
 				for (const entry of results.searchEntries) {
 					if (typeof entry.objectClass === 'string') {
@@ -177,11 +201,21 @@ export class Ldap implements INodeType {
 				try {
 					await client.bind(credentials.bindDN as string, credentials.bindPassword as string);
 				} catch (error) {
+					await client.unbind();
 					console.log(error);
+					return [];
 				}
 
+				let results;
 				const baseDN = this.getNodeParameter('dn', 0) as string;
-				const results = await client.search(baseDN, { sizeLimit: 1, paged: false });
+				try {
+					results = await client.search(baseDN, { sizeLimit: 1, paged: false });
+				} catch (error) {
+					console.log(error);
+					return [];
+				} finally {
+					await client.unbind();
+				}
 
 				const unique = Object.keys(Object.assign({}, ...results.searchEntries));
 				return unique.map((x) => ({
@@ -218,6 +252,7 @@ export class Ldap implements INodeType {
 			await client.bind(credentials.bindDN as string, credentials.bindPassword as string);
 		} catch (error) {
 			delete error.cert;
+			await client.unbind();
 			if (this.continueOnFail()) {
 				return [
 					items.map((x) => {
@@ -386,6 +421,7 @@ export class Ldap implements INodeType {
 				if (this.continueOnFail()) {
 					returnItems.push({ json: items[itemIndex].json, error, pairedItem: itemIndex });
 				} else {
+					await client.unbind();
 					if (error.context) {
 						error.context.itemIndex = itemIndex;
 						throw error;
@@ -399,6 +435,9 @@ export class Ldap implements INodeType {
 		if (nodeDebug) {
 			Logger.info(`[${this.getNode().type} | ${this.getNode().name}] - Finished`);
 		}
+
+		await client.unbind();
+
 		return [returnItems];
 	}
 }