Merge pull request #3082 from Infisical/app-connections-and-secret-syncs-unique-constraint

Fix: Move App Connection and Secret Sync Unique Name Constraint to DB

backend/src/db/migrations/20250204025010_app-connections-and-secret-syncs-unique-constraint.ts

@@ -0,0 +1,23 @@
+import { Knex } from "knex";
+
+import { TableName } from "@app/db/schemas";
+
+export async function up(knex: Knex): Promise<void> {
+  await knex.schema.alterTable(TableName.AppConnection, (t) => {
+    t.unique(["orgId", "name"]);
+  });
+
+  await knex.schema.alterTable(TableName.SecretSync, (t) => {
+    t.unique(["projectId", "name"]);
+  });
+}
+
+export async function down(knex: Knex): Promise<void> {
+  await knex.schema.alterTable(TableName.AppConnection, (t) => {
+    t.dropUnique(["orgId", "name"]);
+  });
+
+  await knex.schema.alterTable(TableName.SecretSync, (t) => {
+    t.dropUnique(["projectId", "name"]);
+  });
+}

backend/src/lib/error-codes/database.ts

@@ -0,0 +1,4 @@
+export enum DatabaseErrorCode {
+  ForeignKeyViolation = "23503",
+  UniqueViolation = "23505"
+}

backend/src/lib/error-codes/index.ts

@@ -0,0 +1 @@
+export * from "./database";

backend/src/services/app-connection/app-connection-service.ts

@@ -3,6 +3,7 @@ import { ForbiddenError, subject } from "@casl/ability";
 import { OrgPermissionAppConnectionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission";
 import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service";
 import { generateHash } from "@app/lib/crypto/encryption";
+import { DatabaseErrorCode } from "@app/lib/error-codes";
 import { BadRequestError, DatabaseError, NotFoundError } from "@app/lib/errors";
 import { DiscriminativePick, OrgServiceActor } from "@app/lib/types";
 import { AppConnection } from "@app/services/app-connection/app-connection-enums";
@@ -144,54 +145,40 @@ export const appConnectionServiceFactory = ({
       OrgPermissionSubjects.AppConnections
     );
 
-    const appConnection = await appConnectionDAL.transaction(async (tx) => {
+    const validatedCredentials = await validateAppConnectionCredentials({
-      const isConflictingName = Boolean(
+      app,
-        await appConnectionDAL.findOne(
+      credentials,
-          {
+      method,
-            name: params.name,
+      orgId: actor.orgId
-            orgId: actor.orgId
+    } as TAppConnectionConfig);
-          },
-          tx
-        )
-      );
 
-      if (isConflictingName)
+    const encryptedCredentials = await encryptAppConnectionCredentials({
-        throw new BadRequestError({
+      credentials: validatedCredentials,
-          message: `An App Connection with the name "${params.name}" already exists`
+      orgId: actor.orgId,
-        });
+      kmsService
+    });
 
-      const validatedCredentials = await validateAppConnectionCredentials({
+    try {
-        app,
+      const connection = await appConnectionDAL.create({
-        credentials,
-        method,
-        orgId: actor.orgId
-      } as TAppConnectionConfig);
-
-      const encryptedCredentials = await encryptAppConnectionCredentials({
-        credentials: validatedCredentials,
         orgId: actor.orgId,
-        kmsService
+        encryptedCredentials,
+        method,
+        app,
+        ...params
       });
 
-      const connection = await appConnectionDAL.create(
-        {
-          orgId: actor.orgId,
-          encryptedCredentials,
-          method,
-          app,
-          ...params
-        },
-        tx
-      );
-
       return {
         ...connection,
         credentialsHash: generateHash(connection.encryptedCredentials),
         credentials: validatedCredentials
-      };
+      } as TAppConnection;
-    });
+    } catch (err) {
+      if (err instanceof DatabaseError && (err.error as { code: string })?.code === DatabaseErrorCode.UniqueViolation) {
+        throw new BadRequestError({ message: `An App Connection with the name "${params.name}" already exists` });
+      }
 
-    return appConnection as TAppConnection;
+      throw err;
+    }
   };
 
   const updateAppConnection = async (
@@ -215,72 +202,55 @@ export const appConnectionServiceFactory = ({
       OrgPermissionSubjects.AppConnections
     );
 
-    const updatedAppConnection = await appConnectionDAL.transaction(async (tx) => {
+    let encryptedCredentials: undefined | Buffer;
-      if (params.name && appConnection.name !== params.name) {
-        const isConflictingName = Boolean(
-          await appConnectionDAL.findOne(
-            {
-              name: params.name,
-              orgId: appConnection.orgId
-            },
-            tx
-          )
-        );
 
-        if (isConflictingName)
+    if (credentials) {
-          throw new BadRequestError({
+      const { app, method } = appConnection as DiscriminativePick<TAppConnectionConfig, "app" | "method">;
-            message: `An App Connection with the name "${params.name}" already exists`
-          });
-      }
 
-      let encryptedCredentials: undefined | Buffer;
+      if (
-
+        !VALIDATE_APP_CONNECTION_CREDENTIALS_MAP[app].safeParse({
-      if (credentials) {
+          method,
-        const { app, method } = appConnection as DiscriminativePick<TAppConnectionConfig, "app" | "method">;
+          credentials
-
+        }).success
-        if (
+      )
-          !VALIDATE_APP_CONNECTION_CREDENTIALS_MAP[app].safeParse({
+        throw new BadRequestError({
-            method,
+          message: `Invalid credential format for ${
-            credentials
+            APP_CONNECTION_NAME_MAP[app]
-          }).success
+          } Connection with method ${getAppConnectionMethodName(method)}`
-        )
-          throw new BadRequestError({
-            message: `Invalid credential format for ${
-              APP_CONNECTION_NAME_MAP[app]
-            } Connection with method ${getAppConnectionMethodName(method)}`
-          });
-
-        const validatedCredentials = await validateAppConnectionCredentials({
-          app,
-          orgId: actor.orgId,
-          credentials,
-          method
-        } as TAppConnectionConfig);
-
-        if (!validatedCredentials)
-          throw new BadRequestError({ message: "Unable to validate connection - check credentials" });
-
-        encryptedCredentials = await encryptAppConnectionCredentials({
-          credentials: validatedCredentials,
-          orgId: actor.orgId,
-          kmsService
         });
+
+      const validatedCredentials = await validateAppConnectionCredentials({
+        app,
+        orgId: actor.orgId,
+        credentials,
+        method
+      } as TAppConnectionConfig);
+
+      if (!validatedCredentials)
+        throw new BadRequestError({ message: "Unable to validate connection - check credentials" });
+
+      encryptedCredentials = await encryptAppConnectionCredentials({
+        credentials: validatedCredentials,
+        orgId: actor.orgId,
+        kmsService
+      });
+    }
+
+    try {
+      const updatedConnection = await appConnectionDAL.updateById(connectionId, {
+        orgId: actor.orgId,
+        encryptedCredentials,
+        ...params
+      });
+
+      return await decryptAppConnection(updatedConnection, kmsService);
+    } catch (err) {
+      if (err instanceof DatabaseError && (err.error as { code: string })?.code === DatabaseErrorCode.UniqueViolation) {
+        throw new BadRequestError({ message: `An App Connection with the name "${params.name}" already exists` });
       }
 
-      const updatedConnection = await appConnectionDAL.updateById(
+      throw err;
-        connectionId,
+    }
-        {
-          orgId: actor.orgId,
-          encryptedCredentials,
-          ...params
-        },
-        tx
-      );
-
-      return updatedConnection;
-    });
-
-    return decryptAppConnection(updatedAppConnection, kmsService);
   };
 
   const deleteAppConnection = async (app: AppConnection, connectionId: string, actor: OrgServiceActor) => {
@@ -311,7 +281,10 @@ export const appConnectionServiceFactory = ({
 
       return await decryptAppConnection(deletedAppConnection, kmsService);
     } catch (err) {
-      if (err instanceof DatabaseError && (err.error as { code: string })?.code === "23503") {
+      if (
+        err instanceof DatabaseError &&
+        (err.error as { code: string })?.code === DatabaseErrorCode.ForeignKeyViolation
+      ) {
         throw new BadRequestError({
           message:
             "Cannot delete App Connection with existing connections. Remove all existing connections and try again."

backend/src/services/secret-sync/secret-sync-dal.ts

@@ -123,47 +123,39 @@ export const secretSyncDALFactory = (
   };
 
   const create = async (data: Parameters<(typeof secretSyncOrm)["create"]>[0]) => {
-    try {
+    const secretSync = (await secretSyncOrm.transaction(async (tx) => {
-      const secretSync = (await secretSyncOrm.transaction(async (tx) => {
+      const sync = await secretSyncOrm.create(data, tx);
-        const sync = await secretSyncOrm.create(data, tx);
 
-        return baseSecretSyncQuery({
+      return baseSecretSyncQuery({
-          filter: { id: sync.id },
+        filter: { id: sync.id },
-          db,
+        db,
-          tx
+        tx
-        }).first();
+      }).first();
-      }))!;
+    }))!;
 
-      // TODO (scott): replace with cached folder path once implemented
+    // TODO (scott): replace with cached folder path once implemented
-      const [folderWithPath] = secretSync.folderId
+    const [folderWithPath] = secretSync.folderId
-        ? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])
+      ? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])
-        : [];
+      : [];
-      return expandSecretSync(secretSync, folderWithPath);
+    return expandSecretSync(secretSync, folderWithPath);
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Create - Secret Sync" });
-    }
   };
 
   const updateById = async (syncId: string, data: Parameters<(typeof secretSyncOrm)["updateById"]>[1]) => {
-    try {
+    const secretSync = (await secretSyncOrm.transaction(async (tx) => {
-      const secretSync = (await secretSyncOrm.transaction(async (tx) => {
+      const sync = await secretSyncOrm.updateById(syncId, data, tx);
-        const sync = await secretSyncOrm.updateById(syncId, data, tx);
 
-        return baseSecretSyncQuery({
+      return baseSecretSyncQuery({
-          filter: { id: sync.id },
+        filter: { id: sync.id },
-          db,
+        db,
-          tx
+        tx
-        }).first();
+      }).first();
-      }))!;
+    }))!;
 
-      // TODO (scott): replace with cached folder path once implemented
+    // TODO (scott): replace with cached folder path once implemented
-      const [folderWithPath] = secretSync.folderId
+    const [folderWithPath] = secretSync.folderId
-        ? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])
+      ? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])
-        : [];
+      : [];
-      return expandSecretSync(secretSync, folderWithPath);
+    return expandSecretSync(secretSync, folderWithPath);
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Update by ID - Secret Sync" });
-    }
   };
 
   const findOne = async (filter: Parameters<(typeof secretSyncOrm)["findOne"]>[0], tx?: Knex) => {

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

@@ -8,7 +8,8 @@ import {
   ProjectPermissionSub
 } from "@app/ee/services/permission/project-permission";
 import { KeyStorePrefixes, TKeyStoreFactory } from "@app/keystore/keystore";
-import { BadRequestError, NotFoundError } from "@app/lib/errors";
+import { DatabaseErrorCode } from "@app/lib/error-codes";
+import { BadRequestError, DatabaseError, NotFoundError } from "@app/lib/errors";
 import { OrgServiceActor } from "@app/lib/types";
 import { TAppConnectionServiceFactory } from "@app/services/app-connection/app-connection-service";
 import { TProjectBotServiceFactory } from "@app/services/project-bot/project-bot-service";
@@ -197,37 +198,26 @@ export const secretSyncServiceFactory = ({
     // validates permission to connect and app is valid for sync destination
     await appConnectionService.connectAppConnectionById(destinationApp, params.connectionId, actor);
 
-    const secretSync = await secretSyncDAL.transaction(async (tx) => {
+    try {
-      const isConflictingName = Boolean(
+      const secretSync = await secretSyncDAL.create({
-        (
-          await secretSyncDAL.find(
-            {
-              name: params.name,
-              projectId
-            },
-            tx
-          )
-        ).length
-      );
-
-      if (isConflictingName)
-        throw new BadRequestError({
-          message: `A Secret Sync with the name "${params.name}" already exists for the project with ID "${folder.projectId}"`
-        });
-
-      const sync = await secretSyncDAL.create({
         folderId: folder.id,
         ...params,
         ...(params.isAutoSyncEnabled && { syncStatus: SecretSyncStatus.Pending }),
         projectId
       });
 
-      return sync;
+      if (secretSync.isAutoSyncEnabled) await secretSyncQueue.queueSecretSyncSyncSecretsById({ syncId: secretSync.id });
-    });
 
-    if (secretSync.isAutoSyncEnabled) await secretSyncQueue.queueSecretSyncSyncSecretsById({ syncId: secretSync.id });
+      return secretSync as TSecretSync;
+    } catch (err) {
+      if (err instanceof DatabaseError && (err.error as { code: string })?.code === DatabaseErrorCode.UniqueViolation) {
+        throw new BadRequestError({
+          message: `A Secret Sync with the name "${params.name}" already exists for the project with ID "${folder.projectId}"`
+        });
+      }
 
-    return secretSync as TSecretSync;
+      throw err;
+    }
   };
 
   const updateSecretSync = async (
@@ -260,78 +250,65 @@ export const secretSyncServiceFactory = ({
         message: `Secret sync with ID "${secretSync.id}" is not configured for ${SECRET_SYNC_NAME_MAP[destination]}`
       });
 
-    const updatedSecretSync = await secretSyncDAL.transaction(async (tx) => {
+    let { folderId } = secretSync;
-      let { folderId } = secretSync;
 
-      if (params.connectionId) {
+    if (params.connectionId) {
-        const destinationApp = SECRET_SYNC_CONNECTION_MAP[secretSync.destination as SecretSync];
+      const destinationApp = SECRET_SYNC_CONNECTION_MAP[secretSync.destination as SecretSync];
 
-        // validates permission to connect and app is valid for sync destination
+      // validates permission to connect and app is valid for sync destination
-        await appConnectionService.connectAppConnectionById(destinationApp, params.connectionId, actor);
+      await appConnectionService.connectAppConnectionById(destinationApp, params.connectionId, actor);
-      }
+    }
 
-      if (
+    if (
-        (secretPath && secretPath !== secretSync.folder?.path) ||
+      (secretPath && secretPath !== secretSync.folder?.path) ||
-        (environment && environment !== secretSync.environment?.slug)
+      (environment && environment !== secretSync.environment?.slug)
-      ) {
+    ) {
-        const updatedEnvironment = environment ?? secretSync.environment?.slug;
+      const updatedEnvironment = environment ?? secretSync.environment?.slug;
-        const updatedSecretPath = secretPath ?? secretSync.folder?.path;
+      const updatedSecretPath = secretPath ?? secretSync.folder?.path;
 
-        if (!updatedEnvironment || !updatedSecretPath)
+      if (!updatedEnvironment || !updatedSecretPath)
-          throw new BadRequestError({ message: "Must specify both source environment and secret path" });
+        throw new BadRequestError({ message: "Must specify both source environment and secret path" });
 
-        ForbiddenError.from(permission).throwUnlessCan(
+      ForbiddenError.from(permission).throwUnlessCan(
-          ProjectPermissionActions.Read,
+        ProjectPermissionActions.Read,
-          subject(ProjectPermissionSub.Secrets, {
+        subject(ProjectPermissionSub.Secrets, {
-            environment: updatedEnvironment,
+          environment: updatedEnvironment,
-            secretPath: updatedSecretPath
+          secretPath: updatedSecretPath
-          })
+        })
-        );
+      );
 
-        const newFolder = await folderDAL.findBySecretPath(secretSync.projectId, updatedEnvironment, updatedSecretPath);
+      const newFolder = await folderDAL.findBySecretPath(secretSync.projectId, updatedEnvironment, updatedSecretPath);
 
-        if (!newFolder)
+      if (!newFolder)
-          throw new BadRequestError({
+        throw new BadRequestError({
-            message: `Could not find folder with path "${secretPath}" in environment "${environment}" for project with ID "${secretSync.projectId}"`
+          message: `Could not find folder with path "${secretPath}" in environment "${environment}" for project with ID "${secretSync.projectId}"`
-          });
+        });
 
-        folderId = newFolder.id;
+      folderId = newFolder.id;
-      }
+    }
 
-      if (params.name && secretSync.name !== params.name) {
+    const isAutoSyncEnabled = params.isAutoSyncEnabled ?? secretSync.isAutoSyncEnabled;
-        const isConflictingName = Boolean(
-          (
-            await secretSyncDAL.find(
-              {
-                name: params.name,
-                projectId: secretSync.projectId
-              },
-              tx
-            )
-          ).length
-        );
 
-        if (isConflictingName)
+    try {
-          throw new BadRequestError({
+      const updatedSecretSync = await secretSyncDAL.updateById(syncId, {
-            message: `A Secret Sync with the name "${params.name}" already exists for project with ID "${secretSync.projectId}"`
-          });
-      }
-
-      const isAutoSyncEnabled = params.isAutoSyncEnabled ?? secretSync.isAutoSyncEnabled;
-
-      const updatedSync = await secretSyncDAL.updateById(syncId, {
         ...params,
         ...(isAutoSyncEnabled && folderId && { syncStatus: SecretSyncStatus.Pending }),
         folderId
       });
 
-      return updatedSync;
+      if (updatedSecretSync.isAutoSyncEnabled)
-    });
+        await secretSyncQueue.queueSecretSyncSyncSecretsById({ syncId: secretSync.id });
 
-    if (updatedSecretSync.isAutoSyncEnabled)
+      return updatedSecretSync as TSecretSync;
-      await secretSyncQueue.queueSecretSyncSyncSecretsById({ syncId: secretSync.id });
+    } catch (err) {
+      if (err instanceof DatabaseError && (err.error as { code: string })?.code === DatabaseErrorCode.UniqueViolation) {
+        throw new BadRequestError({
+          message: `A Secret Sync with the name "${params.name}" already exists for the project with ID "${secretSync.projectId}"`
+        });
+      }
 
-    return updatedSecretSync as TSecretSync;
+      throw err;
+    }
   };
 
   const deleteSecretSync = async (