Merge pull request #2517 from Infisical/revert-2505-revert-2494-danie…

…l/api-errors

feat(api): better errors and documentation

backend/e2e-test/routes/v2/service-token.spec.ts

@@ -510,7 +510,7 @@ describe("Service token fail cases", async () => {
         authorization: `Bearer ${serviceToken}`
       }
     });
-    expect(fetchSecrets.statusCode).toBe(401);
+    expect(fetchSecrets.statusCode).toBe(403);
     expect(fetchSecrets.json().error).toBe("PermissionDenied");
     await deleteServiceToken();
   });
@@ -532,7 +532,7 @@ describe("Service token fail cases", async () => {
         authorization: `Bearer ${serviceToken}`
       }
     });
-    expect(fetchSecrets.statusCode).toBe(401);
+    expect(fetchSecrets.statusCode).toBe(403);
     expect(fetchSecrets.json().error).toBe("PermissionDenied");
     await deleteServiceToken();
   });
@@ -557,7 +557,7 @@ describe("Service token fail cases", async () => {
         authorization: `Bearer ${serviceToken}`
       }
     });
-    expect(writeSecrets.statusCode).toBe(401);
+    expect(writeSecrets.statusCode).toBe(403);
     expect(writeSecrets.json().error).toBe("PermissionDenied");
 
     // but read access should still work fine

backend/e2e-test/routes/v3/secrets.spec.ts

@@ -1075,7 +1075,7 @@ describe("Secret V3 Raw Router Without E2EE enabled", async () => {
       },
       body: createSecretReqBody
     });
-    expect(createSecRes.statusCode).toBe(400);
+    expect(createSecRes.statusCode).toBe(404);
   });
 
   test("Update secret raw", async () => {
@@ -1093,7 +1093,7 @@ describe("Secret V3 Raw Router Without E2EE enabled", async () => {
       },
       body: updateSecretReqBody
     });
-    expect(updateSecRes.statusCode).toBe(400);
+    expect(updateSecRes.statusCode).toBe(404);
   });
 
   test("Delete secret raw", async () => {
@@ -1110,6 +1110,6 @@ describe("Secret V3 Raw Router Without E2EE enabled", async () => {
       },
       body: deletedSecretReqBody
     });
-    expect(deletedSecRes.statusCode).toBe(400);
+    expect(deletedSecRes.statusCode).toBe(404);
   });
 });

backend/src/ee/routes/v1/identity-project-additional-privilege-router.ts

@@ -5,7 +5,7 @@ import { z } from "zod";
 
 import { IdentityProjectAdditionalPrivilegeTemporaryMode } from "@app/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-types";
 import { IDENTITY_ADDITIONAL_PRIVILEGE } from "@app/lib/api-docs";
-import { BadRequestError } from "@app/lib/errors";
+import { UnauthorizedError } from "@app/lib/errors";
 import { alphaNumericNanoId } from "@app/lib/nanoid";
 import { readLimit, writeLimit } from "@app/server/config/rateLimiter";
 import { verifyAuth } from "@app/server/plugins/auth/verify-auth";
@@ -61,7 +61,7 @@ export const registerIdentityProjectAdditionalPrivilegeRouter = async (server: F
     handler: async (req) => {
       const { permissions, privilegePermission } = req.body;
       if (!permissions && !privilegePermission) {
-        throw new BadRequestError({ message: "Permission or privilegePermission must be provided" });
+        throw new UnauthorizedError({ message: "Permission or privilegePermission must be provided" });
       }
 
       const permission = privilegePermission
@@ -140,7 +140,7 @@ export const registerIdentityProjectAdditionalPrivilegeRouter = async (server: F
     handler: async (req) => {
       const { permissions, privilegePermission } = req.body;
       if (!permissions && !privilegePermission) {
-        throw new BadRequestError({ message: "Permission or privilegePermission must be provided" });
+        throw new UnauthorizedError({ message: "Permission or privilegePermission must be provided" });
       }
 
       const permission = privilegePermission
@@ -224,7 +224,7 @@ export const registerIdentityProjectAdditionalPrivilegeRouter = async (server: F
     handler: async (req) => {
       const { permissions, privilegePermission, ...updatedInfo } = req.body.privilegeDetails;
       if (!permissions && !privilegePermission) {
-        throw new BadRequestError({ message: "Permission or privilegePermission must be provided" });
+        throw new UnauthorizedError({ message: "Permission or privilegePermission must be provided" });
       }
 
       const permission = privilegePermission

backend/src/ee/routes/v1/rate-limit-router.ts

@@ -1,7 +1,7 @@
 import { z } from "zod";
 
 import { RateLimitSchema } from "@app/db/schemas";
-import { BadRequestError } from "@app/lib/errors";
+import { NotFoundError } from "@app/lib/errors";
 import { readLimit } from "@app/server/config/rateLimiter";
 import { verifySuperAdmin } from "@app/server/plugins/auth/superAdmin";
 import { verifyAuth } from "@app/server/plugins/auth/verify-auth";
@@ -29,7 +29,7 @@ export const registerRateLimitRouter = async (server: FastifyZodProvider) => {
     handler: async () => {
       const rateLimit = await server.services.rateLimit.getRateLimits();
       if (!rateLimit) {
-        throw new BadRequestError({
+        throw new NotFoundError({
           name: "Get Rate Limit Error",
           message: "Rate limit configuration does not exist."
         });

backend/src/ee/routes/v1/saml-router.ts

@@ -61,7 +61,7 @@ export const registerSamlRouter = async (server: FastifyZodProvider) => {
                 id: samlConfigId
               };
             } else {
-              throw new BadRequestError({ message: "Missing sso identitier or org slug" });
+              throw new BadRequestError({ message: "Missing sso identifier or org slug" });
             }
 
             const ssoConfig = await server.services.saml.getSaml(ssoLookupDetails);

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

@@ -1,22 +1,21 @@
 import { ForbiddenError, subject } from "@casl/ability";
 
-import { BadRequestError } from "@app/lib/errors";
 import { ActorType } from "@app/services/auth/auth-type";
 
 import { ProjectPermissionActions, ProjectPermissionSub } from "../permission/project-permission";
-import { TVerifyApprovers } 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
-}: TVerifyApprovers) => {
-  for await (const userId of userIds) {
-    try {
+}: TIsApproversValid) => {
+  try {
+    for await (const userId of userIds) {
       const { permission: approverPermission } = await permissionService.getProjectPermission(
         ActorType.USER,
         userId,
@@ -29,8 +28,9 @@ export const verifyApprovers = async ({
         ProjectPermissionActions.Create,
         subject(ProjectPermissionSub.Secrets, { environment: envSlug, secretPath })
       );
-    } catch (err) {
-      throw new BadRequestError({ message: "One or more approvers doesn't have access to be specified secret path" });
     }
+  } 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,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,
@@ -58,7 +58,7 @@ export const accessApprovalPolicyServiceFactory = ({
     enforcementLevel
   }: TCreateAccessApprovalPolicy) => {
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
-    if (!project) throw new BadRequestError({ message: "Project not found" });
+    if (!project) throw new NotFoundError({ message: "Project not found" });
 
     // If there is a group approver people might be added to the group later to meet the approvers quota
     const groupApprovers = approvers
@@ -89,7 +89,7 @@ export const accessApprovalPolicyServiceFactory = ({
       ProjectPermissionSub.SecretApproval
     );
     const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id });
-    if (!env) throw new BadRequestError({ message: "Environment not found" });
+    if (!env) throw new NotFoundError({ message: "Environment not found" });
 
     let approverUserIds = userApprovers;
     if (userApproverNames.length) {
@@ -132,7 +132,7 @@ export const accessApprovalPolicyServiceFactory = ({
       .map((user) => user.id);
     verifyAllApprovers.push(...verifyGroupApprovers);
 
-    await verifyApprovers({
+    const approversValid = await isApproversValid({
       projectId: project.id,
       orgId: actorOrgId,
       envSlug: environment,
@@ -142,6 +142,12 @@ export const accessApprovalPolicyServiceFactory = ({
       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(
         {
@@ -186,7 +192,7 @@ export const accessApprovalPolicyServiceFactory = ({
     projectSlug
   }: TListAccessApprovalPoliciesDTO) => {
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
-    if (!project) throw new BadRequestError({ message: "Project not found" });
+    if (!project) throw new NotFoundError({ message: "Project not found" });
 
     // Anyone in the project should be able to get the policies.
     /* const { permission } = */ await permissionService.getProjectPermission(
@@ -237,7 +243,7 @@ export const accessApprovalPolicyServiceFactory = ({
       throw new BadRequestError({ message: "Approvals cannot be greater than approvers" });
     }
 
-    if (!accessApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" });
+    if (!accessApprovalPolicy) throw new NotFoundError({ message: "Secret approval policy not found" });
     const { permission } = await permissionService.getProjectPermission(
       actor,
       actorId,
@@ -283,7 +289,7 @@ 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,
@@ -293,6 +299,12 @@ export const accessApprovalPolicyServiceFactory = ({
           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,
@@ -322,7 +334,7 @@ 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,
@@ -331,6 +343,13 @@ export const accessApprovalPolicyServiceFactory = ({
           permissionService,
           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,
@@ -357,7 +376,7 @@ export const accessApprovalPolicyServiceFactory = ({
     actorOrgId
   }: TDeleteAccessApprovalPolicy) => {
     const policy = await accessApprovalPolicyDAL.findById(policyId);
-    if (!policy) throw new BadRequestError({ message: "Secret approval policy not found" });
+    if (!policy) throw new NotFoundError({ message: "Secret approval policy not found" });
 
     const { permission } = await permissionService.getProjectPermission(
       actor,
@@ -385,7 +404,7 @@ export const accessApprovalPolicyServiceFactory = ({
   }: TGetAccessPolicyCountByEnvironmentDTO) => {
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
 
-    if (!project) throw new BadRequestError({ message: "Project not found" });
+    if (!project) throw new NotFoundError({ message: "Project not found" });
 
     const { membership } = await permissionService.getProjectPermission(
       actor,
@@ -394,13 +413,15 @@ export const accessApprovalPolicyServiceFactory = ({
       actorAuthMethod,
       actorOrgId
     );
-    if (!membership) throw new BadRequestError({ 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 BadRequestError({ message: "Environment not found" });
+    if (!environment) throw new NotFoundError({ message: "Environment not found" });
 
     const policies = await accessApprovalPolicyDAL.find({ envId: environment.id, projectId: project.id });
-    if (!policies) throw new BadRequestError({ message: "No policies found" });
+    if (!policies) throw new NotFoundError({ message: "No policies found" });
 
     return { count: policies.length };
   };

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

@@ -3,7 +3,7 @@ import { ActorAuthMethod } from "@app/services/auth/auth-type";
 
 import { TPermissionServiceFactory } from "../permission/permission-service";
 
-export type TVerifyApprovers = {
+export type TIsApproversValid = {
   userIds: string[];
   permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
   envSlug: string;

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

@@ -1,6 +1,6 @@
 import { PackRule, unpackRules } from "@casl/ability/extra";
 
-import { UnauthorizedError } from "@app/lib/errors";
+import { BadRequestError } from "@app/lib/errors";
 
 import { TVerifyPermission } from "./access-approval-request-types";
 
@@ -19,7 +19,7 @@ export const verifyRequestedPermissions = ({ permissions }: TVerifyPermission) =
   );
 
   if (!permission || !permission.length) {
-    throw new UnauthorizedError({ message: "No permission provided" });
+    throw new BadRequestError({ message: "No permission provided" });
   }
 
   const requestedPermissions: string[] = [];
@@ -39,10 +39,10 @@ export const verifyRequestedPermissions = ({ permissions }: TVerifyPermission) =
   const permissionEnv = firstPermission.conditions?.environment;
 
   if (!permissionEnv || typeof permissionEnv !== "string") {
-    throw new UnauthorizedError({ message: "Permission environment is not a string" });
+    throw new BadRequestError({ message: "Permission environment is not a string" });
   }
   if (!permissionSecretPath || typeof permissionSecretPath !== "string") {
-    throw new UnauthorizedError({ message: "Permission path is not a string" });
+    throw new BadRequestError({ message: "Permission path is not a string" });
   }
 
   return {