diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index ef3e83b6e..fd9daddcc 100644 --- a/backend/src/server/routes/index.ts +++ b/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 diff --git a/backend/src/server/routes/v1/approval-policy-routers/approval-policy-endpoints.ts b/backend/src/server/routes/v1/approval-policy-routers/approval-policy-endpoints.ts index 44e56138d..b72f5eaa5 100644 --- a/backend/src/server/routes/v1/approval-policy-routers/approval-policy-endpoints.ts +++ b/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 =

({ }, 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 =

({ }, 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 =

({ }), 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 }; } }); }; diff --git a/backend/src/services/approval-policy/approval-policy-dal.ts b/backend/src/services/approval-policy/approval-policy-dal.ts index 795e72d02..6276b14fc 100644 --- a/backend/src/services/approval-policy/approval-policy-dal.ts +++ b/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; 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>((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 }; }; diff --git a/backend/src/services/approval-policy/approval-policy-schemas.ts b/backend/src/services/approval-policy/approval-policy-schemas.ts index b724b6e54..0ac225ba2 100644 --- a/backend/src/services/approval-policy/approval-policy-schemas.ts +++ b/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() }); diff --git a/backend/src/services/approval-policy/approval-policy-service.ts b/backend/src/services/approval-policy/approval-policy-service.ts index e2fa5d117..94dbc3d93 100644 --- a/backend/src/services/approval-policy/approval-policy-service.ts +++ b/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; + projectMembershipDAL: Pick; }; export type TApprovalPolicyServiceFactory = ReturnType; @@ -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 = {}; @@ -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 }; diff --git a/backend/src/services/approval-policy/approval-policy-types.ts b/backend/src/services/approval-policy/approval-policy-types.ts index ecfa87470..c2cc858b1 100644 --- a/backend/src/services/approval-policy/approval-policy-types.ts +++ b/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"]; diff --git a/backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts b/backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts index 4bc20fa0c..2373acf0e 100644 --- a/backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts +++ b/backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts @@ -26,7 +26,7 @@ export const pamAccessPolicyFactory: TApprovalResourceFactory r === inputs.resourceId)) { + if (!c.resourceIds.some((r) => r === inputs.resourceId)) { // eslint-disable-next-line no-continue continue; } diff --git a/backend/src/services/approval-policy/pam-access/pam-access-policy-schemas.ts b/backend/src/services/approval-policy/pam-access/pam-access-policy-schemas.ts index b2b4cc440..caa9562b2 100644 --- a/backend/src/services/approval-policy/pam-access/pam-access-policy-schemas.ts +++ b/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();