fix: requested changes

backend/src/db/migrations/20250218020306_backfill-secret-permissions-with-readvalue.ts

@@ -16,7 +16,7 @@ enum SecretActions {
   ReadValue = "readValue"
 }
 
-export const UnpackedPermissionSchema = z.object({
+const UnpackedPermissionSchema = z.object({
   subject: z
     .union([z.string().min(1), z.string().array()])
     .transform((el) => (typeof el !== "string" ? el[0] : el))
@@ -26,9 +26,59 @@ export const UnpackedPermissionSchema = z.object({
   inverted: z.boolean().optional()
 });
 
-export const unpackPermissions = (permissions: unknown) =>
+const $unpackPermissions = (permissions: unknown) =>
   UnpackedPermissionSchema.array().parse(unpackRules((permissions || []) as PackRule<RawRuleOf<MongoAbility>>[]));
 
+const $updatePermissionsUp = (permissions: unknown) => {
+  const parsedPermissions = $unpackPermissions(permissions);
+  let shouldUpdate = false;
+
+  for (let i = 0; i < parsedPermissions.length; i += 1) {
+    const parsedPermission = parsedPermissions[i];
+    const { subject, action } = parsedPermission;
+
+    if (subject === ProjectPermissionSub.Secrets) {
+      if (action.includes(SecretActions.Read) && !action.includes(SecretActions.ReadValue)) {
+        action.push(SecretActions.ReadValue);
+        parsedPermissions[i] = { ...parsedPermission, action };
+        shouldUpdate = true;
+      }
+    }
+  }
+
+  return {
+    parsedPermissions,
+    shouldUpdate
+  };
+};
+
+const $updatePermissionsDown = (permissions: unknown) => {
+  const parsedPermissions = $unpackPermissions(permissions);
+
+  let shouldUpdate = false;
+  for (let i = 0; i < parsedPermissions.length; i += 1) {
+    const parsedPermission = parsedPermissions[i];
+
+    const { subject, action } = parsedPermission;
+
+    if (subject === ProjectPermissionSub.Secrets) {
+      if (action.includes(SecretActions.ReadValue)) {
+        action.splice(action.indexOf(SecretActions.ReadValue));
+        parsedPermissions[i] = { ...parsedPermission, action };
+
+        shouldUpdate = true;
+      }
+    }
+  }
+
+  const repackedPermissions = packRules(parsedPermissions);
+
+  return {
+    repackedPermissions,
+    shouldUpdate
+  };
+};
+
 export async function up(knex: Knex): Promise<void> {
   const projectRoles = await knex(TableName.ProjectRoles).select(selectAllTableCols(TableName.ProjectRoles));
   const projectIdentityAdditionalPrivileges = await knex(TableName.IdentityProjectAdditionalPrivilege).select(
@@ -38,88 +88,62 @@ export async function up(knex: Knex): Promise<void> {
     selectAllTableCols(TableName.ProjectUserAdditionalPrivilege)
   );
 
-  for await (const projectRole of projectRoles) {
-    const { permissions } = projectRole;
-
-    const parsedPermissions = unpackPermissions(permissions);
-    let shouldUpdate = false;
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.Read) && !action.includes(SecretActions.ReadValue)) {
-          action.push(SecretActions.ReadValue);
-          parsedPermissions[i] = { ...parsedPermission, action };
-          shouldUpdate = true;
-        }
-      }
-    }
+  const updatedRoles = projectRoles.reduce<typeof projectRoles>((acc, projectRole) => {
+    const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(projectRole.permissions);
 
     if (shouldUpdate) {
-      const repackedPermissions = packRules(parsedPermissions);
-
-      await knex(TableName.ProjectRoles)
-        .where("id", projectRole.id)
-        .update({ permissions: JSON.stringify(repackedPermissions) });
+      acc.push({
+        ...projectRole,
+        permissions: JSON.stringify(packRules(parsedPermissions))
+      });
     }
+    return acc;
+  }, []);
+
+  const updatedIdentityAdditionalPrivileges = projectIdentityAdditionalPrivileges.reduce<
+    typeof projectIdentityAdditionalPrivileges
+  >((acc, identityAdditionalPrivilege) => {
+    const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(identityAdditionalPrivilege.permissions);
+
+    if (shouldUpdate) {
+      acc.push({
+        ...identityAdditionalPrivilege,
+        permissions: JSON.stringify(packRules(parsedPermissions))
+      });
+    }
+    return acc;
+  }, []);
+
+  const updatedUserAdditionalPrivileges = projectUserAdditionalPrivileges.reduce<
+    typeof projectUserAdditionalPrivileges
+  >((acc, userAdditionalPrivilege) => {
+    const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(userAdditionalPrivilege.permissions);
+
+    if (shouldUpdate) {
+      acc.push({
+        ...userAdditionalPrivilege,
+        permissions: JSON.stringify(packRules(parsedPermissions))
+      });
+    }
+    return acc;
+  }, []);
+
+  if (updatedRoles.length > 0) {
+    await knex(TableName.ProjectRoles).insert(updatedRoles).onConflict("id").merge(["permissions"]);
   }
 
-  for await (const identityAdditionalPrivilege of projectIdentityAdditionalPrivileges) {
-    const { permissions } = identityAdditionalPrivilege;
-
-    const parsedPermissions = unpackPermissions(permissions);
-    let shouldUpdate = false;
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.Read) && !action.includes(SecretActions.ReadValue)) {
-          action.push(SecretActions.ReadValue);
-          parsedPermissions[i] = { ...parsedPermission, action };
-          shouldUpdate = true;
-        }
-      }
-    }
-
-    if (shouldUpdate) {
-      const repackedPermissions = packRules(parsedPermissions);
-
-      await knex(TableName.IdentityProjectAdditionalPrivilege)
-        .where("id", identityAdditionalPrivilege.id)
-        .update({ permissions: JSON.stringify(repackedPermissions) });
-    }
+  if (updatedIdentityAdditionalPrivileges.length > 0) {
+    await knex(TableName.IdentityProjectAdditionalPrivilege)
+      .insert(updatedIdentityAdditionalPrivileges)
+      .onConflict("id")
+      .merge(["permissions"]);
   }
 
-  for await (const userAdditionalPrivilege of projectUserAdditionalPrivileges) {
-    const { permissions } = userAdditionalPrivilege;
-
-    const parsedPermissions = unpackPermissions(permissions);
-    let shouldUpdate = false;
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.Read) && !action.includes(SecretActions.ReadValue)) {
-          action.push(SecretActions.ReadValue);
-          parsedPermissions[i] = { ...parsedPermission, action };
-          shouldUpdate = true;
-        }
-      }
-    }
-
-    if (shouldUpdate) {
-      const repackedPermissions = packRules(parsedPermissions);
-
-      await knex(TableName.ProjectUserAdditionalPrivilege)
-        .where("id", userAdditionalPrivilege.id)
-        .update({ permissions: JSON.stringify(repackedPermissions) });
-    }
+  if (updatedUserAdditionalPrivileges.length > 0) {
+    await knex(TableName.ProjectUserAdditionalPrivilege)
+      .insert(updatedUserAdditionalPrivileges)
+      .onConflict("id")
+      .merge(["permissions"]);
   }
 }
 
@@ -132,78 +156,63 @@ export async function down(knex: Knex): Promise<void> {
     selectAllTableCols(TableName.ProjectUserAdditionalPrivilege)
   );
 
-  for await (const projectRole of projectRoles) {
-    const { permissions } = projectRole;
+  const updatedRoles = projectRoles.reduce<typeof projectRoles>((acc, projectRole) => {
+    const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(projectRole.permissions);
 
-    const parsedPermissions = unpackPermissions(permissions);
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.ReadValue)) {
-          action.splice(action.indexOf(SecretActions.ReadValue));
-          parsedPermissions[i] = { ...parsedPermission, action };
-        }
-      }
+    if (shouldUpdate) {
+      acc.push({
+        ...projectRole,
+        permissions: JSON.stringify(repackedPermissions)
+      });
     }
+    return acc;
+  }, []);
 
-    const repackedPermissions = packRules(parsedPermissions);
+  const updatedIdentityAdditionalPrivileges = identityAdditionalPrivileges.reduce<typeof identityAdditionalPrivileges>(
+    (acc, identityAdditionalPrivilege) => {
+      const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(identityAdditionalPrivilege.permissions);
 
-    await knex(TableName.ProjectRoles)
-      .where("id", projectRole.id)
-      .update({ permissions: JSON.stringify(repackedPermissions) });
+      if (shouldUpdate) {
+        acc.push({
+          ...identityAdditionalPrivilege,
+          permissions: JSON.stringify(repackedPermissions)
+        });
+      }
+      return acc;
+    },
+    []
+  );
+
+  const updatedUserAdditionalPrivileges = userAdditionalPrivileges.reduce<typeof userAdditionalPrivileges>(
+    (acc, userAdditionalPrivilege) => {
+      const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(userAdditionalPrivilege.permissions);
+
+      if (shouldUpdate) {
+        acc.push({
+          ...userAdditionalPrivilege,
+          permissions: JSON.stringify(repackedPermissions)
+        });
+      }
+      return acc;
+    },
+    []
+  );
+
+  if (updatedRoles.length > 0) {
+    await knex(TableName.ProjectRoles).insert(updatedRoles).onConflict("id").merge(["permissions"]);
   }
 
-  for await (const identityAdditionalPrivilege of identityAdditionalPrivileges) {
-    const { permissions } = identityAdditionalPrivilege;
-
-    const parsedPermissions = unpackPermissions(permissions);
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.ReadValue)) {
-          action.splice(action.indexOf(SecretActions.ReadValue));
-          parsedPermissions[i] = { ...parsedPermission, action };
-        }
-      }
-    }
-
-    const repackedPermissions = packRules(parsedPermissions);
-
+  if (updatedIdentityAdditionalPrivileges.length > 0) {
     await knex(TableName.IdentityProjectAdditionalPrivilege)
-      .where("id", identityAdditionalPrivilege.id)
-      .update({ permissions: JSON.stringify(repackedPermissions) });
+      .insert(updatedIdentityAdditionalPrivileges)
+      .onConflict("id")
+      .merge(["permissions"]);
   }
 
-  for await (const userAdditionalPrivilege of userAdditionalPrivileges) {
-    const { permissions } = userAdditionalPrivilege;
-
-    const parsedPermissions = unpackPermissions(permissions);
-
-    for (let i = 0; i < parsedPermissions.length; i += 1) {
-      const parsedPermission = parsedPermissions[i];
-
-      const { subject, action } = parsedPermission;
-
-      if (subject === ProjectPermissionSub.Secrets) {
-        if (action.includes(SecretActions.ReadValue)) {
-          action.splice(action.indexOf(SecretActions.ReadValue));
-          parsedPermissions[i] = { ...parsedPermission, action };
-        }
-      }
-    }
-
-    const repackedPermissions = packRules(parsedPermissions);
-
+  if (updatedUserAdditionalPrivileges.length > 0) {
     await knex(TableName.ProjectUserAdditionalPrivilege)
-      .where("id", userAdditionalPrivilege.id)
-      .update({ permissions: JSON.stringify(repackedPermissions) });
+      .insert(updatedUserAdditionalPrivileges)
+      .onConflict("id")
+      .merge(["permissions"]);
   }
 }

backend/src/ee/services/permission/project-permission.ts

@@ -437,11 +437,12 @@ const GeneralPermissionSchema = [
   })
 ];
 
+// Do not update this schema anymore, as it's kept purely for backwards compatability. Update V2 schema only.
 export const ProjectPermissionV1Schema = z.discriminatedUnion("subject", [
   z.object({
     subject: z.literal(ProjectPermissionSub.Secrets).describe("The entity this permission pertains to."),
     inverted: z.boolean().optional().describe("Whether rule allows or forbids."),
-    action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionSecretActions).describe(
+    action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe(
       "Describe what action an entity can take."
     ),
     conditions: SecretConditionV1Schema.describe(