fix: error improvements

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

@@ -1,23 +1,21 @@
 import { ForbiddenError, subject } from "@casl/ability";
 
-import { BadRequestError, ForbiddenRequestError } from "@app/lib/errors";
 import { ActorType } from "@app/services/auth/auth-type";
 
 import { ProjectPermissionActions, ProjectPermissionSub } from "../permission/project-permission";
-import { TVerifyApprovers, VerifyApproversError } from "./access-approval-policy-types";
+import { TIsApproversValid } from "./access-approval-policy-types";
 
-export const verifyApprovers = async ({
+export const isApproversValid = async ({
   userIds,
   projectId,
   orgId,
   envSlug,
   actorAuthMethod,
   secretPath,
-  permissionService,
-  error
-}: TVerifyApprovers) => {
-  for await (const userId of userIds) {
-    try {
+  permissionService
+}: TIsApproversValid) => {
+  try {
+    for await (const userId of userIds) {
       const { permission: approverPermission } = await permissionService.getProjectPermission(
         ActorType.USER,
         userId,
@@ -30,17 +28,9 @@ export const verifyApprovers = async ({
         ProjectPermissionActions.Create,
         subject(ProjectPermissionSub.Secrets, { environment: envSlug, secretPath })
       );
-    } catch (err) {
-      if (error === VerifyApproversError.BadRequestError) {
-        throw new BadRequestError({
-          message: "One or more approvers doesn't have access to be specified secret path"
-        });
-      }
-      if (error === VerifyApproversError.ForbiddenError) {
-        throw new ForbiddenRequestError({
-          message: "You don't have access to approve this request"
-        });
-      }
     }
+  } catch (err) {
+    return false;
   }
+  return true;
 };

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

@@ -2,7 +2,7 @@ import { ForbiddenError } from "@casl/ability";
 
 import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service";
 import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission";
-import { BadRequestError, NotFoundError } from "@app/lib/errors";
+import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors";
 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";
@@ -11,7 +11,7 @@ import { TUserDALFactory } from "@app/services/user/user-dal";
 import { TGroupDALFactory } from "../group/group-dal";
 import { TAccessApprovalPolicyApproverDALFactory } from "./access-approval-policy-approver-dal";
 import { TAccessApprovalPolicyDALFactory } from "./access-approval-policy-dal";
-import { verifyApprovers } from "./access-approval-policy-fns";
+import { isApproversValid } from "./access-approval-policy-fns";
 import {
   ApproverType,
   TCreateAccessApprovalPolicy,
@@ -19,8 +19,7 @@ import {
   TGetAccessApprovalPolicyByIdDTO,
   TGetAccessPolicyCountByEnvironmentDTO,
   TListAccessApprovalPoliciesDTO,
-  TUpdateAccessApprovalPolicy,
-  VerifyApproversError
+  TUpdateAccessApprovalPolicy
 } from "./access-approval-policy-types";
 
 type TSecretApprovalPolicyServiceFactoryDep = {
@@ -133,17 +132,22 @@ export const accessApprovalPolicyServiceFactory = ({
       .map((user) => user.id);
     verifyAllApprovers.push(...verifyGroupApprovers);
 
-    await verifyApprovers({
+    const approversValid = await isApproversValid({
       projectId: project.id,
       orgId: actorOrgId,
       envSlug: environment,
       secretPath,
       actorAuthMethod,
       permissionService,
-      userIds: verifyAllApprovers,
-      error: VerifyApproversError.BadRequestError
+      userIds: verifyAllApprovers
     });
 
+    if (!approversValid) {
+      throw new BadRequestError({
+        message: "One or more approvers doesn't have access to be specified secret path"
+      });
+    }
+
     const accessApproval = await accessApprovalPolicyDAL.transaction(async (tx) => {
       const doc = await accessApprovalPolicyDAL.create(
         {
@@ -285,17 +289,22 @@ export const accessApprovalPolicyServiceFactory = ({
           userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id));
         }
 
-        await verifyApprovers({
+        const approversValid = await isApproversValid({
           projectId: accessApprovalPolicy.projectId,
           orgId: actorOrgId,
           envSlug: accessApprovalPolicy.environment.slug,
           secretPath: doc.secretPath!,
           actorAuthMethod,
           permissionService,
-          userIds: userApproverIds,
-          error: VerifyApproversError.BadRequestError
+          userIds: userApproverIds
         });
 
+        if (!approversValid) {
+          throw new BadRequestError({
+            message: "One or more approvers doesn't have access to be specified secret path"
+          });
+        }
+
         await accessApprovalPolicyApproverDAL.insertMany(
           userApproverIds.map((userId) => ({
             approverUserId: userId,
@@ -325,16 +334,22 @@ export const accessApprovalPolicyServiceFactory = ({
           .filter((user) => user.isPartOfGroup)
           .map((user) => user.id);
 
-        await verifyApprovers({
+        const approversValid = await isApproversValid({
           projectId: accessApprovalPolicy.projectId,
           orgId: actorOrgId,
           envSlug: accessApprovalPolicy.environment.slug,
           secretPath: doc.secretPath!,
           actorAuthMethod,
           permissionService,
-          userIds: verifyGroupApprovers,
-          error: VerifyApproversError.BadRequestError
+          userIds: verifyGroupApprovers
         });
+
+        if (!approversValid) {
+          throw new BadRequestError({
+            message: "One or more approvers doesn't have access to be specified secret path"
+          });
+        }
+
         await accessApprovalPolicyApproverDAL.insertMany(
           groupApprovers.map((groupId) => ({
             approverGroupId: groupId,
@@ -398,7 +413,9 @@ export const accessApprovalPolicyServiceFactory = ({
       actorAuthMethod,
       actorOrgId
     );
-    if (!membership) throw new NotFoundError({ message: "User not found in project" });
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
 
     const environment = await projectEnvDAL.findOne({ projectId: project.id, slug: envSlug });
     if (!environment) throw new NotFoundError({ message: "Environment not found" });

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

@@ -3,12 +3,7 @@ import { ActorAuthMethod } from "@app/services/auth/auth-type";
 
 import { TPermissionServiceFactory } from "../permission/permission-service";
 
-export enum VerifyApproversError {
-  ForbiddenError = "ForbiddenError",
-  BadRequestError = "BadRequestError"
-}
-
-export type TVerifyApprovers = {
+export type TIsApproversValid = {
   userIds: string[];
   permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
   envSlug: string;
@@ -16,7 +11,6 @@ export type TVerifyApprovers = {
   secretPath: string;
   projectId: string;
   orgId: string;
-  error: VerifyApproversError;
 };
 
 export enum ApproverType {