policy api improvements

backend/src/server/routes/index.ts

@@ -2470,8 +2470,8 @@ export const registerRoutes = async (
     approvalPolicyDAL,
     approvalPolicyStepsDAL,
     approvalPolicyStepApproversDAL,
-    projectDAL,
-    permissionService
+    permissionService,
+    projectMembershipDAL
   });
 
   // setup the communication with license key server

backend/src/server/routes/v1/approval-policy-routers/approval-policy-endpoints.ts

@@ -1,6 +1,6 @@
 import { z } from "zod";
 
-import { writeLimit } from "@app/server/config/rateLimiter";
+import { readLimit, writeLimit } from "@app/server/config/rateLimiter";
 import { verifyAuth } from "@app/server/plugins/auth/verify-auth";
 import { ApprovalPolicyType } from "@app/services/approval-policy/approval-policy-enums";
 import {
@@ -50,7 +50,34 @@ export const registerApprovalPolicyEndpoints = <P extends TApprovalPolicy>({
     },
     onRequest: verifyAuth([AuthMode.JWT]),
     handler: async (req) => {
-      const policy = await server.services.approvalPolicy.create(policyType, req.body, req.permission);
+      const { policy } = await server.services.approvalPolicy.create(policyType, req.body, req.permission);
+
+      // TODO: Audit log
+
+      return { policy };
+    }
+  });
+
+  server.route({
+    method: "GET",
+    url: "/:policyId",
+    config: {
+      rateLimit: readLimit
+    },
+    schema: {
+      description: "Get approval policy",
+      params: z.object({
+        policyId: z.string().uuid()
+      }),
+      response: {
+        200: z.object({
+          policy: policyResponseSchema
+        })
+      }
+    },
+    onRequest: verifyAuth([AuthMode.JWT]),
+    handler: async (req) => {
+      const { policy } = await server.services.approvalPolicy.getById(req.params.policyId, req.permission);
 
       // TODO: Audit log
 
@@ -78,7 +105,7 @@ export const registerApprovalPolicyEndpoints = <P extends TApprovalPolicy>({
     },
     onRequest: verifyAuth([AuthMode.JWT]),
     handler: async (req) => {
-      const policy = await server.services.approvalPolicy.updateById(req.params.policyId, req.body, req.permission);
+      const { policy } = await server.services.approvalPolicy.updateById(req.params.policyId, req.body, req.permission);
 
       // TODO: Audit log
 
@@ -99,17 +126,17 @@ export const registerApprovalPolicyEndpoints = <P extends TApprovalPolicy>({
       }),
       response: {
         200: z.object({
-          policy: policyResponseSchema
+          policyId: z.string().uuid()
         })
       }
     },
     onRequest: verifyAuth([AuthMode.JWT]),
     handler: async (req) => {
-      const policy = await server.services.approvalPolicy.deleteById(req.params.policyId, req.permission);
+      const { policyId } = await server.services.approvalPolicy.deleteById(req.params.policyId, req.permission);
 
       // TODO: Audit log
 
-      return { policy };
+      return { policyId };
     }
   });
 };

backend/src/services/approval-policy/approval-policy-dal.ts

@@ -1,14 +1,70 @@
 import { TDbClient } from "@app/db";
 import { TableName } from "@app/db/schemas";
+import { DatabaseError } from "@app/lib/errors";
 import { ormify } from "@app/lib/knex";
 
+import { ApproverType } from "./approval-policy-enums";
+
 // Approval Policy
 export type TApprovalPolicyDALFactory = ReturnType<typeof approvalPolicyDALFactory>;
 export const approvalPolicyDALFactory = (db: TDbClient) => {
   const orm = ormify(db, TableName.ApprovalPolicies);
 
+  const findStepsByPolicyId = async (policyId: string) => {
+    try {
+      const dbInstance = db.replicaNode();
+      const steps = await dbInstance(TableName.ApprovalPolicySteps).where({ policyId }).orderBy("stepNumber", "asc");
+
+      if (!steps.length) {
+        return [];
+      }
+
+      const stepIds = steps.map((step) => step.id);
+
+      const approvers = await dbInstance(TableName.ApprovalPolicyStepApprovers)
+        .whereIn("policyStepId", stepIds)
+        .select("policyStepId", "userId", "groupId");
+
+      const approversByStepId = approvers.reduce<Record<string, { type: string; id: string }[]>>((acc, approver) => {
+        const stepApprovers = acc[approver.policyStepId] || [];
+        stepApprovers.push({
+          type: approver.userId ? ApproverType.User : ApproverType.Group,
+          id: (approver.userId || approver.groupId) as string
+        });
+        acc[approver.policyStepId] = stepApprovers;
+        return acc;
+      }, {});
+
+      return steps.map((step) => {
+        const stepApprovers = approversByStepId[step.id] || [];
+
+        const formattedStep: {
+          name?: string;
+          requiredApprovals: number;
+          notifyApprovers?: boolean;
+          approvers: { type: string; id: string }[];
+        } = {
+          requiredApprovals: step.requiredApprovals,
+          approvers: stepApprovers
+        };
+
+        if (step.name) {
+          formattedStep.name = step.name;
+        }
+        if (typeof step.notifyApprovers === "boolean") {
+          formattedStep.notifyApprovers = step.notifyApprovers;
+        }
+
+        return formattedStep;
+      });
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Find approval policy steps" });
+    }
+  };
+
   return {
-    ...orm
+    ...orm,
+    findStepsByPolicyId
   };
 };
 

backend/src/services/approval-policy/approval-policy-schemas.ts

@@ -4,43 +4,31 @@ import { ApprovalPoliciesSchema } from "@app/db/schemas";
 
 import { ApproverType } from "./approval-policy-enums";
 
-export const BaseApprovalPolicySchema = ApprovalPoliciesSchema;
+const ApprovalPolicyStepSchema = z.object({
+  name: z.string().min(1).max(128).nullable().optional(),
+  requiredApprovals: z.number().min(1).max(100),
+  notifyApprovers: z.boolean().optional(),
+  approvers: z
+    .object({
+      type: z.nativeEnum(ApproverType),
+      id: z.string().uuid()
+    })
+    .array()
+});
+
+export const BaseApprovalPolicySchema = ApprovalPoliciesSchema.extend({
+  steps: ApprovalPolicyStepSchema.array()
+});
 
 export const BaseCreateApprovalPolicySchema = z.object({
   projectId: z.string().uuid(),
-  organizationId: z.string().uuid(),
   name: z.string().min(1).max(128),
   maxRequestTtlSeconds: z.number().min(3600).max(2592000).nullable().optional(), // 1 hour to 30 days
-  steps: z
-    .object({
-      name: z.string().min(1).max(128).nullable().optional(),
-      requiredApprovals: z.number().min(1).max(100),
-      notifyApprovers: z.boolean().optional(),
-      approvers: z
-        .object({
-          type: z.nativeEnum(ApproverType),
-          id: z.string().uuid()
-        })
-        .array()
-    })
-    .array()
+  steps: ApprovalPolicyStepSchema.array()
 });
 
 export const BaseUpdateApprovalPolicySchema = z.object({
   name: z.string().min(1).max(128).optional(),
   maxRequestTtlSeconds: z.number().min(3600).max(2592000).nullable().optional(), // 1 hour to 30 days
-  steps: z
-    .object({
-      name: z.string().min(1).max(128).nullable().optional(),
-      requiredApprovals: z.number().min(1).max(100),
-      notifyApprovers: z.boolean().optional(),
-      approvers: z
-        .object({
-          type: z.nativeEnum(ApproverType),
-          id: z.string().uuid()
-        })
-        .array()
-    })
-    .array()
-    .optional()
+  steps: ApprovalPolicyStepSchema.array().optional()
 });

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

@@ -1,9 +1,9 @@
 import { ActionProjectType, ProjectMembershipRole, TApprovalPolicies } from "@app/db/schemas";
 import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service-types";
-import { ForbiddenRequestError } from "@app/lib/errors";
+import { BadRequestError, ForbiddenRequestError } from "@app/lib/errors";
 import { OrgServiceActor } from "@app/lib/types";
-import { TProjectDALFactory } from "@app/services/project/project-dal";
 
+import { TProjectMembershipDALFactory } from "../project-membership/project-membership-dal";
 import {
   TApprovalPolicyDALFactory,
   TApprovalPolicyStepApproversDALFactory,
@@ -16,8 +16,8 @@ type TApprovalPolicyServiceFactoryDep = {
   approvalPolicyDAL: TApprovalPolicyDALFactory;
   approvalPolicyStepsDAL: TApprovalPolicyStepsDALFactory;
   approvalPolicyStepApproversDAL: TApprovalPolicyStepApproversDALFactory;
-  projectDAL: TProjectDALFactory;
   permissionService: Pick<TPermissionServiceFactory, "getProjectPermission" | "getOrgPermission">;
+  projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findProjectMembershipsByUserIds">;
 };
 export type TApprovalPolicyServiceFactory = ReturnType<typeof approvalPolicyServiceFactory>;
 
@@ -25,11 +25,28 @@ export const approvalPolicyServiceFactory = ({
   approvalPolicyDAL,
   approvalPolicyStepsDAL,
   approvalPolicyStepApproversDAL,
-  permissionService
+  permissionService,
+  projectMembershipDAL
 }: TApprovalPolicyServiceFactoryDep) => {
+  const $verifyProjectUserMembership = async (userIds: string[], orgId: string, projectId: string) => {
+    const uniqueUserIds = [...new Set(userIds)];
+    if (uniqueUserIds.length === 0) return;
+
+    const allMemberships = await projectMembershipDAL.findProjectMembershipsByUserIds(orgId, uniqueUserIds);
+    const projectMemberships = allMemberships.filter((membership) => membership.projectId === projectId);
+
+    if (projectMemberships.length !== uniqueUserIds.length) {
+      const projectMemberUserIds = new Set(projectMemberships.map((membership) => membership.userId));
+      const userIdsNotInProject = uniqueUserIds.filter((id) => !projectMemberUserIds.has(id));
+      throw new BadRequestError({
+        message: `Some users are not members of the project: ${userIdsNotInProject.join(", ")}`
+      });
+    }
+  };
+
   const create = async (
     policyType: ApprovalPolicyType,
-    { projectId, organizationId, name, maxRequestTtlSeconds, conditions, constraints, steps }: TCreatePolicyDTO,
+    { projectId, name, maxRequestTtlSeconds, conditions, constraints, steps }: TCreatePolicyDTO,
     actor: OrgServiceActor
   ) => {
     const { hasRole } = await permissionService.getProjectPermission({
@@ -45,11 +62,18 @@ export const approvalPolicyServiceFactory = ({
       throw new ForbiddenRequestError({ message: "User has insufficient privileges" });
     }
 
+    // Verify all users are part of project
+    const approverUserIds = steps
+      .flatMap((step) => step.approvers ?? [])
+      .filter((approver) => approver.type === ApproverType.User)
+      .map((approver) => approver.id);
+    await $verifyProjectUserMembership(approverUserIds, actor.orgId, projectId);
+
     const policy = await approvalPolicyDAL.transaction(async (tx) => {
       const newPolicy = await approvalPolicyDAL.create(
         {
           projectId,
-          organizationId,
+          organizationId: actor.orgId,
           name,
           maxRequestTtlSeconds,
           conditions: { version: 1, conditions },
@@ -94,10 +118,34 @@ export const approvalPolicyServiceFactory = ({
     });
 
     return {
-      policy
+      policy: { ...policy, steps }
     };
   };
 
+  const getById = async (policyId: string, actor: OrgServiceActor) => {
+    const policy = await approvalPolicyDAL.findById(policyId);
+    if (!policy) {
+      throw new ForbiddenRequestError({ message: "Policy not found" });
+    }
+
+    const { hasRole } = await permissionService.getProjectPermission({
+      actor: actor.type,
+      actorAuthMethod: actor.authMethod,
+      actorId: actor.id,
+      actorOrgId: actor.orgId,
+      projectId: policy.projectId,
+      actionProjectType: ActionProjectType.Any
+    });
+
+    if (!hasRole(ProjectMembershipRole.Admin)) {
+      throw new ForbiddenRequestError({ message: "User has insufficient privileges" });
+    }
+
+    const steps = await approvalPolicyDAL.findStepsByPolicyId(policyId);
+
+    return { policy: { ...policy, steps } };
+  };
+
   const updateById = async (
     policyId: string,
     { name, maxRequestTtlSeconds, conditions, constraints, steps }: TUpdatePolicyDTO,
@@ -121,6 +169,15 @@ export const approvalPolicyServiceFactory = ({
       throw new ForbiddenRequestError({ message: "User has insufficient privileges" });
     }
 
+    if (steps !== undefined) {
+      // Verify all users are part of project
+      const approverUserIds = steps
+        .flatMap((step) => step.approvers ?? [])
+        .filter((approver) => approver.type === ApproverType.User)
+        .map((approver) => approver.id);
+      await $verifyProjectUserMembership(approverUserIds, actor.orgId, policy.projectId);
+    }
+
     const updatedPolicy = await approvalPolicyDAL.transaction(async (tx) => {
       const updateDoc: Partial<TApprovalPolicies> = {};
 
@@ -178,8 +235,10 @@ export const approvalPolicyServiceFactory = ({
       return updated;
     });
 
+    const fetchedSteps = await approvalPolicyDAL.findStepsByPolicyId(policyId);
+
     return {
-      policy: updatedPolicy
+      policy: { ...updatedPolicy, steps: fetchedSteps }
     };
   };
 
@@ -202,15 +261,16 @@ export const approvalPolicyServiceFactory = ({
       throw new ForbiddenRequestError({ message: "User has insufficient privileges" });
     }
 
-    const deletedPolicy = await approvalPolicyDAL.deleteById(policyId);
+    await approvalPolicyDAL.deleteById(policyId);
 
     return {
-      policy: deletedPolicy
+      policyId
     };
   };
 
   return {
     create,
+    getById,
     updateById,
     deleteById
   };

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

@@ -15,7 +15,6 @@ export type TApprovalPolicyConstraints = TPamAccessPolicyConstraints;
 // DTOs
 export interface TCreatePolicyDTO {
   projectId: TApprovalPolicy["projectId"];
-  organizationId: TApprovalPolicy["organizationId"];
   name: TApprovalPolicy["name"];
   maxRequestTtlSeconds?: TApprovalPolicy["maxRequestTtlSeconds"];
   conditions: TApprovalPolicy["conditions"]["conditions"];

backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts

@@ -26,7 +26,7 @@ export const pamAccessPolicyFactory: TApprovalResourceFactory<TPamAccessPolicyIn
     for (const policy of policies) {
       const p = policy as TPamAccessPolicy;
       for (const c of p.conditions.conditions) {
-        if (!c.targetResources.some((r) => r === inputs.resourceId)) {
+        if (!c.resourceIds.some((r) => r === inputs.resourceId)) {
           // eslint-disable-next-line no-continue
           continue;
         }

backend/src/services/approval-policy/pam-access/pam-access-policy-schemas.ts

@@ -15,7 +15,7 @@ export const PamAccessPolicyInputsSchema = z.object({
 // Conditions
 export const PamAccessPolicyConditionsSchema = z
   .object({
-    targetResources: z.string().uuid().array(),
+    resourceIds: z.string().uuid().array(),
     accountPaths: z.string().array() // TODO: Add path & wildcard validation
   })
   .array();