From 5255c4075ae6f54e95cdb864bf7ab5a93c7304b3 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:39:15 -0700 Subject: [PATCH] Fix: Validate approvers access --- .../access-approval-policy-service.ts | 87 ++++++++++++++++--- 1 file changed, 75 insertions(+), 12 deletions(-) diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts index d1f0ce1e7..30f5f10fe 100644 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts @@ -3,22 +3,26 @@ 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 } 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"; import { TAccessApprovalPolicyApproverDALFactory } from "./access-approval-policy-approver-dal"; import { TAccessApprovalPolicyDALFactory } from "./access-approval-policy-dal"; +import { verifyApprovers } from "./access-approval-policy-fns"; import { TCreateAccessApprovalPolicy, TDeleteAccessApprovalPolicy, + TGetAccessPolicyCountByEnvironmentDTO, TListAccessApprovalPoliciesDTO, TUpdateAccessApprovalPolicy } from "./access-approval-policy-types"; type TSecretApprovalPolicyServiceFactoryDep = { + projectDAL: TProjectDALFactory; permissionService: Pick; accessApprovalPolicyDAL: TAccessApprovalPolicyDALFactory; - projectEnvDAL: Pick; + projectEnvDAL: Pick; accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory; projectMembershipDAL: Pick; }; @@ -30,6 +34,7 @@ export const accessApprovalPolicyServiceFactory = ({ accessApprovalPolicyApproverDAL, permissionService, projectEnvDAL, + projectDAL, projectMembershipDAL }: TSecretApprovalPolicyServiceFactoryDep) => { const createAccessApprovalPolicy = async ({ @@ -37,19 +42,24 @@ export const accessApprovalPolicyServiceFactory = ({ actor, actorId, actorOrgId, + secretPath, actorAuthMethod, approvals, approvers, - projectId, + projectSlug, environment }: TCreateAccessApprovalPolicy) => { + const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId); + if (!project) throw new BadRequestError({ message: "Project not found" }); + if (approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); + if (!secretPath) throw new BadRequestError({ message: "Secret path is required" }); const { permission } = await permissionService.getProjectPermission( actor, actorId, - projectId, + project.id, actorAuthMethod, actorOrgId ); @@ -57,13 +67,24 @@ export const accessApprovalPolicyServiceFactory = ({ ProjectPermissionActions.Create, ProjectPermissionSub.SecretApproval ); - const env = await projectEnvDAL.findOne({ slug: environment, projectId }); + const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id }); if (!env) throw new BadRequestError({ message: "Environment not found" }); const secretApprovers = await projectMembershipDAL.find({ - projectId, + projectId: project.id, $in: { id: approvers } }); + + await verifyApprovers({ + projectId: project.id, + orgId: actorOrgId, + envSlug: environment, + secretPath, + actorAuthMethod, + permissionService, + approverProjectMemberships: secretApprovers + }); + if (secretApprovers.length !== approvers.length) { throw new BadRequestError({ message: "Approver not found in project" }); } @@ -73,6 +94,7 @@ export const accessApprovalPolicyServiceFactory = ({ { envId: env.id, approvals, + secretPath, name }, tx @@ -86,7 +108,7 @@ export const accessApprovalPolicyServiceFactory = ({ ); return doc; }); - return { ...accessApproval, environment: env, projectId }; + return { ...accessApproval, environment: env, projectId: project.id }; }; const getAccessApprovalPolicyByProjectId = async ({ @@ -94,24 +116,29 @@ export const accessApprovalPolicyServiceFactory = ({ actor, actorOrgId, actorAuthMethod, - projectId + projectSlug }: TListAccessApprovalPoliciesDTO) => { - const { permission } = await permissionService.getProjectPermission( + const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId); + if (!project) throw new BadRequestError({ message: "Project not found" }); + + // Anyone in the project should be able to get the policies. + /* const { permission } = */ await permissionService.getProjectPermission( actor, actorId, - projectId, + project.id, actorAuthMethod, actorOrgId ); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval); + // ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval); - const accessApprovalPolicies = await accessApprovalPolicyDAL.find({ projectId }); + const accessApprovalPolicies = await accessApprovalPolicyDAL.find({ projectId: project.id }); return accessApprovalPolicies; }; const updateAccessApprovalPolicy = async ({ policyId, approvers, + secretPath, name, actorId, actor, @@ -121,7 +148,6 @@ export const accessApprovalPolicyServiceFactory = ({ }: TUpdateAccessApprovalPolicy) => { const accessApprovalPolicy = await accessApprovalPolicyDAL.findById(policyId); if (!accessApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" }); - const { permission } = await permissionService.getProjectPermission( actor, actorId, @@ -129,6 +155,7 @@ export const accessApprovalPolicyServiceFactory = ({ actorAuthMethod, actorOrgId ); + ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Edit, ProjectPermissionSub.SecretApproval); const updatedPolicy = await accessApprovalPolicyDAL.transaction(async (tx) => { @@ -136,6 +163,7 @@ export const accessApprovalPolicyServiceFactory = ({ accessApprovalPolicy.id, { approvals, + secretPath, name }, tx @@ -149,6 +177,17 @@ export const accessApprovalPolicyServiceFactory = ({ }, { tx } ); + + await verifyApprovers({ + projectId: accessApprovalPolicy.projectId, + orgId: actorOrgId, + envSlug: accessApprovalPolicy.environment.slug, + secretPath: doc.secretPath!, + actorAuthMethod, + permissionService, + approverProjectMemberships: secretApprovers + }); + if (secretApprovers.length !== approvers.length) throw new BadRequestError({ message: "Approver not found in project" }); if (doc.approvals > secretApprovers.length) @@ -197,7 +236,31 @@ export const accessApprovalPolicyServiceFactory = ({ return policy; }; + const getAccessPolicyCountByEnvSlug = async ({ + actor, + actorOrgId, + actorAuthMethod, + projectSlug, + actorId, + envSlug + }: TGetAccessPolicyCountByEnvironmentDTO) => { + const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId); + + if (!project) throw new BadRequestError({ message: "Project not found" }); + + await permissionService.getProjectPermission(actor, actorId, project.id, actorAuthMethod, actorOrgId); + + const environment = await projectEnvDAL.findOne({ projectId: project.id, slug: envSlug }); + if (!environment) throw new BadRequestError({ message: "Environment not found" }); + + const policies = await accessApprovalPolicyDAL.find({ envId: environment.id, projectId: project.id }); + if (!policies) throw new BadRequestError({ message: "No policies found" }); + + return { policyCount: policies.length }; + }; + return { + getAccessPolicyCountByEnvSlug, createAccessApprovalPolicy, deleteAccessApprovalPolicy, updateAccessApprovalPolicy,