fix(api): 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 {
+    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,16 +11,15 @@ 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,
   TDeleteAccessApprovalPolicy,
   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,20 +3,14 @@ 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;
   actorAuthMethod: ActorAuthMethod;
   secretPath: string;
   projectId: string;
   orgId: string;
-  error: VerifyApproversError;
 };
 
 export enum ApproverType {

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

@@ -17,8 +17,7 @@ import { TUserDALFactory } from "@app/services/user/user-dal";
 
 import { TAccessApprovalPolicyApproverDALFactory } from "../access-approval-policy/access-approval-policy-approver-dal";
 import { TAccessApprovalPolicyDALFactory } from "../access-approval-policy/access-approval-policy-dal";
-import { verifyApprovers } from "../access-approval-policy/access-approval-policy-fns";
-import { VerifyApproversError } from "../access-approval-policy/access-approval-policy-types";
+import { isApproversValid } from "../access-approval-policy/access-approval-policy-fns";
 import { TGroupDALFactory } from "../group/group-dal";
 import { TPermissionServiceFactory } from "../permission/permission-service";
 import { TProjectUserAdditionalPrivilegeDALFactory } from "../project-user-additional-privilege/project-user-additional-privilege-dal";
@@ -100,7 +99,7 @@ export const accessApprovalRequestServiceFactory = ({
   }: TCreateAccessApprovalRequestDTO) => {
     const cfg = getConfig();
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
-    if (!project) throw new ForbiddenRequestError({ message: "Project not found" });
+    if (!project) throw new NotFoundError({ message: "Project not found" });
 
     // Anyone can create an access approval request.
     const { membership } = await permissionService.getProjectPermission(
@@ -110,7 +109,9 @@ export const accessApprovalRequestServiceFactory = ({
       actorAuthMethod,
       actorOrgId
     );
-    if (!membership) throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
 
     const requestedByUser = await userDAL.findById(actorId);
     if (!requestedByUser) throw new ForbiddenRequestError({ message: "User not found" });
@@ -272,7 +273,9 @@ export const accessApprovalRequestServiceFactory = ({
       actorAuthMethod,
       actorOrgId
     );
-    if (!membership) throw new NotFoundError({ message: "You don't have a membership for the specified project" });
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
 
     const policies = await accessApprovalPolicyDAL.find({ projectId: project.id });
     let requests = await accessApprovalRequestDAL.findRequestsWithPrivilegeByPolicyIds(policies.map((p) => p.id));
@@ -308,7 +311,9 @@ export const accessApprovalRequestServiceFactory = ({
       actorOrgId
     );
 
-    if (!membership) throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
 
     if (
       !hasRole(ProjectMembershipRole.Admin) &&
@@ -320,17 +325,20 @@ export const accessApprovalRequestServiceFactory = ({
 
     const reviewerProjectMembership = await projectMembershipDAL.findById(membership.id);
 
-    await verifyApprovers({
+    const approversValid = await isApproversValid({
       projectId: accessApprovalRequest.projectId,
       orgId: actorOrgId,
       envSlug: accessApprovalRequest.environment,
       secretPath: accessApprovalRequest.policy.secretPath!,
       actorAuthMethod,
       permissionService,
-      userIds: [reviewerProjectMembership.userId],
-      error: VerifyApproversError.ForbiddenError
+      userIds: [reviewerProjectMembership.userId]
     });
 
+    if (!approversValid) {
+      throw new ForbiddenRequestError({ message: "You don't have access to approve this request" });
+    }
+
     const existingReviews = await accessApprovalRequestReviewerDAL.find({ requestId: accessApprovalRequest.id });
     if (existingReviews.some((review) => review.status === ApprovalStatus.REJECTED)) {
       throw new BadRequestError({ message: "The request has already been rejected by another reviewer" });
@@ -422,7 +430,9 @@ export const accessApprovalRequestServiceFactory = ({
       actorAuthMethod,
       actorOrgId
     );
-    if (!membership) throw new NotFoundError({ message: "You don't have a membership for the specified project" });
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
 
     const count = await accessApprovalRequestDAL.getCount({ projectId: project.id });
 

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

@@ -116,7 +116,7 @@ export const permissionServiceFactory = ({
     if (userOrgId && userOrgId !== orgId)
       throw new ForbiddenRequestError({ message: "Invalid user token. Scoped to different organization." });
     const membership = await permissionDAL.getOrgPermission(userId, orgId);
-    if (!membership) throw new ForbiddenRequestError({ name: "User is not a part of the specified organization" });
+    if (!membership) throw new ForbiddenRequestError({ name: "You are not apart of this organization" });
     if (membership.role === OrgMembershipRole.Custom && !membership.permissions) {
       throw new BadRequestError({ name: "Custom organization permission not found" });
     }
@@ -143,7 +143,7 @@ export const permissionServiceFactory = ({
 
   const getIdentityOrgPermission = async (identityId: string, orgId: string) => {
     const membership = await permissionDAL.getOrgIdentityPermission(identityId, orgId);
-    if (!membership) throw new ForbiddenRequestError({ name: "Identity is not a part of the specified organization" });
+    if (!membership) throw new ForbiddenRequestError({ name: "Identity is not apart of this organization" });
     if (membership.role === OrgMembershipRole.Custom && !membership.permissions) {
       throw new NotFoundError({ name: "Custom organization permission not found" });
     }

backend/src/server/plugins/error-handler.ts

@@ -19,28 +19,47 @@ enum JWTErrors {
   InvalidAlgorithm = "invalid algorithm"
 }
 
+enum HttpStatusCodes {
+  BadRequest = 400,
+  NotFound = 404,
+  Unauthorized = 401,
+  Forbidden = 403,
+  // eslint-disable-next-line @typescript-eslint/no-shadow
+  InternalServerError = 500
+}
+
 export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider) => {
   server.setErrorHandler((error, req, res) => {
     req.log.error(error);
     if (error instanceof BadRequestError) {
-      void res.status(400).send({ statusCode: 400, message: error.message, error: error.name });
+      void res
+        .status(HttpStatusCodes.BadRequest)
+        .send({ statusCode: HttpStatusCodes.BadRequest, message: error.message, error: error.name });
     } else if (error instanceof NotFoundError) {
-      void res.status(404).send({ statusCode: 404, message: error.message, error: error.name });
+      void res
+        .status(HttpStatusCodes.NotFound)
+        .send({ statusCode: HttpStatusCodes.NotFound, message: error.message, error: error.name });
     } else if (error instanceof UnauthorizedError) {
-      void res.status(401).send({ statusCode: 401, message: error.message, error: error.name });
+      void res
+        .status(HttpStatusCodes.Unauthorized)
+        .send({ statusCode: HttpStatusCodes.Unauthorized, message: error.message, error: error.name });
     } else if (error instanceof DatabaseError || error instanceof InternalServerError) {
-      void res.status(500).send({ statusCode: 500, message: "Something went wrong", error: error.name });
+      void res
+        .status(HttpStatusCodes.InternalServerError)
+        .send({ statusCode: HttpStatusCodes.InternalServerError, message: "Something went wrong", error: error.name });
     } else if (error instanceof ZodError) {
-      void res.status(401).send({ statusCode: 401, error: "ValidationFailure", message: error.issues });
+      void res
+        .status(HttpStatusCodes.Unauthorized)
+        .send({ statusCode: HttpStatusCodes.Unauthorized, error: "ValidationFailure", message: error.issues });
     } else if (error instanceof ForbiddenError) {
-      void res.status(403).send({
-        statusCode: 403,
+      void res.status(HttpStatusCodes.Forbidden).send({
+        statusCode: HttpStatusCodes.Forbidden,
         error: "PermissionDenied",
         message: `You are not allowed to ${error.action} on ${error.subjectType}`
       });
     } else if (error instanceof ForbiddenRequestError) {
-      void res.status(403).send({
-        statusCode: 403,
+      void res.status(HttpStatusCodes.Forbidden).send({
+        statusCode: HttpStatusCodes.Forbidden,
         message: error.message,
         error: error.name
       });
@@ -66,8 +85,8 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider
         return error.message;
       })();
 
-      void res.status(403).send({
-        statusCode: 403,
+      void res.status(HttpStatusCodes.Forbidden).send({
+        statusCode: HttpStatusCodes.Forbidden,
         error: "TokenError",
         message
       });

backend/src/server/routes/sanitizedSchemas.ts

@@ -40,12 +40,12 @@ export const DefaultResponseErrorsSchema = {
   }),
   401: z.object({
     statusCode: z.literal(401),
-    message: z.string(),
+    message: z.any(),
     error: z.string()
   }),
   403: z.object({
     statusCode: z.literal(403),
-    message: z.any(),
+    message: z.string(),
     error: z.string()
   }),
   500: z.object({