Merge pull request #4690 from Infisical/fix/error-on-duplicate-folder-creation

fix: returns error on duplicate folder creation

backend/e2e-test/routes/v1/secret-folder.spec.ts

@@ -40,7 +40,7 @@ describe("Secret Folder Router", async () => {
     { name: "folder1", path: "/" }, // one in root
     { name: "folder1", path: "/level1/level2" }, // then create a deep one creating intermediate ones
     { name: "folder2", path: "/" },
-    { name: "folder1", path: "/level1/level2" } // this should not create folder return same thing
+    { name: "folder3", path: "/level1/level2" }
   ])("Create folder $name in $path", async ({ name, path }) => {
     const createdFolder = await createFolder({ path, name });
     // check for default environments
@@ -57,7 +57,7 @@ describe("Secret Folder Router", async () => {
     {
       path: "/",
       expected: {
-        folders: [{ name: "folder1" }, { name: "level1" }, { name: "folder2" }],
+        folders: [{ name: "folder4" }, { name: "level2" }, { name: "folder5" }],
         length: 3
       }
     },
@@ -162,4 +162,25 @@ describe("Secret Folder Router", async () => {
     expect(updatedFolderList).toHaveProperty("folders");
     expect(updatedFolderList.folders.length).toEqual(0);
   });
+  test("Creating a duplicate folder should return a 400 error", async () => {
+    const newFolder = await createFolder({ name: "folder-duplicate", path: "/level1/level2" });
+
+    const res = await testServer.inject({
+      method: "POST",
+      url: `/api/v1/folders`,
+      headers: {
+        authorization: `Bearer ${jwtAuthToken}`
+      },
+      body: {
+        workspaceId: seedData1.project.id,
+        environment: seedData1.environment.slug,
+        name: "folder-duplicate",
+        path: "/level1/level2"
+      }
+    });
+    expect(res.statusCode).toBe(400);
+    const payload = JSON.parse(res.payload);
+    expect(payload).toHaveProperty("error");
+    await deleteFolder({ path: "/level1/level2", id: newFolder.id });
+  });
 });

backend/e2e-test/routes/v2/secret-folder.spec.ts

@@ -41,7 +41,7 @@ describe("Secret Folder Router", async () => {
     { name: "folder1", path: "/" }, // one in root
     { name: "folder1", path: "/level1/level2" }, // then create a deep one creating intermediate ones
     { name: "folder2", path: "/" },
-    { name: "folder1", path: "/level1/level2" } // this should not create folder return same thing
+    { name: "folder3", path: "/level1/level2" }
   ])("Create folder $name in $path", async ({ name, path }) => {
     const createdFolder = await createFolder({ path, name });
     // check for default environments
@@ -58,7 +58,7 @@ describe("Secret Folder Router", async () => {
     {
       path: "/",
       expected: {
-        folders: [{ name: "folder1" }, { name: "level1" }, { name: "folder2" }],
+        folders: [{ name: "folder4" }, { name: "level2" }, { name: "folder5" }],
         length: 3
       }
     },
@@ -163,4 +163,26 @@ describe("Secret Folder Router", async () => {
     expect(updatedFolderList).toHaveProperty("folders");
     expect(updatedFolderList.folders.length).toEqual(0);
   });
+
+  test("Creating a duplicate folder should return a 400 error", async () => {
+    const newFolder = await createFolder({ name: "folder-duplicate", path: "/level1/level2" });
+
+    const res = await testServer.inject({
+      method: "POST",
+      url: `/api/v2/folders`,
+      headers: {
+        authorization: `Bearer ${jwtAuthToken}`
+      },
+      body: {
+        projectId: seedData1.project.id,
+        environment: seedData1.environment.slug,
+        name: "folder-duplicate",
+        path: "/level1/level2"
+      }
+    });
+    expect(res.statusCode).toBe(400);
+    const payload = JSON.parse(res.payload);
+    expect(payload).toHaveProperty("error");
+    await deleteFolder({ path: "/level1/level2", id: newFolder.id });
+  });
 });

backend/src/services/secret-folder/secret-folder-service.ts

@@ -118,24 +118,11 @@ export const secretFolderServiceFactory = ({
         });
       }
 
-      // check if the exact folder already exists
-      const existingFolder = await folderDAL.findOne(
-        {
-          envId: env.id,
-          parentId: parentFolder.id,
-          name,
-          isReserved: false
-        },
-        tx
-      );
-
-      if (existingFolder) {
-        return existingFolder;
-      }
-
       // exact folder case
       if (parentFolder.path === pathWithFolder) {
-        return parentFolder;
+        throw new BadRequestError({
+          message: `Folder with name '${name}' already exists in path '${secretPath}'`
+        });
       }
 
       let currentParentId = parentFolder.id;

frontend/src/hooks/api/secretFolders/index.tsx

@@ -2,6 +2,7 @@ export {
   useCreateFolder,
   useDeleteFolder,
   useGetFoldersByEnv,
+  useGetOrCreateFolder,
   useGetProjectFolders,
   useUpdateFolder
 } from "./queries";

frontend/src/hooks/api/secretFolders/queries.tsx

@@ -140,6 +140,59 @@ export const useGetFoldersByEnv = ({
   return { folders, folderNames, isFolderPresentInEnv, getFolderByNameAndEnv };
 };
 
+export const useGetOrCreateFolder = () => {
+  const queryClient = useQueryClient();
+
+  return useMutation<TSecretFolder, object, TCreateFolderDTO>({
+    mutationFn: async (dto) => {
+      const { data: existingFolder } = await apiRequest.get<{ folders: TSecretFolder[] }>(
+        "/api/v2/folders",
+        {
+          params: {
+            projectId: dto.projectId,
+            environment: dto.environment,
+            path: dto.path || "/"
+          }
+        }
+      );
+
+      const folder = existingFolder.folders.find((f) => f.name === dto.name);
+
+      if (folder) return folder;
+
+      const { data } = await apiRequest.post("/api/v2/folders", {
+        ...dto,
+        projectId: dto.projectId
+      });
+
+      return data;
+    },
+    onSuccess: (_, { projectId, environment, path }) => {
+      queryClient.invalidateQueries({
+        queryKey: dashboardKeys.getDashboardSecrets({
+          projectId,
+          secretPath: path ?? "/"
+        })
+      });
+      queryClient.invalidateQueries({
+        queryKey: folderQueryKeys.getSecretFolders({ projectId, environment, path })
+      });
+      queryClient.invalidateQueries({
+        queryKey: secretSnapshotKeys.list({ projectId, environment, directory: path })
+      });
+      queryClient.invalidateQueries({
+        queryKey: secretSnapshotKeys.count({ projectId, environment, directory: path })
+      });
+      queryClient.invalidateQueries({
+        queryKey: commitKeys.count({ projectId, environment, directory: path })
+      });
+      queryClient.invalidateQueries({
+        queryKey: commitKeys.history({ projectId, environment, directory: path })
+      });
+    }
+  });
+};
+
 export const useCreateFolder = () => {
   const queryClient = useQueryClient();
 
@@ -170,6 +223,9 @@ export const useCreateFolder = () => {
       queryClient.invalidateQueries({
         queryKey: commitKeys.count({ projectId, environment, directory: path })
       });
+      queryClient.invalidateQueries({
+        queryKey: commitKeys.history({ projectId, environment, directory: path })
+      });
     }
   });
 };

frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx

@@ -86,6 +86,7 @@ import {
   useCreateSecretV3,
   useDeleteSecretV3,
   useGetImportedSecretsAllEnvs,
+  useGetOrCreateFolder,
   useGetWsTags,
   useUpdateSecretV3
 } from "@app/hooks/api";
@@ -367,6 +368,7 @@ export const OverviewPage = () => {
   const { mutateAsync: updateSecretV3 } = useUpdateSecretV3();
   const { mutateAsync: deleteSecretV3 } = useDeleteSecretV3();
   const { mutateAsync: createFolder } = useCreateFolder();
+  const { mutateAsync: getOrCreateFolder } = useGetOrCreateFolder();
   const { mutateAsync: updateFolderBatch } = useUpdateFolderBatch();
 
   const { handlePopUpOpen, handlePopUpToggle, handlePopUpClose, popUp } = usePopUp([
@@ -384,16 +386,32 @@ export const OverviewPage = () => {
   ] as const);
 
   const handleFolderCreate = async (folderName: string, description: string | null) => {
-    const promises = userAvailableEnvs.map((env) => {
+    const promises = userAvailableEnvs
-      const environment = env.slug;
+      .map((env) => {
-      return createFolder({
+        const environment = env.slug;
-        name: folderName,
+        const isFolderPresent = isFolderPresentInEnv(folderName, environment);
-        path: secretPath,
+        if (isFolderPresent) {
-        environment,
+          return undefined;
-        projectId,
+        }
-        description
+
+        return createFolder({
+          name: folderName,
+          path: secretPath,
+          environment,
+          projectId,
+          description
+        });
+      })
+      .filter((promise) => promise !== undefined);
+
+    if (promises.length === 0) {
+      handlePopUpClose("addFolder");
+      createNotification({
+        type: "info",
+        text: "Folder already exists in all environments"
       });
-    });
+      return;
+    }
 
     const results = await Promise.allSettled(promises);
     const isFoldersAdded = results.some((result) => result.status === "fulfilled");
@@ -481,7 +499,7 @@ export const OverviewPage = () => {
           })
         );
         if (folderName && parentPath && canCreateFolder) {
-          await createFolder({
+          await getOrCreateFolder({
             projectId,
             path: parentPath,
             environment: env,
@@ -645,7 +663,7 @@ export const OverviewPage = () => {
         })
       );
       if (folderName && parentPath && canCreateFolder) {
-        await createFolder({
+        await getOrCreateFolder({
           projectId,
           environment: slug,
           path: parentPath,

frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx

@@ -25,7 +25,12 @@ import {
 } from "@app/context";
 import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types";
 import { getKeyValue } from "@app/helpers/parseEnvVar";
-import { useCreateFolder, useCreateSecretV3, useCreateWsTag, useGetWsTags } from "@app/hooks/api";
+import {
+  useCreateSecretV3,
+  useCreateWsTag,
+  useGetOrCreateFolder,
+  useGetWsTags
+} from "@app/hooks/api";
 import { SecretType } from "@app/hooks/api/types";
 
 const typeSchema = z
@@ -64,7 +69,7 @@ export const CreateSecretForm = ({ secretPath = "/", onClose }: Props) => {
   const environments = currentProject?.environments || [];
 
   const { mutateAsync: createSecretV3 } = useCreateSecretV3();
-  const { mutateAsync: createFolder } = useCreateFolder();
+  const { mutateAsync: getOrCreateFolder } = useGetOrCreateFolder();
   const { data: projectTags, isPending: isTagsLoading } = useGetWsTags(
     canReadTags ? projectId : ""
   );
@@ -92,7 +97,7 @@ export const CreateSecretForm = ({ secretPath = "/", onClose }: Props) => {
         );
 
         if (folderName && parentPath && canCreateFolder) {
-          await createFolder({
+          await getOrCreateFolder({
             projectId,
             path: parentPath,
             environment,