feat: addressed backend review changes needed by scott

backend/src/db/migrations/20241008172622_project-permission-split.ts

@@ -39,6 +39,8 @@ export async function up(knex: Knex): Promise<void> {
       }
     }
 
+    // secret permission is split into multiple ones like secrets, folders, imports and dynamic-secrets
+    // so we just find all the privileges with respective mapping and map it as needed
     const identityPrivileges = await knex(TableName.IdentityProjectAdditionalPrivilege).select("*");
     const updatedIdentityPrivilegesDocs = identityPrivileges
       .filter((i) => {
@@ -63,7 +65,7 @@ export async function up(knex: Knex): Promise<void> {
     }
 
     const userPrivileges = await knex(TableName.ProjectUserAdditionalPrivilege).select("*");
-    const updatedUserPriviegeDocs = userPrivileges
+    const updatedUserPrivilegeDocs = userPrivileges
       .filter((i) => {
         const permissionString = JSON.stringify(i.permissions || []);
         return (
@@ -79,8 +81,8 @@ export async function up(knex: Knex): Promise<void> {
         permissions: JSON.stringify(packRules(backfillPermissionV1SchemaToV2Schema(unpackRules(el.permissions))))
       }));
     if (docs.length) {
-      for (let i = 0; i < updatedUserPriviegeDocs.length; i += CHUNK_SIZE) {
-        const chunk = updatedUserPriviegeDocs.slice(i, i + CHUNK_SIZE);
+      for (let i = 0; i < updatedUserPrivilegeDocs.length; i += CHUNK_SIZE) {
+        const chunk = updatedUserPrivilegeDocs.slice(i, i + CHUNK_SIZE);
         await knex(TableName.ProjectUserAdditionalPrivilege).insert(chunk).onConflict("id").merge();
       }
     }

backend/src/ee/routes/v2/project-role-router.ts

@@ -203,14 +203,14 @@ export const registerProjectRoleRouter = async (server: FastifyZodProvider) => {
 
   server.route({
     method: "GET",
-    url: "/:projectSlug/roles/slug/:slug",
+    url: "/:projectSlug/roles/slug/:roleSlug",
     config: {
       rateLimit: readLimit
     },
     schema: {
       params: z.object({
         projectSlug: z.string().trim().describe(PROJECT_ROLE.GET_ROLE_BY_SLUG.projectSlug),
-        slug: z.string().trim().describe(PROJECT_ROLE.GET_ROLE_BY_SLUG.roleSlug)
+        roleSlug: z.string().trim().describe(PROJECT_ROLE.GET_ROLE_BY_SLUG.roleSlug)
       }),
       response: {
         200: z.object({
@@ -226,7 +226,7 @@ export const registerProjectRoleRouter = async (server: FastifyZodProvider) => {
         actorOrgId: req.permission.orgId,
         actor: req.permission.type,
         projectSlug: req.params.projectSlug,
-        roleSlug: req.params.slug
+        roleSlug: req.params.roleSlug
       });
       return { role };
     }

backend/src/ee/services/access-approval-policy/access-approval-policy-fns.ts

@@ -1,36 +0,0 @@
-import { ForbiddenError, subject } from "@casl/ability";
-
-import { ActorType } from "@app/services/auth/auth-type";
-
-import { ProjectPermissionActions, ProjectPermissionSub } from "../permission/project-permission";
-import { TIsApproversValid } from "./access-approval-policy-types";
-
-export const isApproversValid = async ({
-  userIds,
-  projectId,
-  orgId,
-  envSlug,
-  actorAuthMethod,
-  secretPath,
-  permissionService
-}: TIsApproversValid) => {
-  try {
-    for await (const userId of userIds) {
-      const { permission: approverPermission } = await permissionService.getProjectPermission(
-        ActorType.USER,
-        userId,
-        projectId,
-        actorAuthMethod,
-        orgId
-      );
-
-      ForbiddenError.from(approverPermission).throwUnlessCan(
-        ProjectPermissionActions.Create,
-        subject(ProjectPermissionSub.Secrets, { environment: envSlug, secretPath, secretName: "", secretTags: [] })
-      );
-    }
-  } catch {
-    return false;
-  }
-  return true;
-};

backend/src/ee/services/dynamic-secret-lease/dynamic-secret-lease-service.ts

@@ -41,7 +41,6 @@ type TDynamicSecretLeaseServiceFactoryDep = {
 
 export type TDynamicSecretLeaseServiceFactory = ReturnType<typeof dynamicSecretLeaseServiceFactory>;
 
-// TODO(casl): change the lease to different permission
 export const dynamicSecretLeaseServiceFactory = ({
   dynamicSecretLeaseDAL,
   dynamicSecretProviders,

backend/src/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service.ts

@@ -32,8 +32,6 @@ export type TIdentityProjectAdditionalPrivilegeServiceFactory = ReturnType<
   typeof identityProjectAdditionalPrivilegeServiceFactory
 >;
 
-// TODO(akhilmhdh): move this to more centralized
-
 const unpackPermissions = (permissions: unknown) =>
   UnpackedPermissionSchema.array().parse(
     unpackRules((permissions || []) as PackRule<RawRuleOf<MongoAbility<ProjectPermissionSet>>>[])

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

@@ -142,7 +142,7 @@ const CASL_ACTION_SCHEMA_NATIVE_ENUM = <ACTION extends z.EnumLike>(actions: ACTI
 const CASL_ACTION_SCHEMA_ENUM = <ACTION extends z.EnumValues>(actions: ACTION) =>
   z.union([z.enum(actions), z.enum(actions).array().min(1)]).transform((el) => (typeof el === "string" ? [el] : el));
 
-// akhilmhdh: don't mondify this for v2
+// akhilmhdh: don't modify this for v2
 // if you want to update create a new schema
 const SecretConditionV1Schema = z
   .object({
@@ -339,6 +339,13 @@ const GeneralPermissionSchema = [
     action: CASL_ACTION_SCHEMA_ENUM([ProjectPermissionActions.Edit]).describe(
       "Describe what action an entity can take."
     )
+  }),
+  z.object({
+    subject: z.literal(ProjectPermissionSub.Cmek).describe("The entity this permission pertains to."),
+    inverted: z.boolean().optional().describe("Whether rule allows or forbids."),
+    action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionCmekActions).describe(
+      "Describe what action an entity can take."
+    )
   })
 ];
 
@@ -360,13 +367,6 @@ export const ProjectPermissionV1Schema = z.discriminatedUnion("subject", [
       "Describe what action an entity can take."
     )
   }),
-  z.object({
-    subject: z.literal(ProjectPermissionSub.Cmek).describe("The entity this permission pertains to."),
-    inverted: z.boolean().optional().describe("Whether rule allows or forbids."),
-    action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionCmekActions).describe(
-      "Describe what action an entity can take."
-    )
-  }),
   ...GeneralPermissionSchema
 ]);
 

backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts

@@ -74,7 +74,10 @@ export const projectUserAdditionalPrivilegeServiceFactory = ({
         slug,
         permissions: customPermission
       });
-      return additionalPrivilege;
+      return {
+        ...additionalPrivilege,
+        permissions: unpackPermissions(additionalPrivilege.permissions)
+      };
     }
 
     const relativeTempAllocatedTimeInMs = ms(dto.temporaryRange);

backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts

@@ -43,7 +43,7 @@ import {
   fnSecretBulkDelete as fnSecretV2BridgeBulkDelete,
   fnSecretBulkInsert as fnSecretV2BridgeBulkInsert,
   fnSecretBulkUpdate as fnSecretV2BridgeBulkUpdate,
-  getAllSecretReferences as getAllNestedSecretReferencesV2Bridge
+  getAllSecretReferences as getAllSecretReferencesV2Bridge
 } from "@app/services/secret-v2-bridge/secret-v2-bridge-fns";
 import { TSecretVersionV2DALFactory } from "@app/services/secret-v2-bridge/secret-version-dal";
 import { TSecretVersionV2TagDALFactory } from "@app/services/secret-v2-bridge/secret-version-tag-dal";
@@ -523,7 +523,7 @@ export const secretApprovalRequestServiceFactory = ({
                 skipMultilineEncoding: el.skipMultilineEncoding,
                 key: el.key,
                 references: el.encryptedValue
-                  ? getAllNestedSecretReferencesV2Bridge(
+                  ? getAllSecretReferencesV2Bridge(
                       secretManagerDecryptor({
                         cipherTextBlob: el.encryptedValue
                       }).toString()
@@ -547,7 +547,7 @@ export const secretApprovalRequestServiceFactory = ({
                     ? {
                         encryptedValue: el.encryptedValue as Buffer,
                         references: el.encryptedValue
-                          ? getAllNestedSecretReferencesV2Bridge(
+                          ? getAllSecretReferencesV2Bridge(
                               secretManagerDecryptor({
                                 cipherTextBlob: el.encryptedValue
                               }).toString()

backend/src/ee/services/secret-replication/secret-replication-service.ts

@@ -28,7 +28,6 @@ import { TSecretV2BridgeDALFactory } from "@app/services/secret-v2-bridge/secret
 import {
   fnSecretBulkInsert as fnSecretV2BridgeBulkInsert,
   fnSecretBulkUpdate as fnSecretV2BridgeBulkUpdate,
-  getAllSecretReferences as getAllNestedSecretReferencesV2Bridge,
   getAllSecretReferences
 } from "@app/services/secret-v2-bridge/secret-v2-bridge-fns";
 import { TSecretVersionV2DALFactory } from "@app/services/secret-v2-bridge/secret-version-dal";
@@ -258,7 +257,6 @@ export const secretReplicationServiceFactory = ({
         folderDAL,
         secretImportDAL,
         decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : ""),
-        // TODO(casl): check with team
         hasSecretAccess: () => true
       });
       // secrets that gets replicated across imports
@@ -418,9 +416,7 @@ export const secretReplicationServiceFactory = ({
                         encryptedValue: doc.encryptedValue,
                         encryptedComment: doc.encryptedComment,
                         skipMultilineEncoding: doc.skipMultilineEncoding,
-                        references: doc.secretValue
-                          ? getAllNestedSecretReferencesV2Bridge(doc.secretValue).nestedReferences
-                          : []
+                        references: doc.secretValue ? getAllSecretReferences(doc.secretValue).nestedReferences : []
                       };
                     })
                   });
@@ -446,9 +442,7 @@ export const secretReplicationServiceFactory = ({
                           encryptedValue: doc.encryptedValue as Buffer,
                           encryptedComment: doc.encryptedComment,
                           skipMultilineEncoding: doc.skipMultilineEncoding,
-                          references: doc.secretValue
-                            ? getAllNestedSecretReferencesV2Bridge(doc.secretValue).nestedReferences
-                            : []
+                          references: doc.secretValue ? getAllSecretReferences(doc.secretValue).nestedReferences : []
                         }
                       };
                     })

backend/src/lib/fn/array.ts

@@ -81,3 +81,25 @@ export const chunkArray = <T>(array: T[], chunkSize: number): T[][] => {
   }
   return chunks;
 };
+
+/*
+ * Returns all items from the first list that
+ * do not exist in the second list.
+ */
+export const diff = <T>(
+  root: readonly T[],
+  other: readonly T[],
+  identity: (item: T) => string | number | symbol = (t: T) => t as unknown as string | number | symbol
+): T[] => {
+  if (!root?.length && !other?.length) return [];
+  if (root?.length === undefined) return [...other];
+  if (!other?.length) return [...root];
+  const bKeys = other.reduce(
+    (acc, item) => {
+      acc[identity(item)] = true;
+      return acc;
+    },
+    {} as Record<string | number | symbol, boolean>
+  );
+  return root.filter((a) => !bKeys[identity(a)]);
+};

backend/src/lib/knex/dynamic.ts

@@ -21,10 +21,6 @@ type TKnexGroupOperator<T extends object> = {
   value: (TKnexNonGroupOperator<T> | TKnexGroupOperator<T>)[];
 };
 
-// akhilmhdh: This is still in pending state and not yet ready. If you want to use it ping me.
-// used when you need to write a complex query with the orm
-// use it when you need complex or and and condition - most of the time not needed
-// majorly used with casl permission to filter data based on permission
 export type TKnexDynamicOperator<T extends object> = TKnexGroupOperator<T> | TKnexNonGroupOperator<T>;
 
 export const buildDynamicKnexQuery = <T extends object>(

backend/src/server/routes/v1/dashboard-router.ts

@@ -195,7 +195,7 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => {
         req.permission.orgId
       );
 
-      const allowedDynamicSecretEnviroments = // filter envs user has access to
+      const allowedDynamicSecretEnvironments = // filter envs user has access to
         environments.filter((environment) =>
           permission.can(
             ProjectPermissionDynamicSecretActions.Lease,
@@ -203,7 +203,7 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => {
           )
         );
 
-      if (includeDynamicSecrets && allowedDynamicSecretEnviroments.length) {
+      if (includeDynamicSecrets && allowedDynamicSecretEnvironments.length) {
         // this is the unique count, ie duplicate secrets across envs only count as 1
         totalDynamicSecretCount = await server.services.dynamicSecret.getCountMultiEnv({
           actor: req.permission.type,
@@ -212,7 +212,7 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => {
           actorOrgId: req.permission.orgId,
           projectId,
           search,
-          environmentSlugs: allowedDynamicSecretEnviroments,
+          environmentSlugs: allowedDynamicSecretEnvironments,
           path: secretPath,
           isInternal: true
         });
@@ -227,7 +227,7 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => {
             search,
             orderBy,
             orderDirection,
-            environmentSlugs: allowedDynamicSecretEnviroments,
+            environmentSlugs: allowedDynamicSecretEnvironments,
             path: secretPath,
             limit: remainingLimit,
             offset: adjustedOffset,

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

@@ -543,9 +543,18 @@ export const secretImportServiceFactory = ({
     // this will already order by position
     // so anything based on this order will also be in right position
     const secretImports = await secretImportDAL.find({ folderId: folder.id, isReplication: false });
-
-    // TODO(casl): update here
-    return fnSecretsFromImports({ allowedImports: secretImports, folderDAL, secretDAL, secretImportDAL });
+    const allowedImports = secretImports.filter((el) =>
+      permission.can(
+        ProjectPermissionActions.Read,
+        subject(ProjectPermissionSub.Secrets, {
+          environment: el.importEnv.slug,
+          secretPath: el.importPath,
+          secretName: "",
+          secretTags: []
+        })
+      )
+    );
+    return fnSecretsFromImports({ allowedImports, folderDAL, secretDAL, secretImportDAL });
   };
 
   const getRawSecretsFromImports = async ({
@@ -606,8 +615,19 @@ export const secretImportServiceFactory = ({
         name: "bot_not_found_error"
       });
 
+    const allowedImports = secretImports.filter((el) =>
+      permission.can(
+        ProjectPermissionActions.Read,
+        subject(ProjectPermissionSub.Secrets, {
+          environment: el.importEnv.slug,
+          secretPath: el.importPath,
+          secretName: "",
+          secretTags: []
+        })
+      )
+    );
     const importedSecrets = await fnSecretsFromImports({
-      allowedImports: secretImports,
+      allowedImports,
       folderDAL,
       secretDAL,
       secretImportDAL

backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts

@@ -107,7 +107,7 @@ export const secretV2BridgeDALFactory = (db: TDbClient) => {
       });
       return data;
     } catch (error) {
-      throw new DatabaseError({ error, name: `${TableName.SecretV2}: FindOne` });
+      throw new DatabaseError({ error, name: `${TableName.SecretV2}: Find` });
     }
   };
 

backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts

@@ -434,24 +434,24 @@ export const expandSecretReferencesFactory = ({
             const [secretKey] = entities;
 
             // eslint-disable-next-line no-continue,no-await-in-loop
-            const referedValue = await fetchSecret(environment, secretPath, secretKey);
-            if (!canExpandValue(environment, secretPath, secretKey, referedValue.tags))
+            const referredValue = await fetchSecret(environment, secretPath, secretKey);
+            if (!canExpandValue(environment, secretPath, secretKey, referredValue.tags))
               throw new ForbiddenRequestError({
                 message: `You are attempting to reference secret named ${secretKey} from environment ${environment} in path ${secretPath} which you do not have access to.`
               });
 
             const cacheKey = getCacheUniqueKey(environment, secretPath);
-            secretCache[cacheKey][secretKey] = referedValue;
-            if (INTERPOLATION_SYNTAX_REG.test(referedValue.value)) {
+            secretCache[cacheKey][secretKey] = referredValue;
+            if (INTERPOLATION_SYNTAX_REG.test(referredValue.value)) {
               stack.push({
-                value: referedValue.value,
+                value: referredValue.value,
                 secretPath,
                 environment,
                 depth: depth + 1
               });
             }
-            if (referedValue) {
-              expandedValue = expandedValue.replaceAll(interpolationSyntax, referedValue.value);
+            if (referredValue) {
+              expandedValue = expandedValue.replaceAll(interpolationSyntax, referredValue.value);
             }
           } else {
             const secretReferenceEnvironment = entities[0];

backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts

@@ -9,7 +9,7 @@ import { TSecretApprovalRequestDALFactory } from "@app/ee/services/secret-approv
 import { TSecretApprovalRequestSecretDALFactory } from "@app/ee/services/secret-approval-request/secret-approval-request-secret-dal";
 import { TSecretSnapshotServiceFactory } from "@app/ee/services/secret-snapshot/secret-snapshot-service";
 import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors";
-import { groupBy } from "@app/lib/fn";
+import { diff, groupBy } from "@app/lib/fn";
 import { setKnexStringValue } from "@app/lib/knex";
 import { logger } from "@app/lib/logger";
 import { alphaNumericNanoId } from "@app/lib/nanoid";
@@ -105,7 +105,12 @@ export const secretV2BridgeServiceFactory = ({
     const uniqueReferenceEnvironmentSlugs = Array.from(new Set(references.map((el) => el.environment)));
     const referencesEnvironments = await projectEnvDAL.findBySlugs(projectId, uniqueReferenceEnvironmentSlugs);
     if (referencesEnvironments.length !== uniqueReferenceEnvironmentSlugs.length)
-      throw new BadRequestError({ message: "Referred environment not found" });
+      throw new BadRequestError({
+        message: `Referenced environment not found. Missing ${diff(
+          uniqueReferenceEnvironmentSlugs,
+          referencesEnvironments.map((el) => el.slug)
+        ).join(",")}`
+      });
 
     const referencesEnvironmentGroupBySlug = groupBy(referencesEnvironments, (i) => i.slug);
     const referredFolders = await folderDAL.findByManySecretPath(
@@ -122,7 +127,7 @@ export const secretV2BridgeServiceFactory = ({
           const folderId =
             referencesFolderGroupByPath[`${referencesEnvironmentGroupBySlug[el.environment][0].id}-${el.secretPath}`][0]
               ?.id;
-          if (!folderId) throw new BadRequestError({ message: `Reference path ${el.secretPath} doesn't exist` });
+          if (!folderId) throw new BadRequestError({ message: `Referenced path ${el.secretPath} doesn't exist` });
 
           return {
             operator: "and",
@@ -144,7 +149,12 @@ export const secretV2BridgeServiceFactory = ({
     });
 
     if (referredSecrets.length !== references.length)
-      throw new BadRequestError({ message: "Reference secret not found" });
+      throw new BadRequestError({
+        message: `Referenced secret not found. Found only ${diff(
+          references.map((el) => el.secretKey),
+          referredSecrets.map((el) => el.key)
+        ).join(",")}`
+      });
 
     const referredSecretsGroupBySecretKey = groupBy(referredSecrets, (i) => i.key);
     references.forEach((el) => {
@@ -210,7 +220,8 @@ export const secretV2BridgeServiceFactory = ({
     // validate tags
     // fetch all tags and if not same count throw error meaning one was invalid tags
     const tags = inputSecret.tagIds ? await secretTagDAL.find({ projectId, $in: { id: inputSecret.tagIds } }) : [];
-    if ((inputSecret.tagIds || []).length !== tags.length) throw new NotFoundError({ message: "Tag not found" });
+    if ((inputSecret.tagIds || []).length !== tags.length)
+      throw new NotFoundError({ message: `Tag not found. Found ${tags.map((el) => el.slug).join(",")}` });
 
     const { secretName, type, ...inputSecretData } = inputSecret;
 
@@ -357,7 +368,8 @@ export const secretV2BridgeServiceFactory = ({
     // validate tags
     // fetch all tags and if not same count throw error meaning one was invalid tags
     const tags = inputSecret.tagIds ? await secretTagDAL.find({ projectId, $in: { id: inputSecret.tagIds } }) : [];
-    if ((inputSecret.tagIds || []).length !== tags.length) throw new NotFoundError({ message: "Tag not found" });
+    if ((inputSecret.tagIds || []).length !== tags.length)
+      throw new NotFoundError({ message: `Tag not found. Found ${tags.map((el) => el.slug).join(",")}` });
 
     // now check with new ids
     ForbiddenError.from(permission).throwUnlessCan(
@@ -505,7 +517,7 @@ export const secretV2BridgeServiceFactory = ({
     });
     if (!secretToDelete) throw new NotFoundError({ message: "Secret not found" });
     ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionActions.Create,
+      ProjectPermissionActions.Delete,
       subject(ProjectPermissionSub.Secrets, {
         environment,
         secretPath,
@@ -1088,7 +1100,8 @@ export const secretV2BridgeServiceFactory = ({
     // get all tags
     const sanitizedTagIds = inputSecrets.flatMap(({ tagIds = [] }) => tagIds);
     const tags = sanitizedTagIds.length ? await secretTagDAL.findManyTagsById(projectId, sanitizedTagIds) : [];
-    if (tags.length !== sanitizedTagIds.length) throw new NotFoundError({ message: "Tag not found" });
+    if (tags.length !== sanitizedTagIds.length)
+      throw new NotFoundError({ message: `Tag not found. Found ${tags.map((el) => el.slug).join(",")}` });
     const tagsGroupByID = groupBy(tags, (i) => i.id);
 
     inputSecrets.forEach((el) => {