review fixes + lint

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

@@ -5,7 +5,6 @@ import { TPermissionServiceFactory } from "@app/ee/services/permission/permissio
 import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission";
 import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors";
 import { groupBy } from "@app/lib/fn";
-import { TOrgMembershipDALFactory } from "@app/services/org-membership/org-membership-dal";
 import { TProjectDALFactory } from "@app/services/project/project-dal";
 import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal";
 import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal";
@@ -45,7 +44,6 @@ type TAccessApprovalPolicyServiceFactoryDep = {
   accessApprovalRequestDAL: Pick<TAccessApprovalRequestDALFactory, "update" | "find" | "resetReviewByPolicyId">;
   additionalPrivilegeDAL: Pick<TProjectUserAdditionalPrivilegeDALFactory, "delete">;
   accessApprovalRequestReviewerDAL: Pick<TAccessApprovalRequestReviewerDALFactory, "update" | "delete">;
-  orgMembershipDAL: Pick<TOrgMembershipDALFactory, "find">;
   accessApprovalPolicyEnvironmentDAL: TAccessApprovalPolicyEnvironmentDALFactory;
 };
 
@@ -62,7 +60,7 @@ export const accessApprovalPolicyServiceFactory = ({
   accessApprovalRequestDAL,
   additionalPrivilegeDAL,
   accessApprovalRequestReviewerDAL,
-  orgMembershipDAL
+  projectMembershipDAL
 }: TAccessApprovalPolicyServiceFactoryDep): TAccessApprovalPolicyServiceFactory => {
   const $policyExists = async ({
     envId,
@@ -85,6 +83,22 @@ export const accessApprovalPolicyServiceFactory = ({
     return policyId ? policy && policy.id !== policyId : Boolean(policy);
   };
 
+  const verifyProjectUserMembership = async (userIds: string[], projectId: string) => {
+    if (userIds.length === 0) return;
+    const projectMemberships = await projectMembershipDAL.find({
+      $in: { userId: userIds },
+      projectId
+    });
+
+    if (projectMemberships.length !== userIds.length) {
+      const projectMemberUserIds = new Set(projectMemberships.map((member) => member.userId));
+      const userIdsNotInProject = userIds.filter((id) => !projectMemberUserIds.has(id));
+      throw new BadRequestError({
+        message: `Some users are not members of the project: ${userIdsNotInProject.join(", ")}`
+      });
+    }
+  };
+
   const createAccessApprovalPolicy: TAccessApprovalPolicyServiceFactory["createAccessApprovalPolicy"] = async ({
     name,
     actor,
@@ -175,19 +189,10 @@ export const accessApprovalPolicyServiceFactory = ({
     }
 
     if (approverUserIds.length > 0) {
-      const allApproverUserIds = approverUserIds.map((au) => au.id);
-      const approverMembers = await orgMembershipDAL.find({
-        $in: { userId: allApproverUserIds },
-        orgId: actorOrgId
-      });
-
-      if (approverMembers.length !== allApproverUserIds.length) {
-        const approverMemberUserIds = new Set(approverMembers.map((member) => member.userId as string));
-        const userIdsNotInOrg = allApproverUserIds.filter((id) => !approverMemberUserIds.has(id));
-        throw new BadRequestError({
-          message: `Some approvers are not in the organization: ${userIdsNotInOrg.join(", ")}`
-        });
-      }
+      await verifyProjectUserMembership(
+        approverUserIds.map((au) => au.id),
+        project.id
+      );
     }
     let groupBypassers: string[] = [];
     let bypasserUserIds: string[] = [];
@@ -440,18 +445,7 @@ export const accessApprovalPolicyServiceFactory = ({
 
       // Validate user bypassers
       if (bypasserUserIds.length > 0) {
-        const orgMemberships = await orgMembershipDAL.find({
-          $in: { userId: bypasserUserIds },
-          orgId: actorOrgId
-        });
-
-        if (orgMemberships.length !== bypasserUserIds.length) {
-          const foundUserIdsInOrg = new Set(orgMemberships.map((mem) => mem.userId));
-          const missingUserIds = bypasserUserIds.filter((id) => !foundUserIdsInOrg.has(id));
-          throw new BadRequestError({
-            message: `One or more specified bypasser users are not part of the organization or do not exist. Invalid or non-member user IDs: ${missingUserIds.join(", ")}`
-          });
-        }
+        await verifyProjectUserMembership(bypasserUserIds, accessApprovalPolicy.projectId);
       }
 
       // Validate group bypassers
@@ -517,19 +511,10 @@ export const accessApprovalPolicyServiceFactory = ({
         }
 
         if (approverUserIds.length > 0) {
-          const allApproverUserIds = approverUserIds.map((au) => au.id);
-          const approverMembers = await orgMembershipDAL.find({
-            $in: { userId: allApproverUserIds },
-            orgId: actorOrgId
-          });
-
-          if (approverMembers.length !== allApproverUserIds.length) {
-            const approverMemberUserIds = new Set(approverMembers.map((member) => member.userId as string));
-            const userIdsNotInOrg = allApproverUserIds.filter((id) => !approverMemberUserIds.has(id));
-            throw new BadRequestError({
-              message: `Some approvers are not in the organization: ${userIdsNotInOrg.join(", ")}`
-            });
-          }
+          await verifyProjectUserMembership(
+            approverUserIds.map((au) => au.id),
+            accessApprovalPolicy.projectId
+          );
         }
         await accessApprovalPolicyApproverDAL.insertMany(
           approverUserIds.map((el) => ({

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

@@ -7,8 +7,8 @@ import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services
 import { BadRequestError, NotFoundError } from "@app/lib/errors";
 import { removeTrailingSlash } from "@app/lib/fn";
 import { containsGlobPatterns } from "@app/lib/picomatch";
-import { TOrgMembershipDALFactory } from "@app/services/org-membership/org-membership-dal";
 import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal";
+import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal";
 import { TUserDALFactory } from "@app/services/user/user-dal";
 
 import { ApproverType, BypasserType } from "../access-approval-policy/access-approval-policy-types";
@@ -40,7 +40,7 @@ type TSecretApprovalPolicyServiceFactoryDep = {
   secretApprovalPolicyDAL: TSecretApprovalPolicyDALFactory;
   projectEnvDAL: Pick<TProjectEnvDALFactory, "findOne" | "find">;
   userDAL: Pick<TUserDALFactory, "find">;
-  orgMembershipDAL: Pick<TOrgMembershipDALFactory, "find">;
+  projectMembershipDAL: Pick<TProjectMembershipDALFactory, "find">;
   secretApprovalPolicyApproverDAL: TSecretApprovalPolicyApproverDALFactory;
   secretApprovalPolicyBypasserDAL: TSecretApprovalPolicyBypasserDALFactory;
   licenseService: Pick<TLicenseServiceFactory, "getPlan">;
@@ -58,10 +58,26 @@ export const secretApprovalPolicyServiceFactory = ({
   secretApprovalPolicyEnvironmentDAL,
   projectEnvDAL,
   userDAL,
-  orgMembershipDAL,
+  projectMembershipDAL,
   licenseService,
   secretApprovalRequestDAL
 }: TSecretApprovalPolicyServiceFactoryDep) => {
+  const verifyProjectUserMembership = async (userIds: string[], projectId: string) => {
+    if (userIds.length === 0) return;
+    const projectMemberships = await projectMembershipDAL.find({
+      $in: { userId: userIds },
+      projectId
+    });
+
+    if (projectMemberships.length !== userIds.length) {
+      const projectMemberUserIds = new Set(projectMemberships.map((member) => member.userId));
+      const userIdsNotInProject = userIds.filter((id) => !projectMemberUserIds.has(id));
+      throw new BadRequestError({
+        message: `Some users are not members of the project: ${userIdsNotInProject.join(", ")}`
+      });
+    }
+  };
+
   const $policyExists = async ({
     envIds,
     envId,
@@ -205,6 +221,7 @@ export const secretApprovalPolicyServiceFactory = ({
         },
         tx
       );
+
       await secretApprovalPolicyEnvironmentDAL.insertMany(
         envs.map((env) => ({
           envId: env.id,
@@ -236,23 +253,7 @@ export const secretApprovalPolicyServiceFactory = ({
         userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id));
       }
 
-      if (userApproverIds.length > 0) {
-        const approverMembers = await orgMembershipDAL.find(
-          {
-            $in: { userId: userApproverIds },
-            orgId: actorOrgId
-          },
-          { tx }
-        );
-
-        if (approverMembers.length !== userApproverIds.length) {
-          const approverMemberUserIds = new Set(approverMembers.map((member) => member.userId as string));
-          const userIdsNotInOrg = userApproverIds.filter((id) => !approverMemberUserIds.has(id));
-          throw new BadRequestError({
-            message: `Some approvers are not in the organization: ${userIdsNotInOrg.join(", ")}`
-          });
-        }
-      }
+      await verifyProjectUserMembership(userApproverIds, projectId);
 
       await secretApprovalPolicyApproverDAL.insertMany(
         userApproverIds.map((approverUserId) => ({
@@ -446,23 +447,7 @@ export const secretApprovalPolicyServiceFactory = ({
           userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id));
         }
 
-        if (userApproverIds.length > 0) {
-          const approverMembers = await orgMembershipDAL.find(
-            {
-              $in: { userId: userApproverIds },
-              orgId: actorOrgId
-            },
-            { tx }
-          );
-
-          if (approverMembers.length !== userApproverIds.length) {
-            const approverMemberUserIds = new Set(approverMembers.map((member) => member.userId as string));
-            const userIdsNotInOrg = userApproverIds.filter((id) => !approverMemberUserIds.has(id));
-            throw new BadRequestError({
-              message: `Some approvers are not in the organization: ${userIdsNotInOrg.join(", ")}`
-            });
-          }
-        }
+        await verifyProjectUserMembership(userApproverIds, secretApprovalPolicy.projectId);
 
         await secretApprovalPolicyApproverDAL.insertMany(
           userApproverIds.map((approverUserId) => ({

backend/src/server/routes/index.ts

@@ -611,7 +611,7 @@ export const registerRoutes = async (
     secretApprovalPolicyDAL,
     licenseService,
     userDAL,
-    orgMembershipDAL,
+    projectMembershipDAL,
     secretApprovalRequestDAL
   });
   const tokenService = tokenServiceFactory({ tokenDAL: authTokenDAL, userDAL, orgMembershipDAL });
@@ -1421,8 +1421,7 @@ export const registerRoutes = async (
     userDAL,
     accessApprovalRequestDAL,
     additionalPrivilegeDAL: projectUserAdditionalPrivilegeDAL,
-    accessApprovalRequestReviewerDAL,
-    orgMembershipDAL
+    accessApprovalRequestReviewerDAL
   });
 
   const accessApprovalRequestService = accessApprovalRequestServiceFactory({

backend/src/server/routes/v1/index.ts

@@ -58,8 +58,8 @@ import { registerSecretRequestsRouter } from "./secret-requests-router";
 import { registerSecretSharingRouter } from "./secret-sharing-router";
 import { registerSecretTagRouter } from "./secret-tag-router";
 import { registerSlackRouter } from "./slack-router";
-import { registerUpgradePathRouter } from "./upgrade-path-router";
 import { registerSsoRouter } from "./sso-router";
+import { registerUpgradePathRouter } from "./upgrade-path-router";
 import { registerUserActionRouter } from "./user-action-router";
 import { registerUserEngagementRouter } from "./user-engagement-router";
 import { registerUserRouter } from "./user-router";