Merge pull request #3810 from Infisical/daniel/secret-syncs-permissions

feat(secret-syncs): better permissioning

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

@@ -1,4 +1,4 @@
-import { ForbiddenError } from "@casl/ability";
+import { ForbiddenError, subject } from "@casl/ability";
 
 import { ActionProjectType } from "@app/db/schemas";
 import { TLicenseServiceFactory } from "@app/ee/services/license/license-service";
@@ -89,7 +89,17 @@ export const secretSyncServiceFactory = ({
       projectId
     });
 
-    return secretSyncs as TSecretSync[];
+    return secretSyncs.filter((secretSync) =>
+      permission.can(
+        ProjectPermissionSecretSyncActions.Read,
+        secretSync.environment && secretSync.folder
+          ? subject(ProjectPermissionSub.SecretSyncs, {
+              environment: secretSync.environment.slug,
+              secretPath: secretSync.folder.path
+            })
+          : ProjectPermissionSub.SecretSyncs
+      )
+    ) as TSecretSync[];
   };
 
   const listSecretSyncsBySecretPath = async (
@@ -105,7 +115,15 @@ export const secretSyncServiceFactory = ({
       projectId
     });
 
-    if (permission.cannot(ProjectPermissionSecretSyncActions.Read, ProjectPermissionSub.SecretSyncs)) {
+    if (
+      permission.cannot(
+        ProjectPermissionSecretSyncActions.Read,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment,
+          secretPath
+        })
+      )
+    ) {
       return [];
     }
 
@@ -142,7 +160,12 @@ export const secretSyncServiceFactory = ({
 
     ForbiddenError.from(permission).throwUnlessCan(
       ProjectPermissionSecretSyncActions.Read,
-      ProjectPermissionSub.SecretSyncs
+      secretSync.environment && secretSync.folder
+        ? subject(ProjectPermissionSub.SecretSyncs, {
+            environment: secretSync.environment.slug,
+            secretPath: secretSync.folder.path
+          })
+        : ProjectPermissionSub.SecretSyncs
     );
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
@@ -179,7 +202,12 @@ export const secretSyncServiceFactory = ({
 
     ForbiddenError.from(permission).throwUnlessCan(
       ProjectPermissionSecretSyncActions.Read,
-      ProjectPermissionSub.SecretSyncs
+      secretSync.environment && secretSync.folder
+        ? subject(ProjectPermissionSub.SecretSyncs, {
+            environment: secretSync.environment.slug,
+            secretPath: secretSync.folder.path
+          })
+        : ProjectPermissionSub.SecretSyncs
     );
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
@@ -217,13 +245,17 @@ export const secretSyncServiceFactory = ({
 
     ForbiddenError.from(projectPermission).throwUnlessCan(
       ProjectPermissionSecretSyncActions.Create,
-      ProjectPermissionSub.SecretSyncs
+      subject(ProjectPermissionSub.SecretSyncs, { environment, secretPath })
     );
 
-    throwIfMissingSecretReadValueOrDescribePermission(projectPermission, ProjectPermissionSecretActions.ReadValue, {
-      environment,
-      secretPath
-    });
+    throwIfMissingSecretReadValueOrDescribePermission(
+      projectPermission,
+      ProjectPermissionSecretActions.DescribeSecret,
+      {
+        environment,
+        secretPath
+      }
+    );
 
     const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath);
 
@@ -286,10 +318,38 @@ export const secretSyncServiceFactory = ({
       projectId: secretSync.projectId
     });
 
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretSyncActions.Edit,
-      ProjectPermissionSub.SecretSyncs
-    );
+    // we always check the permission against the existing environment / secret path
+    // if no secret path / environment is present on the secret sync, we need to check without conditions
+    if (secretSync.environment?.slug && secretSync.folder?.path) {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.Edit,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment: secretSync.environment.slug,
+          secretPath: secretSync.folder.path
+        })
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.Edit,
+        ProjectPermissionSub.SecretSyncs
+      );
+    }
+
+    // if the user is updating the secret path or environment, we need to check the permission against the new values
+    if (secretPath || environment) {
+      const environmentToCheck = environment || secretSync.environment?.slug || "";
+      const secretPathToCheck = secretPath || secretSync.folder?.path || "";
+
+      if (environmentToCheck && secretPathToCheck) {
+        ForbiddenError.from(permission).throwUnlessCan(
+          ProjectPermissionSecretSyncActions.Edit,
+          subject(ProjectPermissionSub.SecretSyncs, {
+            environment: environmentToCheck,
+            secretPath: secretPathToCheck
+          })
+        );
+      }
+    }
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
       throw new BadRequestError({
@@ -315,7 +375,7 @@ export const secretSyncServiceFactory = ({
       if (!updatedEnvironment || !updatedSecretPath)
         throw new BadRequestError({ message: "Must specify both source environment and secret path" });
 
-      throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, {
+      throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, {
         environment: updatedEnvironment,
         secretPath: updatedSecretPath
       });
@@ -374,10 +434,20 @@ export const secretSyncServiceFactory = ({
       projectId: secretSync.projectId
     });
 
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretSyncActions.Delete,
-      ProjectPermissionSub.SecretSyncs
-    );
+    if (secretSync.environment?.slug && secretSync.folder?.path) {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.Delete,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment: secretSync.environment.slug,
+          secretPath: secretSync.folder.path
+        })
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.Delete,
+        ProjectPermissionSub.SecretSyncs
+      );
+    }
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
       throw new BadRequestError({
@@ -441,10 +511,20 @@ export const secretSyncServiceFactory = ({
       projectId: secretSync.projectId
     });
 
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretSyncActions.SyncSecrets,
-      ProjectPermissionSub.SecretSyncs
-    );
+    if (secretSync.environment?.slug && secretSync.folder?.path) {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.SyncSecrets,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment: secretSync.environment.slug,
+          secretPath: secretSync.folder.path
+        })
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.SyncSecrets,
+        ProjectPermissionSub.SecretSyncs
+      );
+    }
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
       throw new BadRequestError({
@@ -503,10 +583,20 @@ export const secretSyncServiceFactory = ({
       projectId: secretSync.projectId
     });
 
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretSyncActions.ImportSecrets,
-      ProjectPermissionSub.SecretSyncs
-    );
+    if (secretSync.environment?.slug && secretSync.folder?.path) {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.ImportSecrets,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment: secretSync.environment.slug,
+          secretPath: secretSync.folder.path
+        })
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.ImportSecrets,
+        ProjectPermissionSub.SecretSyncs
+      );
+    }
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
       throw new BadRequestError({
@@ -559,10 +649,20 @@ export const secretSyncServiceFactory = ({
       projectId: secretSync.projectId
     });
 
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretSyncActions.RemoveSecrets,
-      ProjectPermissionSub.SecretSyncs
-    );
+    if (secretSync.environment?.slug && secretSync.folder?.path) {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.RemoveSecrets,
+        subject(ProjectPermissionSub.SecretSyncs, {
+          environment: secretSync.environment.slug,
+          secretPath: secretSync.folder.path
+        })
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretSyncActions.RemoveSecrets,
+        ProjectPermissionSub.SecretSyncs
+      );
+    }
 
     if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination])
       throw new BadRequestError({