From 8fd516d78e59e70a6eecf9d2716ede28e0e4b51c Mon Sep 17 00:00:00 2001 From: Victor Santos Date: Thu, 6 Nov 2025 20:17:18 -0300 Subject: [PATCH] refactor: update LDAP password rotation functions to use executeWithPotentialGateway for connection handling and improve error management --- .../ldap-password-rotation-fns.ts | 65 ++--- backend/src/lib/validator/validate-url.ts | 4 +- .../ldap/ldap-connection-fns.ts | 255 +++++++----------- 3 files changed, 136 insertions(+), 188 deletions(-) diff --git a/backend/src/ee/services/secret-rotation-v2/ldap-password/ldap-password-rotation-fns.ts b/backend/src/ee/services/secret-rotation-v2/ldap-password/ldap-password-rotation-fns.ts index 9b1fd14a0..7bcdb2818 100644 --- a/backend/src/ee/services/secret-rotation-v2/ldap-password/ldap-password-rotation-fns.ts +++ b/backend/src/ee/services/secret-rotation-v2/ldap-password/ldap-password-rotation-fns.ts @@ -10,7 +10,7 @@ import { import { logger } from "@app/lib/logger"; import { DistinguishedNameRegex } from "@app/lib/regex"; import { encryptAppConnectionCredentials } from "@app/services/app-connection/app-connection-fns"; -import { getLdapConnectionClient, LdapProvider, TLdapConnection } from "@app/services/app-connection/ldap"; +import { executeWithPotentialGateway, LdapProvider, TLdapConnection } from "@app/services/app-connection/ldap"; import { generatePassword } from "../shared/utils"; import { @@ -71,17 +71,19 @@ export const ldapPasswordRotationFactory: TRotationFactory< TLdapPasswordRotationWithConnection, TLdapPasswordRotationGeneratedCredentials, TLdapPasswordRotationInput["temporaryParameters"] -> = (secretRotation, appConnectionDAL, kmsService) => { +> = (secretRotation, appConnectionDAL, kmsService, gatewayService, gatewayV2Service) => { const { connection, parameters, secretsMapping, activeIndex } = secretRotation; const { dn, passwordRequirements } = parameters; const $verifyCredentials = async (credentials: Pick) => { try { - const client = await getLdapConnectionClient({ ...connection.credentials, ...credentials }); - - client.unbind(); - client.destroy(); + await executeWithPotentialGateway( + { ...connection, credentials: { ...connection.credentials, ...credentials } }, + gatewayService, + gatewayV2Service, + async () => {} + ); } catch (error) { throw new Error(`Failed to verify credentials - ${(error as Error).message}`); } @@ -92,17 +94,7 @@ export const ldapPasswordRotationFactory: TRotationFactory< if (!credentials.url.startsWith("ldaps")) throw new Error("Password Rotation requires an LDAPS connection"); - const client = await getLdapConnectionClient( - currentPassword - ? { - ...credentials, - password: currentPassword, - dn - } - : credentials - ); const isConnectionRotation = credentials.dn === dn; - const password = generatePassword(passwordRequirements); let changes: ldap.Change[] | ldap.Change; @@ -147,22 +139,33 @@ export const ldapPasswordRotationFactory: TRotationFactory< throw new Error(`Unhandled provider: ${credentials.provider as LdapProvider}`); } - try { - const userDn = await getDN(dn, client); - await new Promise((resolve, reject) => { - client.modify(userDn, changes, (err) => { - if (err) { - logger.error(err, "LDAP Password Rotation Failed"); - reject(new Error(`Provider Modify Error: ${err.message}`)); - } else { - resolve(true); - } + await executeWithPotentialGateway( + { + ...connection, + credentials: currentPassword + ? { + ...credentials, + password: currentPassword, + dn + } + : credentials + }, + gatewayService, + gatewayV2Service, + async (client) => { + const userDn = await getDN(dn, client); + await new Promise((resolve, reject) => { + client.modify(userDn, changes, (err) => { + if (err) { + logger.error(err, "LDAP Password Rotation Failed"); + reject(new Error(`Provider Modify Error: ${err.message}`)); + } else { + resolve(); + } + }); }); - }); - } finally { - client.unbind(); - client.destroy(); - } + } + ); await $verifyCredentials({ dn, password }); diff --git a/backend/src/lib/validator/validate-url.ts b/backend/src/lib/validator/validate-url.ts index a4c07b37d..7b2b09209 100644 --- a/backend/src/lib/validator/validate-url.ts +++ b/backend/src/lib/validator/validate-url.ts @@ -8,10 +8,10 @@ import { getConfig } from "@app/lib/config/env"; import { BadRequestError } from "../errors"; import { isPrivateIp } from "../ip/ipRange"; -export const blockLocalAndPrivateIpAddresses = async (url: string) => { +export const blockLocalAndPrivateIpAddresses = async (url: string, isGateway = false) => { const appCfg = getConfig(); - if (appCfg.isDevelopmentMode) return; + if (appCfg.isDevelopmentMode || isGateway) return; const validUrl = new URL(url); diff --git a/backend/src/services/app-connection/ldap/ldap-connection-fns.ts b/backend/src/services/app-connection/ldap/ldap-connection-fns.ts index f7eb2189d..36036f460 100644 --- a/backend/src/services/app-connection/ldap/ldap-connection-fns.ts +++ b/backend/src/services/app-connection/ldap/ldap-connection-fns.ts @@ -2,6 +2,7 @@ import ldap from "ldapjs"; import { TGatewayServiceFactory } from "@app/ee/services/gateway/gateway-service"; import { TGatewayV2ServiceFactory } from "@app/ee/services/gateway-v2/gateway-v2-service"; +import { getConfig } from "@app/lib/config/env"; import { BadRequestError } from "@app/lib/errors"; import { GatewayProxyProtocol, withGatewayProxy } from "@app/lib/gateway"; import { withGatewayV2Proxy } from "@app/lib/gateway-v2/gateway-v2"; @@ -12,6 +13,8 @@ import { AppConnection } from "@app/services/app-connection/app-connection-enums import { LdapConnectionMethod } from "./ldap-connection-enums"; import { TLdapConnectionConfig } from "./ldap-connection-types"; +const LDAP_TIMEOUT = 15_000; + const parseLdapUrl = (url: string): { protocol: string; host: string; port: number } => { const urlObj = new URL(url); const isSSL = urlObj.protocol === "ldaps:"; @@ -28,6 +31,48 @@ const constructLdapUrl = (protocol: string, host: string, port: number): string return `${protocol}://${host}:${port}`; }; +const setupLdapClientHandlers = ( + client: ldap.Client, + dn: string, + password: string, + onSuccess: (client: ldap.Client) => T | Promise +): Promise => { + return new Promise((resolve, reject) => { + const handleError = (errorType: string, err: Error) => { + logger.error(err, errorType); + client.destroy(); + reject(new Error(`${errorType.replace("LDAP ", "")} - ${err.message}`)); + }; + + client.on("error", (err: Error) => handleError("LDAP Error", err)); + client.on("connectError", (err: Error) => handleError("LDAP Connection Error", err)); + client.on("connectRefused", (err: Error) => handleError("LDAP Connection Refused", err)); + client.on("connectTimeout", (err: Error) => handleError("LDAP Connection Timeout", err)); + + client.on("connect", () => { + client.bind(dn, password, (err) => { + if (err) { + logger.error(err, "LDAP Bind Error"); + client.destroy(); + reject(new Error(`Bind Error: ${err.message}`)); + return; + } + + try { + const result = onSuccess(client); + if (result instanceof Promise) { + result.then((value) => resolve(value)).catch(reject); + } else { + resolve(result); + } + } catch (error) { + reject(error); + } + }); + }); + }); +}; + export const getLdapConnectionListItem = () => { return { name: "LDAP" as const, @@ -36,8 +81,6 @@ export const getLdapConnectionListItem = () => { }; }; -const LDAP_TIMEOUT = 15_000; - export const getLdapConnectionClient = async ({ url, dn, @@ -45,59 +88,23 @@ export const getLdapConnectionClient = async ({ sslCertificate, sslRejectUnauthorized = true }: TLdapConnectionConfig["credentials"]) => { - await blockLocalAndPrivateIpAddresses(url); + await blockLocalAndPrivateIpAddresses(url, false); const isSSL = url.startsWith("ldaps"); - return new Promise((resolve, reject) => { - const client = ldap.createClient({ - url, - timeout: LDAP_TIMEOUT, - connectTimeout: LDAP_TIMEOUT, - tlsOptions: isSSL - ? { - rejectUnauthorized: sslRejectUnauthorized, - ca: sslCertificate ? [sslCertificate] : undefined - } - : undefined - }); - - client.on("error", (err: Error) => { - logger.error(err, "LDAP Error"); - client.destroy(); - reject(new Error(`Provider Error - ${err.message}`)); - }); - - client.on("connectError", (err: Error) => { - logger.error(err, "LDAP Connection Error"); - client.destroy(); - reject(new Error(`Provider Connect Error - ${err.message}`)); - }); - - client.on("connectRefused", (err: Error) => { - logger.error(err, "LDAP Connection Refused"); - client.destroy(); - reject(new Error(`Provider Connection Refused - ${err.message}`)); - }); - - client.on("connectTimeout", (err: Error) => { - logger.error(err, "LDAP Connection Timeout"); - client.destroy(); - reject(new Error(`Provider Connection Timeout - ${err.message}`)); - }); - - client.on("connect", () => { - client.bind(dn, password, (err) => { - if (err) { - logger.error(err, "LDAP Bind Error"); - reject(new Error(`Bind Error: ${err.message}`)); - client.destroy(); + const client = ldap.createClient({ + url, + timeout: LDAP_TIMEOUT, + connectTimeout: LDAP_TIMEOUT, + tlsOptions: isSSL + ? { + rejectUnauthorized: sslRejectUnauthorized, + ca: sslCertificate ? [sslCertificate] : undefined } - - resolve(client); - }); - }); + : undefined }); + + return setupLdapClientHandlers(client, dn, password, (ldapClient) => ldapClient); }; export const executeWithPotentialGateway = async ( @@ -108,8 +115,10 @@ export const executeWithPotentialGateway = async ( ): Promise => { const { gatewayId, credentials } = config; const { protocol, host, port } = parseLdapUrl(credentials.url); + const appCfg = getConfig(); if (gatewayId && gatewayService && gatewayV2Service) { + await blockLocalAndPrivateIpAddresses(credentials.url, true); const platformConnectionDetails = await gatewayV2Service.getPlatformConnectionDetailsByGatewayId({ gatewayId, targetHost: host, @@ -121,62 +130,28 @@ export const executeWithPotentialGateway = async ( async (proxyPort) => { const proxyUrl = constructLdapUrl(protocol, "localhost", proxyPort); const isSSL = protocol === "ldaps"; + const client = ldap.createClient({ url: proxyUrl, timeout: LDAP_TIMEOUT, connectTimeout: LDAP_TIMEOUT, tlsOptions: isSSL ? { - rejectUnauthorized: sslRejectUnauthorized, - ca: sslCertificate ? [sslCertificate] : undefined + rejectUnauthorized: config.credentials.sslRejectUnauthorized, + ca: config.credentials.sslCertificate ? [config.credentials.sslCertificate] : undefined, + servername: host, + // bypass hostname verification for development + ...(appCfg.isDevelopmentMode ? { checkServerIdentity: () => undefined } : {}) } : undefined }); - return new Promise((resolve, reject) => { - client.on("error", (err: Error) => { - logger.error(err, "LDAP Error"); - client.destroy(); - reject(new Error(`Provider Error - ${err.message}`)); - }); - - client.on("connectError", (err: Error) => { - logger.error(err, "LDAP Connection Error"); - client.destroy(); - reject(new Error(`Provider Connect Error - ${err.message}`)); - }); - - client.on("connectRefused", (err: Error) => { - logger.error(err, "LDAP Connection Refused"); - client.destroy(); - reject(new Error(`Provider Connection Refused - ${err.message}`)); - }); - - client.on("connectTimeout", (err: Error) => { - logger.error(err, "LDAP Connection Timeout"); - client.destroy(); - reject(new Error(`Provider Connection Timeout - ${err.message}`)); - }); - - client.on("connect", () => { - client.bind(credentials.dn, credentials.password, async (err) => { - if (err) { - logger.error(err, "LDAP Bind Error"); - client.destroy(); - reject(new Error(`Bind Error: ${err.message}`)); - return; - } - - try { - const result = await operation(client); - resolve(result); - } catch (opError) { - reject(opError); - } finally { - client.destroy(); - } - }); - }); + return setupLdapClientHandlers(client, credentials.dn, credentials.password, async (ldapClient) => { + try { + return await operation(ldapClient); + } finally { + ldapClient.destroy(); + } }); }, { @@ -194,61 +169,28 @@ export const executeWithPotentialGateway = async ( async (proxyPort) => { const proxyUrl = constructLdapUrl(protocol, "localhost", proxyPort); const isSSL = protocol === "ldaps"; + const client = ldap.createClient({ url: proxyUrl, timeout: LDAP_TIMEOUT, connectTimeout: LDAP_TIMEOUT, tlsOptions: isSSL ? { - rejectUnauthorized: sslRejectUnauthorized, - ca: sslCertificate ? [sslCertificate] : undefined + rejectUnauthorized: config.credentials.sslRejectUnauthorized, + ca: config.credentials.sslCertificate ? [config.credentials.sslCertificate] : undefined, + servername: host, + // bypass hostname verification for development + ...(appCfg.isDevelopmentMode ? { checkServerIdentity: () => undefined } : {}) } : undefined }); - return new Promise((resolve, reject) => { - client.on("error", (err: Error) => { - logger.error(err, "LDAP Error"); - client.destroy(); - reject(new Error(`Provider Error - ${err.message}`)); - }); - client.on("connectError", (err: Error) => { - logger.error(err, "LDAP Connection Error"); - client.destroy(); - reject(new Error(`Provider Connect Error - ${err.message}`)); - }); - - client.on("connectRefused", (err: Error) => { - logger.error(err, "LDAP Connection Refused"); - client.destroy(); - reject(new Error(`Provider Connection Refused - ${err.message}`)); - }); - - client.on("connectTimeout", (err: Error) => { - logger.error(err, "LDAP Connection Timeout"); - client.destroy(); - reject(new Error(`Provider Connection Timeout - ${err.message}`)); - }); - - client.on("connect", () => { - client.bind(credentials.dn, credentials.password, async (err) => { - if (err) { - logger.error(err, "LDAP Bind Error"); - client.destroy(); - reject(new Error(`Bind Error: ${err.message}`)); - return; - } - - try { - const result = await operation(client); - resolve(result); - } catch (opError) { - reject(opError); - } finally { - client.destroy(); - } - }); - }); + return setupLdapClientHandlers(client, credentials.dn, credentials.password, async (ldapClient) => { + try { + return await operation(ldapClient); + } finally { + ldapClient.destroy(); + } }); }, { @@ -277,23 +219,26 @@ export const executeWithPotentialGateway = async ( } }; -export const validateLdapConnectionCredentials = async ({ credentials }: TLdapConnectionConfig) => { - let client: ldap.Client | undefined; - +export const validateLdapConnectionCredentials = async ( + config: TLdapConnectionConfig, + gatewayService: Pick, + gatewayV2Service: Pick +) => { try { - client = await getLdapConnectionClient(credentials); + await executeWithPotentialGateway(config, gatewayService, gatewayV2Service, async (client) => { + // this shouldn't occur as handle connection error events in client but here as fallback + if (!client.connected) { + throw new BadRequestError({ message: "Unable to connect to LDAP server" }); + } + }); - // this shouldn't occur as handle connection error events in client but here as fallback - if (!client.connected) { - throw new BadRequestError({ message: "Unable to connect to LDAP server" }); - } - - return credentials; - } catch (e: unknown) { + return config.credentials; + } catch (error) { throw new BadRequestError({ - message: `Unable to validate connection: ${(e as Error).message || "verify credentials"}` + message: `Unable to validate connection: ${ + (error as Error)?.message?.replaceAll(config.credentials.password, "********************") ?? + "verify credentials" + }` }); - } finally { - client?.destroy(); } };