Merge pull request #4329 from Infisical/misc/address-ldap-update-and-test-issues

misc: address LDAP config update and test issues

backend/src/ee/routes/v1/ldap-router.ts

@@ -379,14 +379,17 @@ export const registerLdapRouter = async (server: FastifyZodProvider) => {
 
   server.route({
     method: "POST",
-    url: "/config/:configId/test-connection",
+    url: "/config/test-connection",
     config: {
       rateLimit: readLimit
     },
     onRequest: verifyAuth([AuthMode.JWT]),
     schema: {
-      params: z.object({
+      body: z.object({
-        configId: z.string().trim()
+        url: z.string().trim(),
+        bindDN: z.string().trim(),
+        bindPass: z.string().trim(),
+        caCert: z.string().trim()
       }),
       response: {
         200: z.boolean()
@@ -399,8 +402,9 @@ export const registerLdapRouter = async (server: FastifyZodProvider) => {
         orgId: req.permission.orgId,
         actorAuthMethod: req.permission.authMethod,
         actorOrgId: req.permission.orgId,
-        ldapConfigId: req.params.configId
+        ...req.body
       });
+
       return result;
     }
   });

backend/src/ee/services/ldap-config/ldap-config-service.ts

@@ -1,4 +1,5 @@
 import { ForbiddenError } from "@casl/ability";
+import { Knex } from "knex";
 
 import { OrgMembershipStatus, TableName, TLdapConfigsUpdate, TUsers } from "@app/db/schemas";
 import { TGroupDALFactory } from "@app/ee/services/group/group-dal";
@@ -45,7 +46,7 @@ import { searchGroups, testLDAPConfig } from "./ldap-fns";
 import { TLdapGroupMapDALFactory } from "./ldap-group-map-dal";
 
 type TLdapConfigServiceFactoryDep = {
-  ldapConfigDAL: Pick<TLdapConfigDALFactory, "create" | "update" | "findOne">;
+  ldapConfigDAL: Pick<TLdapConfigDALFactory, "create" | "update" | "findOne" | "transaction">;
   ldapGroupMapDAL: Pick<TLdapGroupMapDALFactory, "find" | "create" | "delete" | "findLdapGroupMapsByLdapConfigId">;
   orgMembershipDAL: Pick<TOrgMembershipDALFactory, "create">;
   orgDAL: Pick<
@@ -131,6 +132,19 @@ export const ldapConfigServiceFactory = ({
       orgId
     });
 
+    const isConnected = await testLDAPConfig({
+      bindDN,
+      bindPass,
+      caCert,
+      url
+    });
+
+    if (!isConnected) {
+      throw new BadRequestError({
+        message: "Failed to establish connection to LDAP directory. Please verify that your credentials are correct."
+      });
+    }
+
     const ldapConfig = await ldapConfigDAL.create({
       orgId,
       isActive,
@@ -148,6 +162,50 @@ export const ldapConfigServiceFactory = ({
     return ldapConfig;
   };
 
+  const getLdapCfg = async (filter: { orgId: string; isActive?: boolean; id?: string }, tx?: Knex) => {
+    const ldapConfig = await ldapConfigDAL.findOne(filter, tx);
+    if (!ldapConfig) {
+      throw new NotFoundError({
+        message: `Failed to find organization LDAP data in organization with ID '${filter.orgId}'`
+      });
+    }
+
+    const { decryptor } = await kmsService.createCipherPairWithDataKey({
+      type: KmsDataKey.Organization,
+      orgId: ldapConfig.orgId
+    });
+
+    let bindDN = "";
+    if (ldapConfig.encryptedLdapBindDN) {
+      bindDN = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapBindDN }).toString();
+    }
+
+    let bindPass = "";
+    if (ldapConfig.encryptedLdapBindPass) {
+      bindPass = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapBindPass }).toString();
+    }
+
+    let caCert = "";
+    if (ldapConfig.encryptedLdapCaCertificate) {
+      caCert = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapCaCertificate }).toString();
+    }
+
+    return {
+      id: ldapConfig.id,
+      organization: ldapConfig.orgId,
+      isActive: ldapConfig.isActive,
+      url: ldapConfig.url,
+      bindDN,
+      bindPass,
+      uniqueUserAttribute: ldapConfig.uniqueUserAttribute,
+      searchBase: ldapConfig.searchBase,
+      searchFilter: ldapConfig.searchFilter,
+      groupSearchBase: ldapConfig.groupSearchBase,
+      groupSearchFilter: ldapConfig.groupSearchFilter,
+      caCert
+    };
+  };
+
   const updateLdapCfg = async ({
     actor,
     actorId,
@@ -202,53 +260,25 @@ export const ldapConfigServiceFactory = ({
       updateQuery.encryptedLdapCaCertificate = encryptor({ plainText: Buffer.from(caCert) }).cipherTextBlob;
     }
 
-    const [ldapConfig] = await ldapConfigDAL.update({ orgId }, updateQuery);
+    const config = await ldapConfigDAL.transaction(async (tx) => {
+      const [updatedLdapCfg] = await ldapConfigDAL.update({ orgId }, updateQuery, tx);
+      const decryptedLdapCfg = await getLdapCfg({ orgId }, tx);
 
-    return ldapConfig;
+      const isSoftDeletion = !decryptedLdapCfg.url && !decryptedLdapCfg.bindDN && !decryptedLdapCfg.bindPass;
-  };
+      if (!isSoftDeletion) {
+        const isConnected = await testLDAPConfig(decryptedLdapCfg);
+        if (!isConnected) {
+          throw new BadRequestError({
+            message:
+              "Failed to establish connection to LDAP directory. Please verify that your credentials are correct."
+          });
+        }
+      }
 
-  const getLdapCfg = async (filter: { orgId: string; isActive?: boolean; id?: string }) => {
+      return updatedLdapCfg;
-    const ldapConfig = await ldapConfigDAL.findOne(filter);
-    if (!ldapConfig) {
-      throw new NotFoundError({
-        message: `Failed to find organization LDAP data in organization with ID '${filter.orgId}'`
-      });
-    }
-
-    const { decryptor } = await kmsService.createCipherPairWithDataKey({
-      type: KmsDataKey.Organization,
-      orgId: ldapConfig.orgId
     });
 
-    let bindDN = "";
+    return config;
-    if (ldapConfig.encryptedLdapBindDN) {
-      bindDN = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapBindDN }).toString();
-    }
-
-    let bindPass = "";
-    if (ldapConfig.encryptedLdapBindPass) {
-      bindPass = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapBindPass }).toString();
-    }
-
-    let caCert = "";
-    if (ldapConfig.encryptedLdapCaCertificate) {
-      caCert = decryptor({ cipherTextBlob: ldapConfig.encryptedLdapCaCertificate }).toString();
-    }
-
-    return {
-      id: ldapConfig.id,
-      organization: ldapConfig.orgId,
-      isActive: ldapConfig.isActive,
-      url: ldapConfig.url,
-      bindDN,
-      bindPass,
-      uniqueUserAttribute: ldapConfig.uniqueUserAttribute,
-      searchBase: ldapConfig.searchBase,
-      searchFilter: ldapConfig.searchFilter,
-      groupSearchBase: ldapConfig.groupSearchBase,
-      groupSearchFilter: ldapConfig.groupSearchFilter,
-      caCert
-    };
   };
 
   const getLdapCfgWithPermissionCheck = async ({
@@ -694,7 +724,17 @@ export const ldapConfigServiceFactory = ({
     return deletedGroupMap;
   };
 
-  const testLDAPConnection = async ({ actor, actorId, orgId, actorAuthMethod, actorOrgId }: TTestLdapConnectionDTO) => {
+  const testLDAPConnection = async ({
+    actor,
+    actorId,
+    orgId,
+    actorAuthMethod,
+    actorOrgId,
+    bindDN,
+    bindPass,
+    caCert,
+    url
+  }: TTestLdapConnectionDTO) => {
     const { permission } = await permissionService.getOrgPermission(actor, actorId, orgId, actorAuthMethod, actorOrgId);
     ForbiddenError.from(permission).throwUnlessCan(OrgPermissionActions.Create, OrgPermissionSubjects.Ldap);
 
@@ -704,11 +744,12 @@ export const ldapConfigServiceFactory = ({
         message: "Failed to test LDAP connection due to plan restriction. Upgrade plan to test the LDAP connection."
       });
 
-    const ldapConfig = await getLdapCfg({
+    return testLDAPConfig({
-      orgId
+      bindDN,
+      bindPass,
+      caCert,
+      url
     });
-
-    return testLDAPConfig(ldapConfig);
   };
 
   return {

backend/src/ee/services/ldap-config/ldap-config-types.ts

@@ -83,6 +83,4 @@ export type TDeleteLdapGroupMapDTO = {
   ldapGroupMapId: string;
 } & TOrgPermission;
 
-export type TTestLdapConnectionDTO = {
+export type TTestLdapConnectionDTO = TOrgPermission & TTestLDAPConfigDTO;
-  ldapConfigId: string;
-} & TOrgPermission;

frontend/src/hooks/api/ldapConfig/mutations.tsx

@@ -151,10 +151,23 @@ export const useDeleteLDAPGroupMapping = () => {
 
 export const useTestLDAPConnection = () => {
   return useMutation({
-    mutationFn: async (ldapConfigId: string) => {
+    mutationFn: async ({
-      const { data } = await apiRequest.post<boolean>(
+      url,
-        `/api/v1/ldap/config/${ldapConfigId}/test-connection`
+      bindDN,
-      );
+      bindPass,
+      caCert
+    }: {
+      url: string;
+      bindDN: string;
+      bindPass: string;
+      caCert: string;
+    }) => {
+      const { data } = await apiRequest.post<boolean>("/api/v1/ldap/config/test-connection", {
+        url,
+        bindDN,
+        bindPass,
+        caCert
+      });
       return data;
     }
   });

frontend/src/pages/organization/SettingsPage/components/OrgSsoTab/LDAPModal.tsx

@@ -92,12 +92,7 @@ export const LDAPModal = ({ popUp, handlePopUpClose, handlePopUpToggle, hideDele
   const watchUrl = watch("url");
   const watchBindDN = watch("bindDN");
   const watchBindPass = watch("bindPass");
-  const watchSearchBase = watch("searchBase");
-  const watchSearchFilter = watch("searchFilter");
-  const watchGroupSearchBase = watch("groupSearchBase");
-  const watchGroupSearchFilter = watch("groupSearchFilter");
   const watchCaCert = watch("caCert");
-  const watchUniqueUserAttribute = watch("uniqueUserAttribute");
 
   useEffect(() => {
     if (data) {
@@ -147,7 +142,6 @@ export const LDAPModal = ({ popUp, handlePopUpClose, handlePopUpToggle, hideDele
       } else {
         await updateMutateAsync({
           organizationId: currentOrg.id,
-          isActive: false,
           url,
           bindDN,
           bindPass,
@@ -179,23 +173,13 @@ export const LDAPModal = ({ popUp, handlePopUpClose, handlePopUpToggle, hideDele
 
   const handleTestLDAPConnection = async () => {
     try {
-      await onSSOModalSubmit({
+      const result = await testLDAPConnection({
         url: watchUrl,
         bindDN: watchBindDN,
         bindPass: watchBindPass,
-        searchBase: watchSearchBase,
+        caCert: watchCaCert ?? ""
-        searchFilter: watchSearchFilter,
-        groupSearchBase: watchGroupSearchBase,
-        groupSearchFilter: watchGroupSearchFilter,
-        uniqueUserAttribute: watchUniqueUserAttribute,
-        caCert: watchCaCert,
-        shouldCloseModal: false
       });
 
-      if (!data) return;
-
-      const result = await testLDAPConnection(data.id);
-
       if (!result) {
         createNotification({
           text: "Failed to test the LDAP connection: Bind operation was unsuccessful",