From 009be0ded83fab86d33d218106b02aa810f395ab Mon Sep 17 00:00:00 2001 From: Meet Date: Fri, 20 Sep 2024 01:24:30 +0530 Subject: [PATCH 1/8] feat: allow access approvals with user groups --- backend/src/@types/knex.d.ts | 6 + .../20240918005344_add-group-approvals.ts | 22 ++++ ...ccess-approval-policies-group-approvers.ts | 25 +++++ backend/src/db/schemas/index.ts | 1 + backend/src/db/schemas/models.ts | 1 + .../v1/access-approval-policy-router.ts | 25 +++-- .../access-approval-policy-dal.ts | 20 ++++ ...cess-approval-policy-group-approver-dal.ts | 12 ++ .../access-approval-policy-service.ts | 103 +++++++++++++++++- .../access-approval-policy-types.ts | 2 + .../access-approval-request-dal.ts | 63 ++++++++++- .../access-approval-request-service.ts | 30 ++++- backend/src/server/routes/index.ts | 8 +- .../src/hooks/api/accessApproval/mutation.tsx | 5 +- .../src/hooks/api/accessApproval/types.ts | 3 + .../ApprovalPolicyList/ApprovalPolicyList.tsx | 7 +- .../components/AccessPolicyModal.tsx | 69 ++++++++++-- .../components/ApprovalPolicyRow.tsx | 88 ++++++++++++++- 18 files changed, 457 insertions(+), 33 deletions(-) create mode 100644 backend/src/db/migrations/20240918005344_add-group-approvals.ts create mode 100644 backend/src/db/schemas/access-approval-policies-group-approvers.ts create mode 100644 backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts diff --git a/backend/src/@types/knex.d.ts b/backend/src/@types/knex.d.ts index 7cf86032f..d7b484dfc 100644 --- a/backend/src/@types/knex.d.ts +++ b/backend/src/@types/knex.d.ts @@ -6,6 +6,7 @@ import { TAccessApprovalPoliciesApprovers, TAccessApprovalPoliciesApproversInsert, TAccessApprovalPoliciesApproversUpdate, + TAccessApprovalPoliciesGroupApprovers, TAccessApprovalPoliciesInsert, TAccessApprovalPoliciesUpdate, TAccessApprovalRequests, @@ -800,5 +801,10 @@ declare module "knex/types/tables" { TWorkflowIntegrationsInsert, TWorkflowIntegrationsUpdate >; + [TableName.AccessApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< + TAccessApprovalPoliciesGroupApprovers, + TAccessApprovalPoliciesGroupApproversInsert, + TAccessApprovalPoliciesGroupApproversInsert + >; } } diff --git a/backend/src/db/migrations/20240918005344_add-group-approvals.ts b/backend/src/db/migrations/20240918005344_add-group-approvals.ts new file mode 100644 index 000000000..aeddbd9cb --- /dev/null +++ b/backend/src/db/migrations/20240918005344_add-group-approvals.ts @@ -0,0 +1,22 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; +import { createOnUpdateTrigger } from "../utils"; + +export async function up(knex: Knex): Promise { + if (!(await knex.schema.hasTable(TableName.AccessApprovalPolicyGroupApprover))) { + await knex.schema.createTable(TableName.AccessApprovalPolicyGroupApprover, (t) => { + t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); + t.uuid("approverGroupId").notNullable(); + t.foreign("approverGroupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); + t.uuid("policyId").notNullable(); + t.foreign("policyId").references("id").inTable(TableName.AccessApprovalPolicy).onDelete("CASCADE"); + t.timestamps(true, true, true); + }); + await createOnUpdateTrigger(knex, TableName.AccessApprovalPolicyGroupApprover); + } +} + +export async function down(knex: Knex): Promise { + await knex.schema.dropTableIfExists(TableName.AccessApprovalPolicyGroupApprover); +} diff --git a/backend/src/db/schemas/access-approval-policies-group-approvers.ts b/backend/src/db/schemas/access-approval-policies-group-approvers.ts new file mode 100644 index 000000000..7092629f6 --- /dev/null +++ b/backend/src/db/schemas/access-approval-policies-group-approvers.ts @@ -0,0 +1,25 @@ +// Code generated by automation script, DO NOT EDIT. +// Automated by pulling database and generating zod schema +// To update. Just run npm run generate:schema +// Written by akhilmhdh. + +import { z } from "zod"; + +import { TImmutableDBKeys } from "./models"; + +export const AccessApprovalPoliciesGroupApproversSchema = z.object({ + id: z.string().uuid(), + approverGroupId: z.string().uuid(), + policyId: z.string().uuid(), + createdAt: z.date(), + updatedAt: z.date() +}); + +export type TAccessApprovalPoliciesGroupApprovers = z.infer; +export type TAccessApprovalPoliciesGroupApproversInsert = Omit< + z.input, + TImmutableDBKeys +>; +export type TAccessApprovalPoliciesGroupApproversUpdate = Partial< + Omit, TImmutableDBKeys> +>; diff --git a/backend/src/db/schemas/index.ts b/backend/src/db/schemas/index.ts index d856cab49..b145684b4 100644 --- a/backend/src/db/schemas/index.ts +++ b/backend/src/db/schemas/index.ts @@ -1,5 +1,6 @@ export * from "./access-approval-policies"; export * from "./access-approval-policies-approvers"; +export * from "./access-approval-policies-group-approvers"; export * from "./access-approval-requests"; export * from "./access-approval-requests-reviewers"; export * from "./api-keys"; diff --git a/backend/src/db/schemas/models.ts b/backend/src/db/schemas/models.ts index 068ef74ad..2ed5c0326 100644 --- a/backend/src/db/schemas/models.ts +++ b/backend/src/db/schemas/models.ts @@ -73,6 +73,7 @@ export enum TableName { ScimToken = "scim_tokens", AccessApprovalPolicy = "access_approval_policies", AccessApprovalPolicyApprover = "access_approval_policies_approvers", + AccessApprovalPolicyGroupApprover = "access_approval_policies_group_approvers", AccessApprovalRequest = "access_approval_requests", AccessApprovalRequestReviewer = "access_approval_requests_reviewers", SecretApprovalPolicy = "secret_approval_policies", diff --git a/backend/src/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index a57a26d81..4a0bc8983 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -17,13 +17,14 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi name: z.string().optional(), secretPath: z.string().trim().default("/"), environment: z.string(), - approvers: z.string().array().min(1), + approvers: z.string().array().default([]), + groupApprovers: z.string().array().default([]), approvals: z.number().min(1).default(1), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers.length > 0 || data.groupApprovers.length > 0, { + path: ["approvers", "groupApprovers"], + message: "At least one approver should be provided." }), response: { 200: z.object({ @@ -63,6 +64,11 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi userId: z.string() }) .array(), + groupApprovers: z + .object({ + groupId: z.string() + }) + .array(), secretPath: z.string().optional().nullable() }) .array() @@ -127,13 +133,14 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi .trim() .optional() .transform((val) => (val === "" ? "/" : val)), - approvers: z.string().array().min(1), - approvals: z.number().min(1).default(1), + approvers: z.string().array().optional().default([]), + approvals: z.number().min(1).optional(), + groupApprovers: z.string().array().optional().default([]), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers || data.groupApprovers, { + path: ["approvers", "groupApprovers"], + message: "At least one approver should be provided." }), response: { 200: z.object({ diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts index c224bd3ca..b5c854e41 100644 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts @@ -20,7 +20,13 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.AccessApprovalPolicyGroupApprover, + `${TableName.AccessApprovalPolicy}.id`, + `${TableName.AccessApprovalPolicyGroupApprover}.policyId` + ) .select(tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover)) + .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyGroupApprover)) .select(tx.ref("name").withSchema(TableName.Environment).as("envName")) .select(tx.ref("slug").withSchema(TableName.Environment).as("envSlug")) .select(tx.ref("id").withSchema(TableName.Environment).as("envId")) @@ -54,6 +60,13 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { mapper: ({ approverUserId }) => ({ userId: approverUserId }) + }, + { + key: "approverGroupId", + label: "groupApprovers" as const, + mapper: ({ approverGroupId }) => ({ + groupId: approverGroupId + }) } ] }); @@ -88,6 +101,13 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { mapper: ({ approverUserId }) => ({ userId: approverUserId }) + }, + { + key: "approverGroupId", + label: "groupApprovers" as const, + mapper: ({ approverGroupId }) => ({ + groupId: approverGroupId + }) } ] }); diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts new file mode 100644 index 000000000..baa330543 --- /dev/null +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts @@ -0,0 +1,12 @@ +import { TDbClient } from "@app/db"; +import { TableName } from "@app/db/schemas"; +import { ormify } from "@app/lib/knex"; + +export type TAccessApprovalPolicyGroupApproverDALFactory = ReturnType< + typeof accessApprovalPolicyGroupApproverDALFactory +>; + +export const accessApprovalPolicyGroupApproverDALFactory = (db: TDbClient) => { + const accessApprovalPolicyGroupApproverOrm = ormify(db, TableName.AccessApprovalPolicyGroupApprover); + return { ...accessApprovalPolicyGroupApproverOrm }; +}; 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 153941771..0e75416dd 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,13 +3,16 @@ 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 { logger } from "@app/lib/logger"; 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 { 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 { TAccessApprovalPolicyGroupApproverDALFactory } from "./access-approval-policy-group-approver-dal"; import { TCreateAccessApprovalPolicy, TDeleteAccessApprovalPolicy, @@ -25,13 +28,17 @@ type TSecretApprovalPolicyServiceFactoryDep = { projectEnvDAL: Pick; accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory; projectMembershipDAL: Pick; + groupDAL: TGroupDALFactory; + accessApprovalPolicyGroupApproverDAL: TAccessApprovalPolicyGroupApproverDALFactory; }; export type TAccessApprovalPolicyServiceFactory = ReturnType; export const accessApprovalPolicyServiceFactory = ({ accessApprovalPolicyDAL, + accessApprovalPolicyGroupApproverDAL, accessApprovalPolicyApproverDAL, + groupDAL, permissionService, projectEnvDAL, projectDAL @@ -45,6 +52,7 @@ export const accessApprovalPolicyServiceFactory = ({ actorAuthMethod, approvals, approvers, + groupApprovers, projectSlug, environment, enforcementLevel @@ -52,7 +60,11 @@ export const accessApprovalPolicyServiceFactory = ({ const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId); if (!project) throw new BadRequestError({ message: "Project not found" }); - if (approvals > approvers.length) + if (!groupApprovers && !approvers) + throw new BadRequestError({ message: "Either of approvers or group approvers must be provided" }); + + // If there is a group approver people might be added to the group later to meet the approvers quota + if (!groupApprovers && approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -69,6 +81,35 @@ export const accessApprovalPolicyServiceFactory = ({ const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id }); if (!env) throw new BadRequestError({ message: "Environment not found" }); + const verifyAllApprovers = approvers; + const usersPromises: Promise< + { + id: string; + email: string | null | undefined; + username: string; + firstName: string | null | undefined; + lastName: string | null | undefined; + isPartOfGroup: boolean; + }[] + >[] = []; + + for (const groupId of groupApprovers) { + usersPromises.push(groupDAL.findAllGroupMembers({ orgId: actorOrgId, groupId, offset: 0 })); + } + const verifyGroupApprovers = (await Promise.all(usersPromises)).flat().map((user) => user.id); + verifyAllApprovers.push(...verifyGroupApprovers); + + logger.info("verifyApproversCreate"); + logger.info({ + projectId: project.id, + orgId: actorOrgId, + envSlug: environment, + secretPath, + actorAuthMethod, + permissionService, + userIds: verifyAllApprovers + }); + await verifyApprovers({ projectId: project.id, orgId: actorOrgId, @@ -76,7 +117,7 @@ export const accessApprovalPolicyServiceFactory = ({ secretPath, actorAuthMethod, permissionService, - userIds: approvers + userIds: verifyAllApprovers }); const accessApproval = await accessApprovalPolicyDAL.transaction(async (tx) => { @@ -97,6 +138,15 @@ export const accessApprovalPolicyServiceFactory = ({ })), tx ); + + await accessApprovalPolicyGroupApproverDAL.insertMany( + groupApprovers.map((groupId) => ({ + approverGroupId: groupId, + policyId: doc.id + })), + tx + ); + return doc; }); return { ...accessApproval, environment: env, projectId: project.id }; @@ -129,6 +179,7 @@ export const accessApprovalPolicyServiceFactory = ({ const updateAccessApprovalPolicy = async ({ policyId, approvers, + groupApprovers, secretPath, name, actorId, @@ -162,6 +213,16 @@ export const accessApprovalPolicyServiceFactory = ({ tx ); if (approvers) { + logger.info("verifyApproversPatch"); + logger.info({ + projectId: accessApprovalPolicy.projectId, + orgId: actorOrgId, + envSlug: accessApprovalPolicy.environment.slug, + secretPath, + actorAuthMethod, + permissionService, + userIds: approvers + }); await verifyApprovers({ projectId: accessApprovalPolicy.projectId, orgId: actorOrgId, @@ -171,7 +232,6 @@ export const accessApprovalPolicyServiceFactory = ({ permissionService, userIds: approvers }); - await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); await accessApprovalPolicyApproverDAL.insertMany( approvers.map((userId) => ({ @@ -181,6 +241,43 @@ export const accessApprovalPolicyServiceFactory = ({ tx ); } + + if (groupApprovers) { + const usersPromises: Promise< + { + id: string; + email: string | null | undefined; + username: string; + firstName: string | null | undefined; + lastName: string | null | undefined; + isPartOfGroup: boolean; + }[] + >[] = []; + + for (const groupId of groupApprovers) { + usersPromises.push(groupDAL.findAllGroupMembers({ orgId: actorOrgId, groupId, offset: 0 })); + } + const verifyGroupApprovers = (await Promise.all(usersPromises)).flat().map((user) => user.id); + + await verifyApprovers({ + projectId: accessApprovalPolicy.projectId, + orgId: actorOrgId, + envSlug: accessApprovalPolicy.environment.slug, + secretPath: doc.secretPath!, + actorAuthMethod, + permissionService, + userIds: verifyGroupApprovers + }); + await accessApprovalPolicyGroupApproverDAL.delete({ policyId: doc.id }, tx); + await accessApprovalPolicyGroupApproverDAL.insertMany( + groupApprovers.map((groupId) => ({ + approverGroupId: groupId, + policyId: doc.id + })), + tx + ); + } + return doc; }); return { diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts index fdb6fc8bb..47d7ead9e 100644 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts @@ -18,6 +18,7 @@ export type TCreateAccessApprovalPolicy = { secretPath: string; environment: string; approvers: string[]; + groupApprovers: string[]; projectSlug: string; name: string; enforcementLevel: EnforcementLevel; @@ -27,6 +28,7 @@ export type TUpdateAccessApprovalPolicy = { policyId: string; approvals?: number; approvers?: string[]; + groupApprovers?: string[]; secretPath?: string; name?: string; enforcementLevel?: EnforcementLevel; diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts index 48e2d88bf..5746ecb59 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts @@ -39,6 +39,17 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.AccessApprovalPolicyGroupApprover, + `${TableName.AccessApprovalPolicy}.id`, + `${TableName.AccessApprovalPolicyGroupApprover}.policyId` + ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.AccessApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + .leftJoin(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`) .join( db(TableName.Users).as("requestedByUser"), @@ -59,6 +70,7 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { ) .select(db.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover)) + .select(db.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId")) .select( db.ref("projectId").withSchema(TableName.Environment), @@ -142,7 +154,12 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { label: "reviewers" as const, mapper: ({ reviewerUserId: userId, reviewerStatus: status }) => (userId ? { userId, status } : undefined) }, - { key: "approverUserId", label: "approvers" as const, mapper: ({ approverUserId }) => approverUserId } + { key: "approverUserId", label: "approvers" as const, mapper: ({ approverUserId }) => approverUserId }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ approverGroupUserId }) => approverGroupUserId + } ] }); @@ -172,18 +189,36 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { `requestedByUser.id` ) - .join( + .leftJoin( TableName.AccessApprovalPolicyApprover, `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) - .join( + .leftJoin( db(TableName.Users).as("accessApprovalPolicyApproverUser"), `${TableName.AccessApprovalPolicyApprover}.approverUserId`, "accessApprovalPolicyApproverUser.id" ) + .leftJoin( + TableName.AccessApprovalPolicyGroupApprover, + `${TableName.AccessApprovalPolicy}.id`, + `${TableName.AccessApprovalPolicyGroupApprover}.policyId` + ) + + .leftJoin( + TableName.UserGroupMembership, + `${TableName.AccessApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + + .leftJoin( + db(TableName.Users).as("accessApprovalPolicyGroupApproverUser"), + `${TableName.UserGroupMembership}.userId`, + "accessApprovalPolicyGroupApproverUser.id" + ) + .leftJoin( TableName.AccessApprovalRequestReviewer, `${TableName.AccessApprovalRequest}.id`, @@ -200,10 +235,15 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { .select(selectAllTableCols(TableName.AccessApprovalRequest)) .select( tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover), + tx.ref("userId").withSchema(TableName.UserGroupMembership), tx.ref("email").withSchema("accessApprovalPolicyApproverUser").as("approverEmail"), + tx.ref("email").withSchema("accessApprovalPolicyGroupApproverUser").as("approverGroupEmail"), tx.ref("username").withSchema("accessApprovalPolicyApproverUser").as("approverUsername"), + tx.ref("username").withSchema("accessApprovalPolicyGroupApproverUser").as("approverGroupUsername"), tx.ref("firstName").withSchema("accessApprovalPolicyApproverUser").as("approverFirstName"), + tx.ref("firstName").withSchema("accessApprovalPolicyGroupApproverUser").as("approverGroupFirstName"), tx.ref("lastName").withSchema("accessApprovalPolicyApproverUser").as("approverLastName"), + tx.ref("lastName").withSchema("accessApprovalPolicyGroupApproverUser").as("approverGroupLastName"), tx.ref("email").withSchema("requestedByUser").as("requestedByUserEmail"), tx.ref("username").withSchema("requestedByUser").as("requestedByUserUsername"), tx.ref("firstName").withSchema("requestedByUser").as("requestedByUserFirstName"), @@ -282,6 +322,23 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { lastName, username }) + }, + { + key: "userId", + label: "approvers" as const, + mapper: ({ + userId, + approverGroupEmail: email, + approverGroupUsername: username, + approverGroupLastName: lastName, + approverFirstName: firstName + }) => ({ + userId, + email, + firstName, + lastName, + username + }) } ] }); diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts index 2a7953ead..108e4607e 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts @@ -18,6 +18,8 @@ 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 { TAccessApprovalPolicyGroupApproverDALFactory } from "../access-approval-policy/access-approval-policy-group-approver-dal"; +import { TGroupDALFactory } from "../group/group-dal"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { TProjectUserAdditionalPrivilegeDALFactory } from "../project-user-additional-privilege/project-user-additional-privilege-dal"; import { ProjectUserAdditionalPrivilegeTemporaryMode } from "../project-user-additional-privilege/project-user-additional-privilege-types"; @@ -36,6 +38,7 @@ type TSecretApprovalRequestServiceFactoryDep = { additionalPrivilegeDAL: Pick; permissionService: Pick; accessApprovalPolicyApproverDAL: Pick; + accessApprovalPolicyGroupApproverDAL: Pick; projectEnvDAL: Pick; projectDAL: Pick< TProjectDALFactory, @@ -57,6 +60,7 @@ type TSecretApprovalRequestServiceFactoryDep = { TAccessApprovalRequestReviewerDALFactory, "create" | "find" | "findOne" | "transaction" >; + groupDAL: Pick; projectMembershipDAL: Pick; smtpService: Pick; userDAL: Pick< @@ -70,6 +74,7 @@ type TSecretApprovalRequestServiceFactoryDep = { export type TAccessApprovalRequestServiceFactory = ReturnType; export const accessApprovalRequestServiceFactory = ({ + groupDAL, projectDAL, projectEnvDAL, permissionService, @@ -78,6 +83,7 @@ export const accessApprovalRequestServiceFactory = ({ projectMembershipDAL, accessApprovalPolicyDAL, accessApprovalPolicyApproverDAL, + accessApprovalPolicyGroupApproverDAL, additionalPrivilegeDAL, smtpService, userDAL, @@ -124,13 +130,35 @@ export const accessApprovalRequestServiceFactory = ({ }); if (!policy) throw new UnauthorizedError({ message: "No policy matching criteria was found." }); + const approverIds = []; + const approvers = await accessApprovalPolicyApproverDAL.find({ policyId: policy.id }); + approvers.forEach((approver) => { + approverIds.push(approver.approverUserId); + }); + + const groupApprovers = await accessApprovalPolicyGroupApproverDAL.find({ + policyId: policy.id + }); + + const groupUsers = ( + await Promise.all( + groupApprovers.map((groupApprover) => + groupDAL.findAllGroupMembers({ + orgId: actorOrgId, + groupId: groupApprover.id + }) + ) + ) + ).flat(); + approverIds.push(...groupUsers.map((user) => user.id)); + const approverUsers = await userDAL.find({ $in: { - id: approvers.map((approver) => approver.approverUserId) + id: [...new Set(approverIds)] } }); diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 3eb6b0031..c1fd71494 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -7,6 +7,7 @@ import { registerCertificateEstRouter } from "@app/ee/routes/est/certificate-est import { registerV1EERoutes } from "@app/ee/routes/v1"; import { accessApprovalPolicyApproverDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-approver-dal"; import { accessApprovalPolicyDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-dal"; +import { accessApprovalPolicyGroupApproverDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-group-approver-dal"; import { accessApprovalPolicyServiceFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-service"; import { accessApprovalRequestDALFactory } from "@app/ee/services/access-approval-request/access-approval-request-dal"; import { accessApprovalRequestReviewerDALFactory } from "@app/ee/services/access-approval-request/access-approval-request-reviewer-dal"; @@ -626,6 +627,7 @@ export const registerRoutes = async ( const pkiAlertDAL = pkiAlertDALFactory(db); const pkiCollectionDAL = pkiCollectionDALFactory(db); const pkiCollectionItemDAL = pkiCollectionItemDALFactory(db); + const accessApprovalPolicyGroupApproverDAL = accessApprovalPolicyGroupApproverDALFactory(db); const certificateService = certificateServiceFactory({ certificateDAL, @@ -923,6 +925,8 @@ export const registerRoutes = async ( const accessApprovalPolicyService = accessApprovalPolicyServiceFactory({ accessApprovalPolicyDAL, accessApprovalPolicyApproverDAL, + accessApprovalPolicyGroupApproverDAL, + groupDAL, permissionService, projectEnvDAL, projectMembershipDAL, @@ -942,7 +946,9 @@ export const registerRoutes = async ( smtpService, accessApprovalPolicyApproverDAL, projectSlackConfigDAL, - kmsService + kmsService, + accessApprovalPolicyGroupApproverDAL, + groupDAL }); const secretReplicationService = secretReplicationServiceFactory({ diff --git a/frontend/src/hooks/api/accessApproval/mutation.tsx b/frontend/src/hooks/api/accessApproval/mutation.tsx index 9c3199a99..c41b633b0 100644 --- a/frontend/src/hooks/api/accessApproval/mutation.tsx +++ b/frontend/src/hooks/api/accessApproval/mutation.tsx @@ -21,6 +21,7 @@ export const useCreateAccessApprovalPolicy = () => { projectSlug, approvals, approvers, + groupApprovers, name, secretPath, enforcementLevel @@ -30,6 +31,7 @@ export const useCreateAccessApprovalPolicy = () => { projectSlug, approvals, approvers, + groupApprovers, secretPath, name, enforcementLevel @@ -46,10 +48,11 @@ export const useUpdateAccessApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TUpdateAccessPolicyDTO>({ - mutationFn: async ({ id, approvers, approvals, name, secretPath, enforcementLevel }) => { + mutationFn: async ({ id, approvers, groupApprovers, approvals, name, secretPath, enforcementLevel }) => { const { data } = await apiRequest.patch(`/api/v1/access-approvals/policies/${id}`, { approvals, approvers, + groupApprovers, secretPath, name, enforcementLevel diff --git a/frontend/src/hooks/api/accessApproval/types.ts b/frontend/src/hooks/api/accessApproval/types.ts index 47917716d..37772878e 100644 --- a/frontend/src/hooks/api/accessApproval/types.ts +++ b/frontend/src/hooks/api/accessApproval/types.ts @@ -17,6 +17,7 @@ export type TAccessApprovalPolicy = { enforcementLevel: EnforcementLevel; updatedAt: Date; userApprovers?: { userId: string }[]; + groupApprovers?: { groupId: string }[]; }; export type TAccessApprovalRequest = { @@ -131,6 +132,7 @@ export type TCreateAccessPolicyDTO = { name?: string; environment: string; approvers?: string[]; + groupApprovers?: string[]; approvals?: number; secretPath?: string; enforcementLevel?: EnforcementLevel; @@ -140,6 +142,7 @@ export type TUpdateAccessPolicyDTO = { id: string; name?: string; approvers?: string[]; + groupApprovers?: string[]; secretPath?: string; environment?: string; approvals?: number; diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx index 7234c18ce..0653a92e8 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx @@ -41,7 +41,8 @@ import { useDeleteAccessApprovalPolicy, useDeleteSecretApprovalPolicy, useGetSecretApprovalPolicies, - useGetWorkspaceUsers + useGetWorkspaceUsers, + useListWorkspaceGroups } from "@app/hooks/api"; import { useGetAccessApprovalPolicies } from "@app/hooks/api/accessApproval/queries"; import { PolicyType } from "@app/hooks/api/policies/enums"; @@ -102,6 +103,8 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { const { currentWorkspace } = useWorkspace(); const { data: members } = useGetWorkspaceUsers(workspaceId, true); + const { data: groups } = useListWorkspaceGroups(currentWorkspace?.slug || ""); + const { policies, isLoading: isPoliciesLoading } = useApprovalPolicies( permission, currentWorkspace @@ -186,6 +189,7 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { Environment Secret Path Eligible Approvers + Eligible Group Approvers Approval Required @@ -257,6 +261,7 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { workspaceId={workspaceId} key={policy.id} members={members} + groups={groups} onEdit={() => handlePopUpOpen("policyForm", policy)} onDelete={() => handlePopUpOpen("deletePolicy", policy)} /> diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx index 24a470bcc..a4218e582 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx @@ -22,7 +22,7 @@ import { } from "@app/components/v2"; import { useWorkspace } from "@app/context"; import { policyDetails } from "@app/helpers/policies"; -import { useCreateSecretApprovalPolicy, useUpdateSecretApprovalPolicy } from "@app/hooks/api"; +import { useCreateSecretApprovalPolicy, useListWorkspaceGroups, useUpdateSecretApprovalPolicy } from "@app/hooks/api"; import { useCreateAccessApprovalPolicy, useUpdateAccessApprovalPolicy @@ -45,13 +45,14 @@ const formSchema = z name: z.string().optional(), secretPath: z.string().optional(), approvals: z.number().min(1), - approvers: z.string().array().min(1), + approvers: z.string().array().optional(), + groupApprovers: z.string().array().optional(), policyType: z.nativeEnum(PolicyType), enforcementLevel: z.nativeEnum(EnforcementLevel) }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers || data.groupApprovers, { + path: ["approvers", "groupApprovers"], + message: "At least one approver should be provided." }); type TFormSchema = z.infer; @@ -75,11 +76,14 @@ export const AccessPolicyForm = ({ ? { ...editValues, environment: editValues.environment.slug, - approvers: editValues?.userApprovers?.map((user) => user.userId) || editValues?.approvers + approvers: editValues?.userApprovers?.map((user) => user.userId) || editValues?.approvers, + groupApprovers: editValues?.groupApprovers?.map((group) => group.groupId) || editValues?.groupApprovers, + approvals: editValues?.approvals } : undefined }); const { currentWorkspace } = useWorkspace(); + const { data: groups } = useListWorkspaceGroups(projectSlug); const environments = currentWorkspace?.environments || []; const isEditMode = Boolean(editValues); @@ -266,8 +270,7 @@ export const AccessPolicyForm = ({ name="approvers" render={({ field: { value, onChange }, fieldState: { error } }) => ( @@ -312,6 +315,56 @@ export const AccessPolicyForm = ({ )} /> + ( + + + + + + + + Select groups that are allowed to approve requests + + {groups && groups.map(({ group }) => { + const { id } = group; + const isChecked = value?.includes(id); + return ( + { + evt.preventDefault(); + onChange( + isChecked + ? value?.filter((el: string) => el !== id) + : [...(value || []), id] + ); + }} + key={`create-policy-members-${id}`} + iconPos="right" + icon={isChecked && } + > + {group.name} + + ); + })} + + + + )} + /> void; @@ -48,12 +51,14 @@ type Props = { export const ApprovalPolicyRow = ({ policy, members = [], + groups = [], projectSlug, workspaceId, onEdit, onDelete }: Props) => { const [selectedApprovers, setSelectedApprovers] = useState(policy.userApprovers?.map(({ userId }) => userId) || policy.approvers || []); + const [selectedGroupApprovers, setSelectedGroupApprovers] = useState(policy.groupApprovers?.map(({ groupId }) => groupId) || []); const { mutate: updateAccessApprovalPolicy, isLoading: isAccessApprovalPolicyLoading } = useUpdateAccessApprovalPolicy(); const { mutate: updateSecretApprovalPolicy, isLoading: isSecretApprovalPolicyLoading } = useUpdateSecretApprovalPolicy(); const isLoading = isAccessApprovalPolicyLoading || isSecretApprovalPolicyLoading; @@ -74,9 +79,10 @@ export const ApprovalPolicyRow = ({ { projectSlug, id: policy.id, - approvers: selectedApprovers + approvers: selectedApprovers, + groupApprovers: selectedGroupApprovers }, - { onSettled: () => {} } + { onSettled: () => { } } ); } else { updateSecretApprovalPolicy( @@ -85,7 +91,7 @@ export const ApprovalPolicyRow = ({ id: policy.id, approvers: selectedApprovers }, - { onSettled: () => {} } + { onSettled: () => { } } ); } } else { @@ -95,7 +101,7 @@ export const ApprovalPolicyRow = ({ ); } }} - > + > Select members that are allowed to approve changes - {members?.map(({ id, user }) => { - const userId = policy.policyType === PolicyType.ChangePolicy ? user.id : id; + {members?.map(({ user }) => { + const userId = user.id; const isChecked = selectedApprovers.includes(userId); return ( + + { + if (!isOpen) { + if (policy.policyType === PolicyType.AccessPolicy) { + updateAccessApprovalPolicy( + { + projectSlug, + id: policy.id, + approvers: selectedApprovers, + groupApprovers: selectedGroupApprovers + }, + { onSettled: () => { } } + ); + } else { + updateSecretApprovalPolicy( + { + workspaceId, + id: policy.id, + approvers: selectedApprovers, + }, + { onSettled: () => { } } + ); + } + } else { + setSelectedGroupApprovers(policy.policyType === PolicyType.ChangePolicy + ? policy?.groupApprovers?.map(({ groupId }) => groupId) || [] + : policy?.groupApprovers?.map(({groupId}) => groupId) || [] + ); + } + }} + > + + + + + + Select groups that are allowed to approve requests + + {groups && groups.map(({ group }) => { + const { id } = group; + const isChecked = selectedGroupApprovers?.includes(id); + return ( + { + evt.preventDefault(); + setSelectedGroupApprovers( + isChecked + ? selectedGroupApprovers?.filter((el: string) => el !== id) + : [...(selectedGroupApprovers || []), id] + ); + }} + key={`create-policy-groups-${id}`} + iconPos="right" + icon={isChecked && } + > + {group.name} + + ); + })} + + + {policy.approvals} From 081502848da26c9a5bcf591b6b6f8f50ebd09e3a Mon Sep 17 00:00:00 2001 From: Meet Date: Fri, 20 Sep 2024 08:51:48 +0530 Subject: [PATCH 2/8] feat: allow secret approvals with user groups --- backend/src/@types/knex.d.ts | 5 ++ ...40919205505_add-group-approvals-secrets.ts | 22 +++++ backend/src/db/schemas/index.ts | 1 + backend/src/db/schemas/models.ts | 1 + ...ecret-approval-policies-group-approvers.ts | 25 ++++++ .../v1/secret-approval-policy-router.ts | 23 +++-- .../v1/secret-approval-request-router.ts | 12 ++- .../secret-approval-policy-dal.ts | 35 +++++++- ...cret-approval-policy-group-approver-dal.ts | 12 +++ .../secret-approval-policy-service.ts | 29 ++++++- .../secret-approval-policy-types.ts | 4 +- .../secret-approval-request-dal.ts | 85 +++++++++++++++++-- backend/src/server/routes/index.ts | 3 + .../src/hooks/api/secretApproval/mutation.tsx | 5 +- .../src/hooks/api/secretApproval/types.ts | 3 + .../components/ApprovalPolicyRow.tsx | 4 +- 16 files changed, 248 insertions(+), 21 deletions(-) create mode 100644 backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts create mode 100644 backend/src/db/schemas/secret-approval-policies-group-approvers.ts create mode 100644 backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts diff --git a/backend/src/@types/knex.d.ts b/backend/src/@types/knex.d.ts index d7b484dfc..81e7c6d4b 100644 --- a/backend/src/@types/knex.d.ts +++ b/backend/src/@types/knex.d.ts @@ -806,5 +806,10 @@ declare module "knex/types/tables" { TAccessApprovalPoliciesGroupApproversInsert, TAccessApprovalPoliciesGroupApproversInsert >; + [TableName.SecretApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< + TSecretApprovalPoliciesGroupApprovers, + TSecretApprovalPoliciesGroupApproversInsert, + TSecretApprovalPoliciesGroupApproversInsert + >; } } diff --git a/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts b/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts new file mode 100644 index 000000000..8a665fc2c --- /dev/null +++ b/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts @@ -0,0 +1,22 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; +import { createOnUpdateTrigger } from "../utils"; + +export async function up(knex: Knex): Promise { + if (!(await knex.schema.hasTable(TableName.SecretApprovalPolicyGroupApprover))) { + await knex.schema.createTable(TableName.SecretApprovalPolicyGroupApprover, (t) => { + t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); + t.uuid("approverGroupId").notNullable(); + t.foreign("approverGroupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); + t.uuid("policyId").notNullable(); + t.foreign("policyId").references("id").inTable(TableName.SecretApprovalPolicy).onDelete("CASCADE"); + t.timestamps(true, true, true); + }); + await createOnUpdateTrigger(knex, TableName.SecretApprovalPolicyGroupApprover); + } +} + +export async function down(knex: Knex): Promise { + await knex.schema.dropTableIfExists(TableName.SecretApprovalPolicyGroupApprover); +} diff --git a/backend/src/db/schemas/index.ts b/backend/src/db/schemas/index.ts index b145684b4..1a3978532 100644 --- a/backend/src/db/schemas/index.ts +++ b/backend/src/db/schemas/index.ts @@ -72,6 +72,7 @@ export * from "./saml-configs"; export * from "./scim-tokens"; export * from "./secret-approval-policies"; export * from "./secret-approval-policies-approvers"; +export * from "./secret-approval-policies-group-approvers"; export * from "./secret-approval-request-secret-tags"; export * from "./secret-approval-request-secret-tags-v2"; export * from "./secret-approval-requests"; diff --git a/backend/src/db/schemas/models.ts b/backend/src/db/schemas/models.ts index 2ed5c0326..b31c0dba6 100644 --- a/backend/src/db/schemas/models.ts +++ b/backend/src/db/schemas/models.ts @@ -78,6 +78,7 @@ export enum TableName { AccessApprovalRequestReviewer = "access_approval_requests_reviewers", SecretApprovalPolicy = "secret_approval_policies", SecretApprovalPolicyApprover = "secret_approval_policies_approvers", + SecretApprovalPolicyGroupApprover = "secret_approval_policies_group_approvers", SecretApprovalRequest = "secret_approval_requests", SecretApprovalRequestReviewer = "secret_approval_requests_reviewers", SecretApprovalRequestSecret = "secret_approval_requests_secrets", diff --git a/backend/src/db/schemas/secret-approval-policies-group-approvers.ts b/backend/src/db/schemas/secret-approval-policies-group-approvers.ts new file mode 100644 index 000000000..6bfec26d2 --- /dev/null +++ b/backend/src/db/schemas/secret-approval-policies-group-approvers.ts @@ -0,0 +1,25 @@ +// Code generated by automation script, DO NOT EDIT. +// Automated by pulling database and generating zod schema +// To update. Just run npm run generate:schema +// Written by akhilmhdh. + +import { z } from "zod"; + +import { TImmutableDBKeys } from "./models"; + +export const SecretApprovalPoliciesGroupApproversSchema = z.object({ + id: z.string().uuid(), + approverGroupId: z.string().uuid(), + policyId: z.string().uuid(), + createdAt: z.date(), + updatedAt: z.date() +}); + +export type TSecretApprovalPoliciesGroupApprovers = z.infer; +export type TSecretApprovalPoliciesGroupApproversInsert = Omit< + z.input, + TImmutableDBKeys +>; +export type TSecretApprovalPoliciesGroupApproversUpdate = Partial< + Omit, TImmutableDBKeys> +>; diff --git a/backend/src/ee/routes/v1/secret-approval-policy-router.ts b/backend/src/ee/routes/v1/secret-approval-policy-router.ts index 25c1bb0b5..8150fb5a5 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -27,13 +27,14 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .nullable() .default("/") .transform((val) => (val ? removeTrailingSlash(val) : val)), - approvers: z.string().array().min(1), + approvers: z.string().array().optional().default([]), + groupApprovers: z.string().array().optional().default([]), approvals: z.number().min(1).default(1), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers || data.groupApprovers, { + path: ["approvers", "groupApprovers"], + message: "At least one approver should be provided." }), response: { 200: z.object({ @@ -70,8 +71,9 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi body: z .object({ name: z.string().optional(), - approvers: z.string().array().min(1), + approvers: z.string().array().optional().default([]), approvals: z.number().min(1).default(1), + groupApprovers: z.string().array().optional().default([]), secretPath: z .string() .optional() @@ -80,9 +82,9 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .transform((val) => (val === "" ? "/" : val)), enforcementLevel: z.nativeEnum(EnforcementLevel).optional() }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers.length > 0 || data.groupApprovers.length > 0, { + path: ["approvers", "groupApprovers"], + message: "At least one approver should be provided." }), response: { 200: z.object({ @@ -151,6 +153,11 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .object({ userId: z.string() }) + .array(), + groupApprovers: z + .object({ + groupId: z.string() + }) .array() }) .array() diff --git a/backend/src/ee/routes/v1/secret-approval-request-router.ts b/backend/src/ee/routes/v1/secret-approval-request-router.ts index bcc4a36c5..29287974e 100644 --- a/backend/src/ee/routes/v1/secret-approval-request-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-request-router.ts @@ -46,7 +46,11 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv id: z.string(), name: z.string(), approvals: z.number(), - approvers: z.string().array(), + approvers: z + .object({ + userId: z.string() + }) + .array(), secretPath: z.string().optional().nullable(), enforcementLevel: z.string() }), @@ -54,7 +58,11 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv commits: z.object({ op: z.string(), secretId: z.string().nullable().optional() }).array(), environment: z.string(), reviewers: z.object({ userId: z.string(), status: z.string() }).array(), - approvers: z.string().array() + approvers: z + .object({ + userId: z.string() + }) + .array() }).array() }) } diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts index 6d5168f26..51dba2efc 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts @@ -22,13 +22,29 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { ) .leftJoin(TableName.Users, `${TableName.SecretApprovalPolicyApprover}.approverUserId`, `${TableName.Users}.id`) - + .leftJoin( + TableName.SecretApprovalPolicyGroupApprover, + `${TableName.SecretApprovalPolicy}.id`, + `${TableName.SecretApprovalPolicyGroupApprover}.policyId` + ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.userId` + ) .select( tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), tx.ref("email").withSchema(TableName.Users).as("approverEmail"), tx.ref("firstName").withSchema(TableName.Users).as("approverFirstName"), tx.ref("lastName").withSchema(TableName.Users).as("approverLastName") ) + .select( + tx.ref("approverGroupId").withSchema(TableName.SecretApprovalPolicyGroupApprover), + tx.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId"), + tx.ref("email").withSchema(TableName.Users).as("approverGroupEmail"), + tx.ref("firstName").withSchema(TableName.Users).as("approverGroupFirstName"), + tx.ref("lastName").withSchema(TableName.Users).as("approverGroupLastName") + ) .select( tx.ref("name").withSchema(TableName.Environment).as("envName"), tx.ref("slug").withSchema(TableName.Environment).as("envSlug"), @@ -61,6 +77,16 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { firstName: approverFirstName, lastName: approverLastName }) + }, + { + key: "approverGroupUserId", + label: "userApprovers" as const, + mapper: ({ approverGroupUserId, approverGroupEmail, approverGroupFirstName, approverGroupLastName }) => ({ + userId: approverGroupUserId, + email: approverGroupEmail, + firstName: approverGroupFirstName, + lastName: approverGroupLastName + }) } ] }); @@ -89,6 +115,13 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { mapper: ({ approverUserId }) => ({ userId: approverUserId }) + }, + { + key: "approverGroupId", + label: "groupApprovers" as const, + mapper: ({ approverGroupId }) => ({ + groupId: approverGroupId + }) } ] }); diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts new file mode 100644 index 000000000..710d251d5 --- /dev/null +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts @@ -0,0 +1,12 @@ +import { TDbClient } from "@app/db"; +import { TableName } from "@app/db/schemas"; +import { ormify } from "@app/lib/knex"; + +export type TSecretApprovalPolicyGroupApproverDALFactory = ReturnType< + typeof secretApprovalPolicyGroupApproverDALFactory +>; + +export const secretApprovalPolicyGroupApproverDALFactory = (db: TDbClient) => { + const sapGroupApproverOrm = ormify(db, TableName.SecretApprovalPolicyGroupApprover); + return sapGroupApproverOrm; +}; diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts index 8c974b05d..b9a385673 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts @@ -11,6 +11,7 @@ import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal import { TLicenseServiceFactory } from "../license/license-service"; import { TSecretApprovalPolicyApproverDALFactory } from "./secret-approval-policy-approver-dal"; import { TSecretApprovalPolicyDALFactory } from "./secret-approval-policy-dal"; +import { TSecretApprovalPolicyGroupApproverDALFactory } from "./secret-approval-policy-group-approver-dal"; import { TCreateSapDTO, TDeleteSapDTO, @@ -29,6 +30,7 @@ type TSecretApprovalPolicyServiceFactoryDep = { secretApprovalPolicyDAL: TSecretApprovalPolicyDALFactory; projectEnvDAL: Pick; secretApprovalPolicyApproverDAL: TSecretApprovalPolicyApproverDALFactory; + secretApprovalPolicyGroupApproverDAL: TSecretApprovalPolicyGroupApproverDALFactory; licenseService: Pick; }; @@ -38,6 +40,7 @@ export const secretApprovalPolicyServiceFactory = ({ secretApprovalPolicyDAL, permissionService, secretApprovalPolicyApproverDAL, + secretApprovalPolicyGroupApproverDAL, projectEnvDAL, licenseService }: TSecretApprovalPolicyServiceFactoryDep) => { @@ -49,12 +52,16 @@ export const secretApprovalPolicyServiceFactory = ({ actorAuthMethod, approvals, approvers, + groupApprovers, projectId, secretPath, environment, enforcementLevel }: TCreateSapDTO) => { - if (approvals > approvers.length) + if (!groupApprovers && !approvers) + throw new BadRequestError({ message: "Either of approvers or group approvers must be provided" }); + + if (!groupApprovers && approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -91,6 +98,7 @@ export const secretApprovalPolicyServiceFactory = ({ }, tx ); + await secretApprovalPolicyApproverDAL.insertMany( approvers.map((approverUserId) => ({ approverUserId, @@ -98,6 +106,14 @@ export const secretApprovalPolicyServiceFactory = ({ })), tx ); + + await secretApprovalPolicyGroupApproverDAL.insertMany( + groupApprovers.map((approverGroupId) => ({ + approverGroupId, + policyId: doc.id + })), + tx + ); return doc; }); return { ...secretApproval, environment: env, projectId }; @@ -105,6 +121,7 @@ export const secretApprovalPolicyServiceFactory = ({ const updateSecretApprovalPolicy = async ({ approvers, + groupApprovers, secretPath, name, actorId, @@ -156,6 +173,16 @@ export const secretApprovalPolicyServiceFactory = ({ tx ); } + if (groupApprovers) { + await secretApprovalPolicyGroupApproverDAL.delete({ policyId: doc.id }, tx); + await secretApprovalPolicyGroupApproverDAL.insertMany( + groupApprovers.map((approverGroupId) => ({ + approverGroupId, + policyId: doc.id + })), + tx + ); + } return doc; }); return { diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts index 8e7099c98..5199aff64 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts @@ -5,6 +5,7 @@ export type TCreateSapDTO = { secretPath?: string | null; environment: string; approvers: string[]; + groupApprovers: string[]; projectId: string; name: string; enforcementLevel: EnforcementLevel; @@ -14,7 +15,8 @@ export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; secretPath?: string | null; - approvers: string[]; + approvers?: string[]; + groupApprovers?: string[]; name?: string; enforcementLevel?: EnforcementLevel; } & Omit; diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts index 54c7563f6..75f19861c 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts @@ -48,16 +48,31 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalRequest}.committerUserId`, `committerUser.id` ) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - .join( + .leftJoin( db(TableName.Users).as("secretApprovalPolicyApproverUser"), `${TableName.SecretApprovalPolicyApprover}.approverUserId`, "secretApprovalPolicyApproverUser.id" ) + .leftJoin( + TableName.SecretApprovalPolicyGroupApprover, + `${TableName.SecretApprovalPolicy}.id`, + `${TableName.SecretApprovalPolicyGroupApprover}.policyId` + ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + .leftJoin( + db(TableName.Users).as("secretApprovalPolicyGroupApproverUser"), + `${TableName.UserGroupMembership}.userId`, + `secretApprovalPolicyGroupApproverUser.id` + ) .leftJoin( TableName.SecretApprovalRequestReviewer, `${TableName.SecretApprovalRequest}.id`, @@ -71,10 +86,15 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { .select(selectAllTableCols(TableName.SecretApprovalRequest)) .select( tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), + tx.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId"), tx.ref("email").withSchema("secretApprovalPolicyApproverUser").as("approverEmail"), + tx.ref("email").withSchema("secretApprovalPolicyGroupApproverUser").as("approverGroupEmail"), tx.ref("username").withSchema("secretApprovalPolicyApproverUser").as("approverUsername"), + tx.ref("username").withSchema("secretApprovalPolicyGroupApproverUser").as("approverGroupUsername"), tx.ref("firstName").withSchema("secretApprovalPolicyApproverUser").as("approverFirstName"), + tx.ref("firstName").withSchema("secretApprovalPolicyGroupApproverUser").as("approverGroupFirstName"), tx.ref("lastName").withSchema("secretApprovalPolicyApproverUser").as("approverLastName"), + tx.ref("lastName").withSchema("secretApprovalPolicyGroupApproverUser").as("approverGroupLastName"), tx.ref("email").withSchema("statusChangedByUser").as("statusChangedByUserEmail"), tx.ref("username").withSchema("statusChangedByUser").as("statusChangedByUserUsername"), tx.ref("firstName").withSchema("statusChangedByUser").as("statusChangedByUserFirstName"), @@ -164,6 +184,23 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { lastName, username }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ + approverGroupUserId, + approverGroupEmail: email, + approverGroupUsername: username, + approverGroupLastName: lastName, + approverGroupFirstName: firstName + }) => ({ + userId: approverGroupUserId, + email, + firstName, + lastName, + username + }) } ] }); @@ -236,11 +273,21 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalRequest}.policyId`, `${TableName.SecretApprovalPolicy}.id` ) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.SecretApprovalPolicyGroupApprover, + `${TableName.SecretApprovalPolicy}.id`, + `${TableName.SecretApprovalPolicyGroupApprover}.policyId` + ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) .join( db(TableName.Users).as("committerUser"), `${TableName.SecretApprovalRequest}.committerUserId`, @@ -269,6 +316,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { void bd .where(`${TableName.SecretApprovalPolicyApprover}.approverUserId`, userId) .orWhere(`${TableName.SecretApprovalRequest}.committerUserId`, userId) + .orWhere(`${TableName.UserGroupMembership}.userId`, userId) ) .select(selectAllTableCols(TableName.SecretApprovalRequest)) .select( @@ -289,6 +337,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { db.ref("enforcementLevel").withSchema(TableName.SecretApprovalPolicy).as("policyEnforcementLevel"), db.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals"), db.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), + db.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId"), db.ref("email").withSchema("committerUser").as("committerUserEmail"), db.ref("username").withSchema("committerUser").as("committerUserUsername"), db.ref("firstName").withSchema("committerUser").as("committerUserFirstName"), @@ -334,7 +383,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId }) => approverUserId + mapper: ({ approverUserI: userId }) => ({ userId }) }, { key: "commitId", @@ -344,6 +393,11 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { id, secretId }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ approverGroupUserId: userId }) => ({ userId }) } ] }); @@ -371,11 +425,21 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalRequest}.policyId`, `${TableName.SecretApprovalPolicy}.id` ) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.SecretApprovalPolicyGroupApprover, + `${TableName.SecretApprovalPolicy}.id`, + `${TableName.SecretApprovalPolicyGroupApprover}.policyId` + ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) .join( db(TableName.Users).as("committerUser"), `${TableName.SecretApprovalRequest}.committerUserId`, @@ -404,6 +468,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { void bd .where(`${TableName.SecretApprovalPolicyApprover}.approverUserId`, userId) .orWhere(`${TableName.SecretApprovalRequest}.committerUserId`, userId) + .orWhere(`${TableName.UserGroupMembership}.userId`, userId) ) .select(selectAllTableCols(TableName.SecretApprovalRequest)) .select( @@ -424,6 +489,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { db.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals"), db.ref("enforcementLevel").withSchema(TableName.SecretApprovalPolicy).as("policyEnforcementLevel"), db.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), + db.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId"), db.ref("email").withSchema("committerUser").as("committerUserEmail"), db.ref("username").withSchema("committerUser").as("committerUserUsername"), db.ref("firstName").withSchema("committerUser").as("committerUserFirstName"), @@ -469,7 +535,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId }) => approverUserId + mapper: ({ approverUserId: userId }) => ({ userId }) }, { key: "commitId", @@ -479,6 +545,13 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { id, secretId }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ approverGroupUserId: userId }) => ({ + userId + }) } ] }); diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index c1fd71494..dbbcf256e 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -52,6 +52,7 @@ import { scimDALFactory } from "@app/ee/services/scim/scim-dal"; import { scimServiceFactory } from "@app/ee/services/scim/scim-service"; import { secretApprovalPolicyApproverDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-approver-dal"; import { secretApprovalPolicyDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-dal"; +import { secretApprovalPolicyGroupApproverDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal"; import { secretApprovalPolicyServiceFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-service"; import { secretApprovalRequestDALFactory } from "@app/ee/services/secret-approval-request/secret-approval-request-dal"; import { secretApprovalRequestReviewerDALFactory } from "@app/ee/services/secret-approval-request/secret-approval-request-reviewer-dal"; @@ -300,6 +301,7 @@ export const registerRoutes = async ( const accessApprovalRequestReviewerDAL = accessApprovalRequestReviewerDALFactory(db); const sapApproverDAL = secretApprovalPolicyApproverDALFactory(db); + const sapGroupApproverDAL = secretApprovalPolicyGroupApproverDALFactory(db); const secretApprovalPolicyDAL = secretApprovalPolicyDALFactory(db); const secretApprovalRequestDAL = secretApprovalRequestDALFactory(db); const secretApprovalRequestReviewerDAL = secretApprovalRequestReviewerDALFactory(db); @@ -379,6 +381,7 @@ export const registerRoutes = async ( const secretApprovalPolicyService = secretApprovalPolicyServiceFactory({ projectEnvDAL, secretApprovalPolicyApproverDAL: sapApproverDAL, + secretApprovalPolicyGroupApproverDAL: sapGroupApproverDAL, permissionService, secretApprovalPolicyDAL, licenseService diff --git a/frontend/src/hooks/api/secretApproval/mutation.tsx b/frontend/src/hooks/api/secretApproval/mutation.tsx index ceebd3493..9a7a97571 100644 --- a/frontend/src/hooks/api/secretApproval/mutation.tsx +++ b/frontend/src/hooks/api/secretApproval/mutation.tsx @@ -14,6 +14,7 @@ export const useCreateSecretApprovalPolicy = () => { workspaceId, approvals, approvers, + groupApprovers, secretPath, name, enforcementLevel @@ -23,6 +24,7 @@ export const useCreateSecretApprovalPolicy = () => { workspaceId, approvals, approvers, + groupApprovers, secretPath, name, enforcementLevel @@ -39,10 +41,11 @@ export const useUpdateSecretApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TUpdateSecretPolicyDTO>({ - mutationFn: async ({ id, approvers, approvals, secretPath, name, enforcementLevel }) => { + mutationFn: async ({ id, approvers, groupApprovers, approvals, secretPath, name, enforcementLevel }) => { const { data } = await apiRequest.patch(`/api/v1/secret-approvals/${id}`, { approvals, approvers, + groupApprovers, secretPath, name, enforcementLevel diff --git a/frontend/src/hooks/api/secretApproval/types.ts b/frontend/src/hooks/api/secretApproval/types.ts index 06ffad432..68ef8a99b 100644 --- a/frontend/src/hooks/api/secretApproval/types.ts +++ b/frontend/src/hooks/api/secretApproval/types.ts @@ -10,6 +10,7 @@ export type TSecretApprovalPolicy = { secretPath?: string; approvals: number; userApprovers: { userId: string }[]; + groupApprovers: { groupId: string }[]; updatedAt: Date; enforcementLevel: EnforcementLevel; }; @@ -30,6 +31,7 @@ export type TCreateSecretPolicyDTO = { environment: string; secretPath?: string | null; approvers?: string[]; + groupApprovers?: string[]; approvals?: number; enforcementLevel: EnforcementLevel; }; @@ -38,6 +40,7 @@ export type TUpdateSecretPolicyDTO = { id: string; name?: string; approvers?: string[]; + groupApprovers?: string[]; secretPath?: string | null; approvals?: number; enforcementLevel?: EnforcementLevel; diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx index b37d9322f..4b4a6a5c8 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx @@ -89,7 +89,8 @@ export const ApprovalPolicyRow = ({ { workspaceId, id: policy.id, - approvers: selectedApprovers + approvers: selectedApprovers, + groupApprovers: selectedGroupApprovers }, { onSettled: () => { } } ); @@ -164,6 +165,7 @@ export const ApprovalPolicyRow = ({ workspaceId, id: policy.id, approvers: selectedApprovers, + groupApprovers: selectedGroupApprovers }, { onSettled: () => { } } ); From dd9a00679d3ee814348531636de459bc2126b398 Mon Sep 17 00:00:00 2001 From: Meet Date: Fri, 20 Sep 2024 09:03:43 +0530 Subject: [PATCH 3/8] chore: fix type --- backend/src/@types/knex.d.ts | 4 ++-- .../ApprovalPolicyList/components/AccessPolicyModal.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/@types/knex.d.ts b/backend/src/@types/knex.d.ts index 81e7c6d4b..1298cfc66 100644 --- a/backend/src/@types/knex.d.ts +++ b/backend/src/@types/knex.d.ts @@ -804,12 +804,12 @@ declare module "knex/types/tables" { [TableName.AccessApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< TAccessApprovalPoliciesGroupApprovers, TAccessApprovalPoliciesGroupApproversInsert, - TAccessApprovalPoliciesGroupApproversInsert + TAccessApprovalPoliciesGroupApproversUpdate >; [TableName.SecretApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< TSecretApprovalPoliciesGroupApprovers, TSecretApprovalPoliciesGroupApproversInsert, - TSecretApprovalPoliciesGroupApproversInsert + TSecretApprovalPoliciesGroupApproversUpdate >; } } diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx index a4218e582..e83f85324 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx @@ -77,7 +77,7 @@ export const AccessPolicyForm = ({ ...editValues, environment: editValues.environment.slug, approvers: editValues?.userApprovers?.map((user) => user.userId) || editValues?.approvers, - groupApprovers: editValues?.groupApprovers?.map((group) => group.groupId) || editValues?.groupApprovers, + groupApprovers: editValues?.groupApprovers?.map((group) => group.groupId), approvals: editValues?.approvals } : undefined From 12ecefa832c4c0adde10c28f2fd0a1cb0c122c09 Mon Sep 17 00:00:00 2001 From: Meet Date: Fri, 20 Sep 2024 09:31:18 +0530 Subject: [PATCH 4/8] chore: remove logs --- .../access-approval-policy-service.ts | 22 ------------------- 1 file changed, 22 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 0e75416dd..b0fe698a8 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,7 +3,6 @@ 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 { logger } from "@app/lib/logger"; 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"; @@ -99,17 +98,6 @@ export const accessApprovalPolicyServiceFactory = ({ const verifyGroupApprovers = (await Promise.all(usersPromises)).flat().map((user) => user.id); verifyAllApprovers.push(...verifyGroupApprovers); - logger.info("verifyApproversCreate"); - logger.info({ - projectId: project.id, - orgId: actorOrgId, - envSlug: environment, - secretPath, - actorAuthMethod, - permissionService, - userIds: verifyAllApprovers - }); - await verifyApprovers({ projectId: project.id, orgId: actorOrgId, @@ -213,16 +201,6 @@ export const accessApprovalPolicyServiceFactory = ({ tx ); if (approvers) { - logger.info("verifyApproversPatch"); - logger.info({ - projectId: accessApprovalPolicy.projectId, - orgId: actorOrgId, - envSlug: accessApprovalPolicy.environment.slug, - secretPath, - actorAuthMethod, - permissionService, - userIds: approvers - }); await verifyApprovers({ projectId: accessApprovalPolicy.projectId, orgId: actorOrgId, From e150673de48ad0e56f4d4c1f3b66c5946db18c5d Mon Sep 17 00:00:00 2001 From: Meet Date: Mon, 23 Sep 2024 10:26:58 +0530 Subject: [PATCH 5/8] chore: Refactor and remove new tables --- backend/src/@types/knex.d.ts | 11 -- .../20240918005344_add-group-approvals.ts | 36 +++++-- ...40919205505_add-group-approvals-secrets.ts | 22 ---- .../access-approval-policies-approvers.ts | 3 +- ...ccess-approval-policies-group-approvers.ts | 25 ----- backend/src/db/schemas/index.ts | 2 - backend/src/db/schemas/models.ts | 2 - .../secret-approval-policies-approvers.ts | 3 +- ...ecret-approval-policies-group-approvers.ts | 25 ----- .../v1/access-approval-policy-router.ts | 79 ++++++-------- .../v1/secret-approval-policy-router.ts | 83 +++++++------- .../v1/secret-approval-request-router.ts | 6 +- .../access-approval-policy-dal.ts | 41 +++---- ...cess-approval-policy-group-approver-dal.ts | 12 --- .../access-approval-policy-service.ts | 77 +++++++------ .../access-approval-policy-types.ts | 11 +- .../access-approval-request-dal.ts | 16 +-- .../access-approval-request-service.ts | 21 ++-- .../secret-approval-policy-dal.ts | 93 ++++++++++------ ...cret-approval-policy-group-approver-dal.ts | 12 --- .../secret-approval-policy-service.ts | 35 +++--- .../secret-approval-policy-types.ts | 8 +- .../secret-approval-request-dal.ts | 39 +++---- .../secret-approval-request-service.ts | 6 +- backend/src/server/routes/index.ts | 7 -- .../src/hooks/api/accessApproval/mutation.tsx | 5 +- .../src/hooks/api/accessApproval/types.ts | 20 ++-- .../src/hooks/api/secretApproval/mutation.tsx | 5 +- .../src/hooks/api/secretApproval/types.ts | 19 ++-- .../components/AccessPolicyModal.tsx | 27 +++-- .../components/ApprovalPolicyRow.tsx | 101 +++++++++--------- 31 files changed, 386 insertions(+), 466 deletions(-) delete mode 100644 backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts delete mode 100644 backend/src/db/schemas/access-approval-policies-group-approvers.ts delete mode 100644 backend/src/db/schemas/secret-approval-policies-group-approvers.ts delete mode 100644 backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts delete mode 100644 backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts diff --git a/backend/src/@types/knex.d.ts b/backend/src/@types/knex.d.ts index 1298cfc66..7cf86032f 100644 --- a/backend/src/@types/knex.d.ts +++ b/backend/src/@types/knex.d.ts @@ -6,7 +6,6 @@ import { TAccessApprovalPoliciesApprovers, TAccessApprovalPoliciesApproversInsert, TAccessApprovalPoliciesApproversUpdate, - TAccessApprovalPoliciesGroupApprovers, TAccessApprovalPoliciesInsert, TAccessApprovalPoliciesUpdate, TAccessApprovalRequests, @@ -801,15 +800,5 @@ declare module "knex/types/tables" { TWorkflowIntegrationsInsert, TWorkflowIntegrationsUpdate >; - [TableName.AccessApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< - TAccessApprovalPoliciesGroupApprovers, - TAccessApprovalPoliciesGroupApproversInsert, - TAccessApprovalPoliciesGroupApproversUpdate - >; - [TableName.SecretApprovalPolicyGroupApprover]: KnexOriginal.CompositeTableType< - TSecretApprovalPoliciesGroupApprovers, - TSecretApprovalPoliciesGroupApproversInsert, - TSecretApprovalPoliciesGroupApproversUpdate - >; } } diff --git a/backend/src/db/migrations/20240918005344_add-group-approvals.ts b/backend/src/db/migrations/20240918005344_add-group-approvals.ts index aeddbd9cb..ed4534eb5 100644 --- a/backend/src/db/migrations/20240918005344_add-group-approvals.ts +++ b/backend/src/db/migrations/20240918005344_add-group-approvals.ts @@ -1,22 +1,36 @@ import { Knex } from "knex"; import { TableName } from "../schemas"; -import { createOnUpdateTrigger } from "../utils"; export async function up(knex: Knex): Promise { - if (!(await knex.schema.hasTable(TableName.AccessApprovalPolicyGroupApprover))) { - await knex.schema.createTable(TableName.AccessApprovalPolicyGroupApprover, (t) => { - t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); - t.uuid("approverGroupId").notNullable(); - t.foreign("approverGroupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); - t.uuid("policyId").notNullable(); - t.foreign("policyId").references("id").inTable(TableName.AccessApprovalPolicy).onDelete("CASCADE"); - t.timestamps(true, true, true); + if (await knex.schema.hasTable(TableName.AccessApprovalPolicyApprover)) { + // add column approverGroupId to AccessApprovalPolicyApprover + await knex.schema.alterTable(TableName.AccessApprovalPolicyApprover, (table) => { + // make nullable + table.uuid("approverGroupId").nullable().references("id").inTable(TableName.Groups).onDelete("CASCADE"); + // make approverUserId nullable + table.uuid("approverUserId").nullable().alter(); + }); + // add column approverGroupId to SecretApprovalPolicyApprover + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (table) => { + table.uuid("approverGroupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); + table.uuid("approverUserId").nullable().alter(); }); - await createOnUpdateTrigger(knex, TableName.AccessApprovalPolicyGroupApprover); } } export async function down(knex: Knex): Promise { - await knex.schema.dropTableIfExists(TableName.AccessApprovalPolicyGroupApprover); + if (await knex.schema.hasTable(TableName.AccessApprovalPolicyApprover)) { + // remove + await knex.schema.alterTable(TableName.AccessApprovalPolicyApprover, (table) => { + table.dropColumn("approverGroupId"); + table.uuid("approverUserId").notNullable().alter(); + }); + + // remove + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (table) => { + table.dropColumn("approverGroupId"); + table.uuid("approverUserId").notNullable().alter(); + }); + } } diff --git a/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts b/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts deleted file mode 100644 index 8a665fc2c..000000000 --- a/backend/src/db/migrations/20240919205505_add-group-approvals-secrets.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Knex } from "knex"; - -import { TableName } from "../schemas"; -import { createOnUpdateTrigger } from "../utils"; - -export async function up(knex: Knex): Promise { - if (!(await knex.schema.hasTable(TableName.SecretApprovalPolicyGroupApprover))) { - await knex.schema.createTable(TableName.SecretApprovalPolicyGroupApprover, (t) => { - t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); - t.uuid("approverGroupId").notNullable(); - t.foreign("approverGroupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); - t.uuid("policyId").notNullable(); - t.foreign("policyId").references("id").inTable(TableName.SecretApprovalPolicy).onDelete("CASCADE"); - t.timestamps(true, true, true); - }); - await createOnUpdateTrigger(knex, TableName.SecretApprovalPolicyGroupApprover); - } -} - -export async function down(knex: Knex): Promise { - await knex.schema.dropTableIfExists(TableName.SecretApprovalPolicyGroupApprover); -} diff --git a/backend/src/db/schemas/access-approval-policies-approvers.ts b/backend/src/db/schemas/access-approval-policies-approvers.ts index 8795c486e..1ecd80513 100644 --- a/backend/src/db/schemas/access-approval-policies-approvers.ts +++ b/backend/src/db/schemas/access-approval-policies-approvers.ts @@ -12,7 +12,8 @@ export const AccessApprovalPoliciesApproversSchema = z.object({ policyId: z.string().uuid(), createdAt: z.date(), updatedAt: z.date(), - approverUserId: z.string().uuid() + approverUserId: z.string().uuid().nullable().optional(), + approverGroupId: z.string().uuid().nullable().optional() }); export type TAccessApprovalPoliciesApprovers = z.infer; diff --git a/backend/src/db/schemas/access-approval-policies-group-approvers.ts b/backend/src/db/schemas/access-approval-policies-group-approvers.ts deleted file mode 100644 index 7092629f6..000000000 --- a/backend/src/db/schemas/access-approval-policies-group-approvers.ts +++ /dev/null @@ -1,25 +0,0 @@ -// Code generated by automation script, DO NOT EDIT. -// Automated by pulling database and generating zod schema -// To update. Just run npm run generate:schema -// Written by akhilmhdh. - -import { z } from "zod"; - -import { TImmutableDBKeys } from "./models"; - -export const AccessApprovalPoliciesGroupApproversSchema = z.object({ - id: z.string().uuid(), - approverGroupId: z.string().uuid(), - policyId: z.string().uuid(), - createdAt: z.date(), - updatedAt: z.date() -}); - -export type TAccessApprovalPoliciesGroupApprovers = z.infer; -export type TAccessApprovalPoliciesGroupApproversInsert = Omit< - z.input, - TImmutableDBKeys ->; -export type TAccessApprovalPoliciesGroupApproversUpdate = Partial< - Omit, TImmutableDBKeys> ->; diff --git a/backend/src/db/schemas/index.ts b/backend/src/db/schemas/index.ts index 1a3978532..d856cab49 100644 --- a/backend/src/db/schemas/index.ts +++ b/backend/src/db/schemas/index.ts @@ -1,6 +1,5 @@ export * from "./access-approval-policies"; export * from "./access-approval-policies-approvers"; -export * from "./access-approval-policies-group-approvers"; export * from "./access-approval-requests"; export * from "./access-approval-requests-reviewers"; export * from "./api-keys"; @@ -72,7 +71,6 @@ export * from "./saml-configs"; export * from "./scim-tokens"; export * from "./secret-approval-policies"; export * from "./secret-approval-policies-approvers"; -export * from "./secret-approval-policies-group-approvers"; export * from "./secret-approval-request-secret-tags"; export * from "./secret-approval-request-secret-tags-v2"; export * from "./secret-approval-requests"; diff --git a/backend/src/db/schemas/models.ts b/backend/src/db/schemas/models.ts index b31c0dba6..068ef74ad 100644 --- a/backend/src/db/schemas/models.ts +++ b/backend/src/db/schemas/models.ts @@ -73,12 +73,10 @@ export enum TableName { ScimToken = "scim_tokens", AccessApprovalPolicy = "access_approval_policies", AccessApprovalPolicyApprover = "access_approval_policies_approvers", - AccessApprovalPolicyGroupApprover = "access_approval_policies_group_approvers", AccessApprovalRequest = "access_approval_requests", AccessApprovalRequestReviewer = "access_approval_requests_reviewers", SecretApprovalPolicy = "secret_approval_policies", SecretApprovalPolicyApprover = "secret_approval_policies_approvers", - SecretApprovalPolicyGroupApprover = "secret_approval_policies_group_approvers", SecretApprovalRequest = "secret_approval_requests", SecretApprovalRequestReviewer = "secret_approval_requests_reviewers", SecretApprovalRequestSecret = "secret_approval_requests_secrets", diff --git a/backend/src/db/schemas/secret-approval-policies-approvers.ts b/backend/src/db/schemas/secret-approval-policies-approvers.ts index 3af5614f4..f9aebf019 100644 --- a/backend/src/db/schemas/secret-approval-policies-approvers.ts +++ b/backend/src/db/schemas/secret-approval-policies-approvers.ts @@ -12,7 +12,8 @@ export const SecretApprovalPoliciesApproversSchema = z.object({ policyId: z.string().uuid(), createdAt: z.date(), updatedAt: z.date(), - approverUserId: z.string().uuid() + approverUserId: z.string().uuid().nullable().optional(), + approverGroupId: z.string().uuid().nullable().optional() }); export type TSecretApprovalPoliciesApprovers = z.infer; diff --git a/backend/src/db/schemas/secret-approval-policies-group-approvers.ts b/backend/src/db/schemas/secret-approval-policies-group-approvers.ts deleted file mode 100644 index 6bfec26d2..000000000 --- a/backend/src/db/schemas/secret-approval-policies-group-approvers.ts +++ /dev/null @@ -1,25 +0,0 @@ -// Code generated by automation script, DO NOT EDIT. -// Automated by pulling database and generating zod schema -// To update. Just run npm run generate:schema -// Written by akhilmhdh. - -import { z } from "zod"; - -import { TImmutableDBKeys } from "./models"; - -export const SecretApprovalPoliciesGroupApproversSchema = z.object({ - id: z.string().uuid(), - approverGroupId: z.string().uuid(), - policyId: z.string().uuid(), - createdAt: z.date(), - updatedAt: z.date() -}); - -export type TSecretApprovalPoliciesGroupApprovers = z.infer; -export type TSecretApprovalPoliciesGroupApproversInsert = Omit< - z.input, - TImmutableDBKeys ->; -export type TSecretApprovalPoliciesGroupApproversUpdate = Partial< - Omit, TImmutableDBKeys> ->; diff --git a/backend/src/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index 4a0bc8983..514f01d25 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -1,6 +1,7 @@ import { nanoid } from "nanoid"; import { z } from "zod"; +import { ApproverType } from "@app/ee/services/access-approval-policy/access-approval-policy-types"; import { EnforcementLevel } from "@app/lib/types"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { sapPubSchema } from "@app/server/routes/sanitizedSchemas"; @@ -11,21 +12,18 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi url: "/", method: "POST", schema: { - body: z - .object({ - projectSlug: z.string().trim(), - name: z.string().optional(), - secretPath: z.string().trim().default("/"), - environment: z.string(), - approvers: z.string().array().default([]), - groupApprovers: z.string().array().default([]), - approvals: z.number().min(1).default(1), - enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) - }) - .refine((data) => data.approvers.length > 0 || data.groupApprovers.length > 0, { - path: ["approvers", "groupApprovers"], - message: "At least one approver should be provided." - }), + body: z.object({ + projectSlug: z.string().trim(), + name: z.string().optional(), + secretPath: z.string().trim().default("/"), + environment: z.string(), + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .array() + .min(1), + approvals: z.number().min(1).default(1), + enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) + }), response: { 200: z.object({ approval: sapPubSchema @@ -59,19 +57,15 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi 200: z.object({ approvals: sapPubSchema .extend({ - userApprovers: z - .object({ - userId: z.string() - }) - .array(), - groupApprovers: z - .object({ - groupId: z.string() - }) - .array(), - secretPath: z.string().optional().nullable() + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string().nullable().optional() }) + .array() + .nullable() + .optional() }) .array() + .nullable() + .optional() }) } }, @@ -84,7 +78,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi actorOrgId: req.permission.orgId, projectSlug: req.query.projectSlug }); - + return { approvals }; } }); @@ -125,23 +119,20 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi params: z.object({ policyId: z.string() }), - body: z - .object({ - name: z.string().optional(), - secretPath: z - .string() - .trim() - .optional() - .transform((val) => (val === "" ? "/" : val)), - approvers: z.string().array().optional().default([]), - approvals: z.number().min(1).optional(), - groupApprovers: z.string().array().optional().default([]), - enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) - }) - .refine((data) => data.approvers || data.groupApprovers, { - path: ["approvers", "groupApprovers"], - message: "At least one approver should be provided." - }), + body: z.object({ + name: z.string().optional(), + secretPath: z + .string() + .trim() + .optional() + .transform((val) => (val === "" ? "/" : val)), + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .array() + .min(1), + approvals: z.number().min(1).optional(), + enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) + }), response: { 200: z.object({ approval: sapPubSchema diff --git a/backend/src/ee/routes/v1/secret-approval-policy-router.ts b/backend/src/ee/routes/v1/secret-approval-policy-router.ts index 8150fb5a5..34a7dc779 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -1,6 +1,7 @@ import { nanoid } from "nanoid"; import { z } from "zod"; +import { ApproverType } from "@app/ee/services/access-approval-policy/access-approval-policy-types"; import { removeTrailingSlash } from "@app/lib/fn"; import { EnforcementLevel } from "@app/lib/types"; import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; @@ -16,26 +17,23 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi rateLimit: writeLimit }, schema: { - body: z - .object({ - workspaceId: z.string(), - name: z.string().optional(), - environment: z.string(), - secretPath: z - .string() - .optional() - .nullable() - .default("/") - .transform((val) => (val ? removeTrailingSlash(val) : val)), - approvers: z.string().array().optional().default([]), - groupApprovers: z.string().array().optional().default([]), - approvals: z.number().min(1).default(1), - enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) - }) - .refine((data) => data.approvers || data.groupApprovers, { - path: ["approvers", "groupApprovers"], - message: "At least one approver should be provided." - }), + body: z.object({ + workspaceId: z.string(), + name: z.string().optional(), + environment: z.string(), + secretPath: z + .string() + .optional() + .nullable() + .default("/") + .transform((val) => (val ? removeTrailingSlash(val) : val)), + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .array() + .min(1), + approvals: z.number().min(1).default(1), + enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) + }), response: { 200: z.object({ approval: sapPubSchema @@ -68,24 +66,21 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi params: z.object({ sapId: z.string() }), - body: z - .object({ - name: z.string().optional(), - approvers: z.string().array().optional().default([]), - approvals: z.number().min(1).default(1), - groupApprovers: z.string().array().optional().default([]), - secretPath: z - .string() - .optional() - .nullable() - .transform((val) => (val ? removeTrailingSlash(val) : val)) - .transform((val) => (val === "" ? "/" : val)), - enforcementLevel: z.nativeEnum(EnforcementLevel).optional() - }) - .refine((data) => data.approvers.length > 0 || data.groupApprovers.length > 0, { - path: ["approvers", "groupApprovers"], - message: "At least one approver should be provided." - }), + body: z.object({ + name: z.string().optional(), + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .array() + .min(1), + approvals: z.number().min(1).default(1), + secretPath: z + .string() + .optional() + .nullable() + .transform((val) => (val ? removeTrailingSlash(val) : val)) + .transform((val) => (val === "" ? "/" : val)), + enforcementLevel: z.nativeEnum(EnforcementLevel).optional() + }), response: { 200: z.object({ approval: sapPubSchema @@ -149,14 +144,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi 200: z.object({ approvals: sapPubSchema .extend({ - userApprovers: z + approvers: z .object({ - userId: z.string() - }) - .array(), - groupApprovers: z - .object({ - groupId: z.string() + id: z.string().nullable().optional(), + type: z.nativeEnum(ApproverType) }) .array() }) @@ -193,7 +184,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi 200: z.object({ policy: sapPubSchema .extend({ - userApprovers: z.object({ userId: z.string() }).array() + userApprovers: z.object({ userId: z.string().nullable().optional() }).array() }) .optional() }) diff --git a/backend/src/ee/routes/v1/secret-approval-request-router.ts b/backend/src/ee/routes/v1/secret-approval-request-router.ts index 29287974e..5fbf784f6 100644 --- a/backend/src/ee/routes/v1/secret-approval-request-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-request-router.ts @@ -13,7 +13,7 @@ import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { secretRawSchema } from "@app/server/routes/sanitizedSchemas"; import { AuthMode } from "@app/services/auth/auth-type"; -const approvalRequestUser = z.object({ userId: z.string() }).merge( +const approvalRequestUser = z.object({ userId: z.string().nullable().optional() }).merge( UsersSchema.pick({ email: true, firstName: true, @@ -48,7 +48,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv approvals: z.number(), approvers: z .object({ - userId: z.string() + userId: z.string().nullable().optional() }) .array(), secretPath: z.string().optional().nullable(), @@ -60,7 +60,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv reviewers: z.object({ userId: z.string(), status: z.string() }).array(), approvers: z .object({ - userId: z.string() + userId: z.string().nullable().optional() }) .array() }).array() diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts index b5c854e41..51303dcfd 100644 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts @@ -5,6 +5,8 @@ import { AccessApprovalPoliciesSchema, TableName, TAccessApprovalPolicies } from import { DatabaseError } from "@app/lib/errors"; import { buildFindFilter, ormify, selectAllTableCols, sqlNestRelationships, TFindFilter } from "@app/lib/knex"; +import { ApproverType } from "./access-approval-policy-types"; + export type TAccessApprovalPolicyDALFactory = ReturnType; export const accessApprovalPolicyDALFactory = (db: TDbClient) => { @@ -20,13 +22,8 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) - .leftJoin( - TableName.AccessApprovalPolicyGroupApprover, - `${TableName.AccessApprovalPolicy}.id`, - `${TableName.AccessApprovalPolicyGroupApprover}.policyId` - ) .select(tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover)) - .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyGroupApprover)) + .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyApprover)) .select(tx.ref("name").withSchema(TableName.Environment).as("envName")) .select(tx.ref("slug").withSchema(TableName.Environment).as("envSlug")) .select(tx.ref("id").withSchema(TableName.Environment).as("envId")) @@ -36,10 +33,10 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { return result; }; - const findById = async (id: string, tx?: Knex) => { + const findById = async (policyId: string, tx?: Knex) => { try { const doc = await accessApprovalPolicyFindQuery(tx || db.replicaNode(), { - [`${TableName.AccessApprovalPolicy}.id` as "id"]: id + [`${TableName.AccessApprovalPolicy}.id` as "id"]: policyId }); const formattedDoc = sqlNestRelationships({ data: doc, @@ -56,16 +53,18 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { childrenMapper: [ { key: "approverUserId", - label: "userApprovers" as const, - mapper: ({ approverUserId }) => ({ - userId: approverUserId + label: "approvers" as const, + mapper: ({ approverUserId: id }) => ({ + id, + type: "user" }) }, { key: "approverGroupId", - label: "groupApprovers" as const, - mapper: ({ approverGroupId }) => ({ - groupId: approverGroupId + label: "approvers" as const, + mapper: ({ approverGroupId: id }) => ({ + id, + type: "group" }) } ] @@ -97,16 +96,18 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { childrenMapper: [ { key: "approverUserId", - label: "userApprovers" as const, - mapper: ({ approverUserId }) => ({ - userId: approverUserId + label: "approvers" as const, + mapper: ({ approverUserId: id }) => ({ + id, + type: ApproverType.User }) }, { key: "approverGroupId", - label: "groupApprovers" as const, - mapper: ({ approverGroupId }) => ({ - groupId: approverGroupId + label: "approvers" as const, + mapper: ({ approverGroupId: id }) => ({ + id, + type: ApproverType.Group }) } ] diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts deleted file mode 100644 index baa330543..000000000 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { TDbClient } from "@app/db"; -import { TableName } from "@app/db/schemas"; -import { ormify } from "@app/lib/knex"; - -export type TAccessApprovalPolicyGroupApproverDALFactory = ReturnType< - typeof accessApprovalPolicyGroupApproverDALFactory ->; - -export const accessApprovalPolicyGroupApproverDALFactory = (db: TDbClient) => { - const accessApprovalPolicyGroupApproverOrm = ormify(db, TableName.AccessApprovalPolicyGroupApprover); - return { ...accessApprovalPolicyGroupApproverOrm }; -}; 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 b0fe698a8..fdb242314 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 @@ -11,8 +11,8 @@ 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 { TAccessApprovalPolicyGroupApproverDALFactory } from "./access-approval-policy-group-approver-dal"; import { + ApproverType, TCreateAccessApprovalPolicy, TDeleteAccessApprovalPolicy, TGetAccessPolicyCountByEnvironmentDTO, @@ -28,14 +28,12 @@ type TSecretApprovalPolicyServiceFactoryDep = { accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory; projectMembershipDAL: Pick; groupDAL: TGroupDALFactory; - accessApprovalPolicyGroupApproverDAL: TAccessApprovalPolicyGroupApproverDALFactory; }; export type TAccessApprovalPolicyServiceFactory = ReturnType; export const accessApprovalPolicyServiceFactory = ({ accessApprovalPolicyDAL, - accessApprovalPolicyGroupApproverDAL, accessApprovalPolicyApproverDAL, groupDAL, permissionService, @@ -51,7 +49,6 @@ export const accessApprovalPolicyServiceFactory = ({ actorAuthMethod, approvals, approvers, - groupApprovers, projectSlug, environment, enforcementLevel @@ -59,11 +56,15 @@ export const accessApprovalPolicyServiceFactory = ({ const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId); if (!project) throw new BadRequestError({ message: "Project not found" }); - if (!groupApprovers && !approvers) - throw new BadRequestError({ message: "Either of approvers or group approvers must be provided" }); - // If there is a group approver people might be added to the group later to meet the approvers quota - if (!groupApprovers && approvals > approvers.length) + const groupApprovers = approvers + .filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id); + const userApprovers = approvers + .filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id); + + if (!groupApprovers && approvals > userApprovers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -80,7 +81,7 @@ export const accessApprovalPolicyServiceFactory = ({ const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id }); if (!env) throw new BadRequestError({ message: "Environment not found" }); - const verifyAllApprovers = approvers; + const verifyAllApprovers = userApprovers; const usersPromises: Promise< { id: string; @@ -119,21 +120,25 @@ export const accessApprovalPolicyServiceFactory = ({ }, tx ); - await accessApprovalPolicyApproverDAL.insertMany( - approvers.map((userId) => ({ - approverUserId: userId, - policyId: doc.id - })), - tx - ); + if (userApprovers) { + await accessApprovalPolicyApproverDAL.insertMany( + userApprovers.map((userId) => ({ + approverUserId: userId, + policyId: doc.id + })), + tx + ); + } - await accessApprovalPolicyGroupApproverDAL.insertMany( - groupApprovers.map((groupId) => ({ - approverGroupId: groupId, - policyId: doc.id - })), - tx - ); + if (groupApprovers) { + await accessApprovalPolicyApproverDAL.insertMany( + groupApprovers.map((groupId) => ({ + approverGroupId: groupId, + policyId: doc.id + })), + tx + ); + } return doc; }); @@ -167,7 +172,6 @@ export const accessApprovalPolicyServiceFactory = ({ const updateAccessApprovalPolicy = async ({ policyId, approvers, - groupApprovers, secretPath, name, actorId, @@ -177,7 +181,19 @@ export const accessApprovalPolicyServiceFactory = ({ approvals, enforcementLevel }: TUpdateAccessApprovalPolicy) => { + const groupApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id); + const userApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id); + const accessApprovalPolicy = await accessApprovalPolicyDAL.findById(policyId); + const currentAppovals = approvals || accessApprovalPolicy.approvals; + if (groupApprovers?.length === 0 && userApprovers && currentAppovals > userApprovers.length) { + throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); + } + if (!accessApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" }); const { permission } = await permissionService.getProjectPermission( actor, @@ -200,7 +216,10 @@ export const accessApprovalPolicyServiceFactory = ({ }, tx ); - if (approvers) { + + await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); + + if (userApprovers) { await verifyApprovers({ projectId: accessApprovalPolicy.projectId, orgId: actorOrgId, @@ -208,11 +227,10 @@ export const accessApprovalPolicyServiceFactory = ({ secretPath: doc.secretPath!, actorAuthMethod, permissionService, - userIds: approvers + userIds: userApprovers }); - await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); await accessApprovalPolicyApproverDAL.insertMany( - approvers.map((userId) => ({ + userApprovers.map((userId) => ({ approverUserId: userId, policyId: doc.id })), @@ -246,8 +264,7 @@ export const accessApprovalPolicyServiceFactory = ({ permissionService, userIds: verifyGroupApprovers }); - await accessApprovalPolicyGroupApproverDAL.delete({ policyId: doc.id }, tx); - await accessApprovalPolicyGroupApproverDAL.insertMany( + await accessApprovalPolicyApproverDAL.insertMany( groupApprovers.map((groupId) => ({ approverGroupId: groupId, policyId: doc.id diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts b/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts index 47d7ead9e..c20aabfad 100644 --- a/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts +++ b/backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts @@ -13,12 +13,16 @@ export type TVerifyApprovers = { orgId: string; }; +export enum ApproverType { + Group = "group", + User = "user" +} + export type TCreateAccessApprovalPolicy = { approvals: number; secretPath: string; environment: string; - approvers: string[]; - groupApprovers: string[]; + approvers: { type: ApproverType; id: string }[]; projectSlug: string; name: string; enforcementLevel: EnforcementLevel; @@ -27,8 +31,7 @@ export type TCreateAccessApprovalPolicy = { export type TUpdateAccessApprovalPolicy = { policyId: string; approvals?: number; - approvers?: string[]; - groupApprovers?: string[]; + approvers?: { type: ApproverType; id: string }[]; secretPath?: string; name?: string; enforcementLevel?: EnforcementLevel; diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts index 5746ecb59..8784d05e2 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts @@ -39,14 +39,9 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) - .leftJoin( - TableName.AccessApprovalPolicyGroupApprover, - `${TableName.AccessApprovalPolicy}.id`, - `${TableName.AccessApprovalPolicyGroupApprover}.policyId` - ) .leftJoin( TableName.UserGroupMembership, - `${TableName.AccessApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.AccessApprovalPolicyApprover}.approverGroupId`, `${TableName.UserGroupMembership}.groupId` ) .leftJoin(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`) @@ -200,16 +195,9 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicyApprover}.approverUserId`, "accessApprovalPolicyApproverUser.id" ) - - .leftJoin( - TableName.AccessApprovalPolicyGroupApprover, - `${TableName.AccessApprovalPolicy}.id`, - `${TableName.AccessApprovalPolicyGroupApprover}.policyId` - ) - .leftJoin( TableName.UserGroupMembership, - `${TableName.AccessApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.AccessApprovalPolicyApprover}.approverGroupId`, `${TableName.UserGroupMembership}.groupId` ) diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts index 108e4607e..684a2f77c 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts @@ -18,7 +18,6 @@ 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 { TAccessApprovalPolicyGroupApproverDALFactory } from "../access-approval-policy/access-approval-policy-group-approver-dal"; import { TGroupDALFactory } from "../group/group-dal"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { TProjectUserAdditionalPrivilegeDALFactory } from "../project-user-additional-privilege/project-user-additional-privilege-dal"; @@ -38,7 +37,6 @@ type TSecretApprovalRequestServiceFactoryDep = { additionalPrivilegeDAL: Pick; permissionService: Pick; accessApprovalPolicyApproverDAL: Pick; - accessApprovalPolicyGroupApproverDAL: Pick; projectEnvDAL: Pick; projectDAL: Pick< TProjectDALFactory, @@ -83,7 +81,6 @@ export const accessApprovalRequestServiceFactory = ({ projectMembershipDAL, accessApprovalPolicyDAL, accessApprovalPolicyApproverDAL, - accessApprovalPolicyGroupApproverDAL, additionalPrivilegeDAL, smtpService, userDAL, @@ -130,26 +127,28 @@ export const accessApprovalRequestServiceFactory = ({ }); if (!policy) throw new UnauthorizedError({ message: "No policy matching criteria was found." }); - const approverIds = []; + const approverIds: string[] = []; + const approverGroupIds: string[] = []; const approvers = await accessApprovalPolicyApproverDAL.find({ policyId: policy.id }); approvers.forEach((approver) => { - approverIds.push(approver.approverUserId); - }); - - const groupApprovers = await accessApprovalPolicyGroupApproverDAL.find({ - policyId: policy.id + if (approver.approverUserId) { + approverIds.push(approver.approverUserId); + } + else if (approver.approverGroupId) { + approverGroupIds.push(approver.approverGroupId); + } }); const groupUsers = ( await Promise.all( - groupApprovers.map((groupApprover) => + approverGroupIds.map((groupApproverId) => groupDAL.findAllGroupMembers({ orgId: actorOrgId, - groupId: groupApprover.id + groupId: groupApproverId }) ) ) diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts index 51dba2efc..651e19b64 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts @@ -1,10 +1,12 @@ import { Knex } from "knex"; import { TDbClient } from "@app/db"; -import { SecretApprovalPoliciesSchema, TableName, TSecretApprovalPolicies } from "@app/db/schemas"; +import { SecretApprovalPoliciesSchema, TableName, TSecretApprovalPolicies, TUsers } from "@app/db/schemas"; import { DatabaseError } from "@app/lib/errors"; import { buildFindFilter, ormify, selectAllTableCols, sqlNestRelationships, TFindFilter } from "@app/lib/knex"; +import { ApproverType } from "../access-approval-policy/access-approval-policy-types"; + export type TSecretApprovalPolicyDALFactory = ReturnType; export const secretApprovalPolicyDALFactory = (db: TDbClient) => { @@ -20,26 +22,25 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - - .leftJoin(TableName.Users, `${TableName.SecretApprovalPolicyApprover}.approverUserId`, `${TableName.Users}.id`) - .leftJoin( - TableName.SecretApprovalPolicyGroupApprover, - `${TableName.SecretApprovalPolicy}.id`, - `${TableName.SecretApprovalPolicyGroupApprover}.policyId` - ) .leftJoin( TableName.UserGroupMembership, - `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, - `${TableName.UserGroupMembership}.userId` + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + .leftJoin( + db(TableName.Users).as("secretApprovalPolicyApproverUser"), + `${TableName.SecretApprovalPolicyApprover}.approverUserId`, + "secretApprovalPolicyApproverUser.id" + ) + .leftJoin(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`) + .select( + tx.ref("id").withSchema("secretApprovalPolicyApproverUser").as("approverUserId"), + tx.ref("email").withSchema("secretApprovalPolicyApproverUser").as("approverEmail"), + tx.ref("firstName").withSchema("secretApprovalPolicyApproverUser").as("approverFirstName"), + tx.ref("lastName").withSchema("secretApprovalPolicyApproverUser").as("approverLastName") ) .select( - tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), - tx.ref("email").withSchema(TableName.Users).as("approverEmail"), - tx.ref("firstName").withSchema(TableName.Users).as("approverFirstName"), - tx.ref("lastName").withSchema(TableName.Users).as("approverLastName") - ) - .select( - tx.ref("approverGroupId").withSchema(TableName.SecretApprovalPolicyGroupApprover), + tx.ref("approverGroupId").withSchema(TableName.SecretApprovalPolicyApprover), tx.ref("userId").withSchema(TableName.UserGroupMembership).as("approverGroupUserId"), tx.ref("email").withSchema(TableName.Users).as("approverGroupEmail"), tx.ref("firstName").withSchema(TableName.Users).as("approverGroupFirstName"), @@ -71,21 +72,31 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "userApprovers" as const, - mapper: ({ approverUserId, approverEmail, approverFirstName, approverLastName }) => ({ - userId: approverUserId, - email: approverEmail, - firstName: approverFirstName, - lastName: approverLastName + mapper: ({ + approverUserId: userId, + approverEmail: email, + approverFirstName: firstName, + approverLastName: lastName + }) => ({ + userId, + email, + firstName, + lastName }) }, { key: "approverGroupUserId", label: "userApprovers" as const, - mapper: ({ approverGroupUserId, approverGroupEmail, approverGroupFirstName, approverGroupLastName }) => ({ - userId: approverGroupUserId, - email: approverGroupEmail, - firstName: approverGroupFirstName, - lastName: approverGroupLastName + mapper: ({ + approverGroupUserId: userId, + approverGroupEmail: email, + approverGroupFirstName: firstName, + approverGroupLastName: lastName + }) => ({ + userId, + email, + firstName, + lastName }) } ] @@ -111,16 +122,32 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { childrenMapper: [ { key: "approverUserId", - label: "userApprovers" as const, - mapper: ({ approverUserId }) => ({ - userId: approverUserId + label: "approvers" as const, + mapper: ({ approverUserId: id }) => ({ + type: ApproverType.User, + id }) }, { key: "approverGroupId", - label: "groupApprovers" as const, - mapper: ({ approverGroupId }) => ({ - groupId: approverGroupId + label: "approvers" as const, + mapper: ({ approverGroupId: id }) => ({ + type: ApproverType.Group, + id + }) + }, + { + key: "approverUserId", + label: "userApprovers" as const, + mapper: ({ approverUserId: userId }) => ({ + userId + }) + }, + { + key: "approverGroupUserId", + label: "userApprovers" as const, + mapper: ({ approverGroupUserId: userId }) => ({ + userId }) } ] diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts deleted file mode 100644 index 710d251d5..000000000 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { TDbClient } from "@app/db"; -import { TableName } from "@app/db/schemas"; -import { ormify } from "@app/lib/knex"; - -export type TSecretApprovalPolicyGroupApproverDALFactory = ReturnType< - typeof secretApprovalPolicyGroupApproverDALFactory ->; - -export const secretApprovalPolicyGroupApproverDALFactory = (db: TDbClient) => { - const sapGroupApproverOrm = ormify(db, TableName.SecretApprovalPolicyGroupApprover); - return sapGroupApproverOrm; -}; diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts index b9a385673..c1f7cce31 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts @@ -8,10 +8,10 @@ import { removeTrailingSlash } from "@app/lib/fn"; import { containsGlobPatterns } from "@app/lib/picomatch"; import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal"; +import { ApproverType } from "../access-approval-policy/access-approval-policy-types"; import { TLicenseServiceFactory } from "../license/license-service"; import { TSecretApprovalPolicyApproverDALFactory } from "./secret-approval-policy-approver-dal"; import { TSecretApprovalPolicyDALFactory } from "./secret-approval-policy-dal"; -import { TSecretApprovalPolicyGroupApproverDALFactory } from "./secret-approval-policy-group-approver-dal"; import { TCreateSapDTO, TDeleteSapDTO, @@ -30,7 +30,6 @@ type TSecretApprovalPolicyServiceFactoryDep = { secretApprovalPolicyDAL: TSecretApprovalPolicyDALFactory; projectEnvDAL: Pick; secretApprovalPolicyApproverDAL: TSecretApprovalPolicyApproverDALFactory; - secretApprovalPolicyGroupApproverDAL: TSecretApprovalPolicyGroupApproverDALFactory; licenseService: Pick; }; @@ -40,7 +39,6 @@ export const secretApprovalPolicyServiceFactory = ({ secretApprovalPolicyDAL, permissionService, secretApprovalPolicyApproverDAL, - secretApprovalPolicyGroupApproverDAL, projectEnvDAL, licenseService }: TSecretApprovalPolicyServiceFactoryDep) => { @@ -52,14 +50,17 @@ export const secretApprovalPolicyServiceFactory = ({ actorAuthMethod, approvals, approvers, - groupApprovers, projectId, secretPath, environment, enforcementLevel }: TCreateSapDTO) => { - if (!groupApprovers && !approvers) - throw new BadRequestError({ message: "Either of approvers or group approvers must be provided" }); + const groupApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id); + const userApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id); if (!groupApprovers && approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); @@ -100,14 +101,14 @@ export const secretApprovalPolicyServiceFactory = ({ ); await secretApprovalPolicyApproverDAL.insertMany( - approvers.map((approverUserId) => ({ + userApprovers.map((approverUserId) => ({ approverUserId, policyId: doc.id })), tx ); - await secretApprovalPolicyGroupApproverDAL.insertMany( + await secretApprovalPolicyApproverDAL.insertMany( groupApprovers.map((approverGroupId) => ({ approverGroupId, policyId: doc.id @@ -121,7 +122,6 @@ export const secretApprovalPolicyServiceFactory = ({ const updateSecretApprovalPolicy = async ({ approvers, - groupApprovers, secretPath, name, actorId, @@ -132,6 +132,13 @@ export const secretApprovalPolicyServiceFactory = ({ secretPolicyId, enforcementLevel }: TUpdateSapDTO) => { + const groupApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id); + const userApprovers = approvers + ?.filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id); + const secretApprovalPolicy = await secretApprovalPolicyDAL.findById(secretPolicyId); if (!secretApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" }); @@ -163,19 +170,21 @@ export const secretApprovalPolicyServiceFactory = ({ }, tx ); + + await secretApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); + if (approvers) { - await secretApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); await secretApprovalPolicyApproverDAL.insertMany( - approvers.map((approverUserId) => ({ + userApprovers.map((approverUserId) => ({ approverUserId, policyId: doc.id })), tx ); } + if (groupApprovers) { - await secretApprovalPolicyGroupApproverDAL.delete({ policyId: doc.id }, tx); - await secretApprovalPolicyGroupApproverDAL.insertMany( + await secretApprovalPolicyApproverDAL.insertMany( groupApprovers.map((approverGroupId) => ({ approverGroupId, policyId: doc.id diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts index 5199aff64..78b77c152 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts @@ -1,11 +1,12 @@ import { EnforcementLevel, TProjectPermission } from "@app/lib/types"; +import { ApproverType } from "../access-approval-policy/access-approval-policy-types"; + export type TCreateSapDTO = { approvals: number; secretPath?: string | null; environment: string; - approvers: string[]; - groupApprovers: string[]; + approvers: { type: ApproverType; id: string }[]; projectId: string; name: string; enforcementLevel: EnforcementLevel; @@ -15,8 +16,7 @@ export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; secretPath?: string | null; - approvers?: string[]; - groupApprovers?: string[]; + approvers: { type: ApproverType; id: string }[]; name?: string; enforcementLevel?: EnforcementLevel; } & Omit; diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts index 75f19861c..803b9464c 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts @@ -58,14 +58,9 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicyApprover}.approverUserId`, "secretApprovalPolicyApproverUser.id" ) - .leftJoin( - TableName.SecretApprovalPolicyGroupApprover, - `${TableName.SecretApprovalPolicy}.id`, - `${TableName.SecretApprovalPolicyGroupApprover}.policyId` - ) .leftJoin( TableName.UserGroupMembership, - `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, `${TableName.UserGroupMembership}.groupId` ) .leftJoin( @@ -172,13 +167,13 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { key: "approverUserId", label: "approvers" as const, mapper: ({ - approverUserId, + approverUserId: userId, approverEmail: email, approverUsername: username, approverLastName: lastName, approverFirstName: firstName }) => ({ - userId: approverUserId, + userId, email, firstName, lastName, @@ -189,13 +184,13 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { key: "approverGroupUserId", label: "approvers" as const, mapper: ({ - approverGroupUserId, + approverGroupUserId: userId, approverGroupEmail: email, approverGroupUsername: username, approverGroupLastName: lastName, approverGroupFirstName: firstName }) => ({ - userId: approverGroupUserId, + userId, email, firstName, lastName, @@ -278,14 +273,9 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - .leftJoin( - TableName.SecretApprovalPolicyGroupApprover, - `${TableName.SecretApprovalPolicy}.id`, - `${TableName.SecretApprovalPolicyGroupApprover}.policyId` - ) .leftJoin( TableName.UserGroupMembership, - `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, `${TableName.UserGroupMembership}.groupId` ) .join( @@ -383,7 +373,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserI: userId }) => ({ userId }) + mapper: ({ approverUserId }) => ({ userId: approverUserId }) }, { key: "commitId", @@ -397,7 +387,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverGroupUserId", label: "approvers" as const, - mapper: ({ approverGroupUserId: userId }) => ({ userId }) + mapper: ({ approverGroupUserId }) => ({ userId: approverGroupUserId }) } ] }); @@ -430,14 +420,9 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - .leftJoin( - TableName.SecretApprovalPolicyGroupApprover, - `${TableName.SecretApprovalPolicy}.id`, - `${TableName.SecretApprovalPolicyGroupApprover}.policyId` - ) .leftJoin( TableName.UserGroupMembership, - `${TableName.SecretApprovalPolicyGroupApprover}.approverGroupId`, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, `${TableName.UserGroupMembership}.groupId` ) .join( @@ -535,7 +520,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId: userId }) => ({ userId }) + mapper: ({ approverUserId }) => ({ userId: approverUserId }) }, { key: "commitId", @@ -549,8 +534,8 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverGroupUserId", label: "approvers" as const, - mapper: ({ approverGroupUserId: userId }) => ({ - userId + mapper: ({ approverGroupUserId }) => ({ + userId: approverGroupUserId }) } ] diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts index 1b606f617..f9bb2ec10 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts @@ -447,8 +447,8 @@ export const secretApprovalRequestServiceFactory = ({ ); const hasMinApproval = secretApprovalRequest.policy.approvals <= - secretApprovalRequest.policy.approvers.filter( - ({ userId: approverId }) => reviewers[approverId.toString()] === ApprovalStatus.APPROVED + secretApprovalRequest.policy.approvers.filter(({ userId: approverId }) => + approverId ? reviewers[approverId.toString()] === ApprovalStatus.APPROVED : null ).length; const isSoftEnforcement = secretApprovalRequest.policy.enforcementLevel === EnforcementLevel.Soft; @@ -805,7 +805,7 @@ export const secretApprovalRequestServiceFactory = ({ const requestedByUser = await userDAL.findOne({ id: actorId }); const approverUsers = await userDAL.find({ $in: { - id: policy.approvers.map((approver: { userId: string }) => approver.userId) + id: policy.approvers.map((approver: { userId: string | null | undefined }) => approver.userId!) } }); diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index dbbcf256e..3946c841a 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -7,7 +7,6 @@ import { registerCertificateEstRouter } from "@app/ee/routes/est/certificate-est import { registerV1EERoutes } from "@app/ee/routes/v1"; import { accessApprovalPolicyApproverDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-approver-dal"; import { accessApprovalPolicyDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-dal"; -import { accessApprovalPolicyGroupApproverDALFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-group-approver-dal"; import { accessApprovalPolicyServiceFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-service"; import { accessApprovalRequestDALFactory } from "@app/ee/services/access-approval-request/access-approval-request-dal"; import { accessApprovalRequestReviewerDALFactory } from "@app/ee/services/access-approval-request/access-approval-request-reviewer-dal"; @@ -52,7 +51,6 @@ import { scimDALFactory } from "@app/ee/services/scim/scim-dal"; import { scimServiceFactory } from "@app/ee/services/scim/scim-service"; import { secretApprovalPolicyApproverDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-approver-dal"; import { secretApprovalPolicyDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-dal"; -import { secretApprovalPolicyGroupApproverDALFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-group-approver-dal"; import { secretApprovalPolicyServiceFactory } from "@app/ee/services/secret-approval-policy/secret-approval-policy-service"; import { secretApprovalRequestDALFactory } from "@app/ee/services/secret-approval-request/secret-approval-request-dal"; import { secretApprovalRequestReviewerDALFactory } from "@app/ee/services/secret-approval-request/secret-approval-request-reviewer-dal"; @@ -301,7 +299,6 @@ export const registerRoutes = async ( const accessApprovalRequestReviewerDAL = accessApprovalRequestReviewerDALFactory(db); const sapApproverDAL = secretApprovalPolicyApproverDALFactory(db); - const sapGroupApproverDAL = secretApprovalPolicyGroupApproverDALFactory(db); const secretApprovalPolicyDAL = secretApprovalPolicyDALFactory(db); const secretApprovalRequestDAL = secretApprovalRequestDALFactory(db); const secretApprovalRequestReviewerDAL = secretApprovalRequestReviewerDALFactory(db); @@ -381,7 +378,6 @@ export const registerRoutes = async ( const secretApprovalPolicyService = secretApprovalPolicyServiceFactory({ projectEnvDAL, secretApprovalPolicyApproverDAL: sapApproverDAL, - secretApprovalPolicyGroupApproverDAL: sapGroupApproverDAL, permissionService, secretApprovalPolicyDAL, licenseService @@ -630,7 +626,6 @@ export const registerRoutes = async ( const pkiAlertDAL = pkiAlertDALFactory(db); const pkiCollectionDAL = pkiCollectionDALFactory(db); const pkiCollectionItemDAL = pkiCollectionItemDALFactory(db); - const accessApprovalPolicyGroupApproverDAL = accessApprovalPolicyGroupApproverDALFactory(db); const certificateService = certificateServiceFactory({ certificateDAL, @@ -928,7 +923,6 @@ export const registerRoutes = async ( const accessApprovalPolicyService = accessApprovalPolicyServiceFactory({ accessApprovalPolicyDAL, accessApprovalPolicyApproverDAL, - accessApprovalPolicyGroupApproverDAL, groupDAL, permissionService, projectEnvDAL, @@ -950,7 +944,6 @@ export const registerRoutes = async ( accessApprovalPolicyApproverDAL, projectSlackConfigDAL, kmsService, - accessApprovalPolicyGroupApproverDAL, groupDAL }); diff --git a/frontend/src/hooks/api/accessApproval/mutation.tsx b/frontend/src/hooks/api/accessApproval/mutation.tsx index c41b633b0..9c3199a99 100644 --- a/frontend/src/hooks/api/accessApproval/mutation.tsx +++ b/frontend/src/hooks/api/accessApproval/mutation.tsx @@ -21,7 +21,6 @@ export const useCreateAccessApprovalPolicy = () => { projectSlug, approvals, approvers, - groupApprovers, name, secretPath, enforcementLevel @@ -31,7 +30,6 @@ export const useCreateAccessApprovalPolicy = () => { projectSlug, approvals, approvers, - groupApprovers, secretPath, name, enforcementLevel @@ -48,11 +46,10 @@ export const useUpdateAccessApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TUpdateAccessPolicyDTO>({ - mutationFn: async ({ id, approvers, groupApprovers, approvals, name, secretPath, enforcementLevel }) => { + mutationFn: async ({ id, approvers, approvals, name, secretPath, enforcementLevel }) => { const { data } = await apiRequest.patch(`/api/v1/access-approvals/policies/${id}`, { approvals, approvers, - groupApprovers, secretPath, name, enforcementLevel diff --git a/frontend/src/hooks/api/accessApproval/types.ts b/frontend/src/hooks/api/accessApproval/types.ts index 37772878e..6df257590 100644 --- a/frontend/src/hooks/api/accessApproval/types.ts +++ b/frontend/src/hooks/api/accessApproval/types.ts @@ -11,15 +11,23 @@ export type TAccessApprovalPolicy = { workspace: string; environment: WorkspaceEnv; projectId: string; - approvers: string[]; policyType: PolicyType; approversRequired: boolean; enforcementLevel: EnforcementLevel; updatedAt: Date; - userApprovers?: { userId: string }[]; - groupApprovers?: { groupId: string }[]; + approvers?: Approver[]; }; +export enum ApproverType{ + User = "user", + Group = "group" +} + +export type Approver ={ + id: string; + type: ApproverType; +} + export type TAccessApprovalRequest = { id: string; policyId: string; @@ -131,8 +139,7 @@ export type TCreateAccessPolicyDTO = { projectSlug: string; name?: string; environment: string; - approvers?: string[]; - groupApprovers?: string[]; + approvers?: Approver[]; approvals?: number; secretPath?: string; enforcementLevel?: EnforcementLevel; @@ -141,8 +148,7 @@ export type TCreateAccessPolicyDTO = { export type TUpdateAccessPolicyDTO = { id: string; name?: string; - approvers?: string[]; - groupApprovers?: string[]; + approvers?: Approver[]; secretPath?: string; environment?: string; approvals?: number; diff --git a/frontend/src/hooks/api/secretApproval/mutation.tsx b/frontend/src/hooks/api/secretApproval/mutation.tsx index 9a7a97571..ceebd3493 100644 --- a/frontend/src/hooks/api/secretApproval/mutation.tsx +++ b/frontend/src/hooks/api/secretApproval/mutation.tsx @@ -14,7 +14,6 @@ export const useCreateSecretApprovalPolicy = () => { workspaceId, approvals, approvers, - groupApprovers, secretPath, name, enforcementLevel @@ -24,7 +23,6 @@ export const useCreateSecretApprovalPolicy = () => { workspaceId, approvals, approvers, - groupApprovers, secretPath, name, enforcementLevel @@ -41,11 +39,10 @@ export const useUpdateSecretApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TUpdateSecretPolicyDTO>({ - mutationFn: async ({ id, approvers, groupApprovers, approvals, secretPath, name, enforcementLevel }) => { + mutationFn: async ({ id, approvers, approvals, secretPath, name, enforcementLevel }) => { const { data } = await apiRequest.patch(`/api/v1/secret-approvals/${id}`, { approvals, approvers, - groupApprovers, secretPath, name, enforcementLevel diff --git a/frontend/src/hooks/api/secretApproval/types.ts b/frontend/src/hooks/api/secretApproval/types.ts index 68ef8a99b..fcf83086c 100644 --- a/frontend/src/hooks/api/secretApproval/types.ts +++ b/frontend/src/hooks/api/secretApproval/types.ts @@ -9,12 +9,21 @@ export type TSecretApprovalPolicy = { environment: WorkspaceEnv; secretPath?: string; approvals: number; - userApprovers: { userId: string }[]; - groupApprovers: { groupId: string }[]; + approvers: Approver[]; updatedAt: Date; enforcementLevel: EnforcementLevel; }; +export enum ApproverType{ + User = "user", + Group = "group" +} + +export type Approver ={ + id: string; + type: ApproverType; +} + export type TGetSecretApprovalPoliciesDTO = { workspaceId: string; }; @@ -30,8 +39,7 @@ export type TCreateSecretPolicyDTO = { name?: string; environment: string; secretPath?: string | null; - approvers?: string[]; - groupApprovers?: string[]; + approvers?: Approver[]; approvals?: number; enforcementLevel: EnforcementLevel; }; @@ -39,8 +47,7 @@ export type TCreateSecretPolicyDTO = { export type TUpdateSecretPolicyDTO = { id: string; name?: string; - approvers?: string[]; - groupApprovers?: string[]; + approvers?: Approver[]; secretPath?: string | null; approvals?: number; enforcementLevel?: EnforcementLevel; diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx index e83f85324..f2da72603 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx @@ -27,7 +27,7 @@ import { useCreateAccessApprovalPolicy, useUpdateAccessApprovalPolicy } from "@app/hooks/api/accessApproval"; -import { TAccessApprovalPolicy } from "@app/hooks/api/accessApproval/types"; +import { ApproverType, TAccessApprovalPolicy } from "@app/hooks/api/accessApproval/types"; import { EnforcementLevel, PolicyType } from "@app/hooks/api/policies/enums"; import { TWorkspaceUser } from "@app/hooks/api/users/types"; @@ -45,13 +45,12 @@ const formSchema = z name: z.string().optional(), secretPath: z.string().optional(), approvals: z.number().min(1), - approvers: z.string().array().optional(), - groupApprovers: z.string().array().optional(), + approvers: z.object({type: z.nativeEnum(ApproverType), id: z.string()}).array().min(1).default([]), policyType: z.nativeEnum(PolicyType), enforcementLevel: z.nativeEnum(EnforcementLevel) }) - .refine((data) => data.approvers || data.groupApprovers, { - path: ["approvers", "groupApprovers"], + .refine((data) => data.approvers, { + path: ["approvers"], message: "At least one approver should be provided." }); @@ -76,8 +75,7 @@ export const AccessPolicyForm = ({ ? { ...editValues, environment: editValues.environment.slug, - approvers: editValues?.userApprovers?.map((user) => user.userId) || editValues?.approvers, - groupApprovers: editValues?.groupApprovers?.map((group) => group.groupId), + approvers: editValues?.approvers || [], approvals: editValues?.approvals } : undefined @@ -291,15 +289,15 @@ export const AccessPolicyForm = ({ {members.map(({ user }) => { const { id: userId } = user; - const isChecked = value?.includes(userId); + const isChecked = value?.filter((el: {id: string, type: ApproverType}) => el.id === userId && el.type === ApproverType.User).length > 0; return ( { evt.preventDefault(); onChange( isChecked - ? value?.filter((el: string) => el !== userId) - : [...(value || []), userId] + ? value?.filter((el: {id: string, type: ApproverType}) => el.id !== userId && el.type !== ApproverType.User) + : [...(value || []), {id:userId, type: ApproverType.User}] ); }} key={`create-policy-members-${userId}`} @@ -317,7 +315,7 @@ export const AccessPolicyForm = ({ /> ( {groups && groups.map(({ group }) => { const { id } = group; - const isChecked = value?.includes(id); + const isChecked = value?.includes({id, type: ApproverType.Group}); + return ( { evt.preventDefault(); onChange( isChecked - ? value?.filter((el: string) => el !== id) - : [...(value || []), id] + ? value?.filter((el: {id: string, type: ApproverType}) => el.id !== id && el.type !== ApproverType.Group) + : [...(value || []), {id, type: ApproverType.Group}] ); }} key={`create-policy-members-${id}`} diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx index 4b4a6a5c8..4b13029df 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx @@ -18,6 +18,7 @@ import { Badge } from "@app/components/v2/Badge"; import { ProjectPermissionActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; import { policyDetails } from "@app/helpers/policies"; import { useUpdateAccessApprovalPolicy, useUpdateSecretApprovalPolicy } from "@app/hooks/api"; +import { Approver, ApproverType } from "@app/hooks/api/accessApproval/types"; import { TGroupMembership } from "@app/hooks/api/groups/types"; import { EnforcementLevel, PolicyType } from "@app/hooks/api/policies/enums"; import { WorkspaceEnv } from "@app/hooks/api/types"; @@ -30,9 +31,7 @@ interface IPolicy { projectId?: string; secretPath?: string; approvals: number; - approvers?: string[]; - userApprovers?: { userId: string }[]; - groupApprovers?: { groupId: string }[]; + approvers?: Approver[]; updatedAt: Date; policyType: PolicyType; enforcementLevel: EnforcementLevel; @@ -57,8 +56,8 @@ export const ApprovalPolicyRow = ({ onEdit, onDelete }: Props) => { - const [selectedApprovers, setSelectedApprovers] = useState(policy.userApprovers?.map(({ userId }) => userId) || policy.approvers || []); - const [selectedGroupApprovers, setSelectedGroupApprovers] = useState(policy.groupApprovers?.map(({ groupId }) => groupId) || []); + const [selectedApprovers, setSelectedApprovers] = useState(policy.approvers?.filter((approver) => approver.type === ApproverType.User) || []); + const [selectedGroupApprovers, setSelectedGroupApprovers] = useState(policy.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); const { mutate: updateAccessApprovalPolicy, isLoading: isAccessApprovalPolicyLoading } = useUpdateAccessApprovalPolicy(); const { mutate: updateSecretApprovalPolicy, isLoading: isSecretApprovalPolicyLoading } = useUpdateSecretApprovalPolicy(); const isLoading = isAccessApprovalPolicyLoading || isSecretApprovalPolicyLoading; @@ -79,27 +78,30 @@ export const ApprovalPolicyRow = ({ { projectSlug, id: policy.id, - approvers: selectedApprovers, - groupApprovers: selectedGroupApprovers + approvers: selectedApprovers.concat(selectedGroupApprovers), }, - { onSettled: () => { } } + { + onError: () => { + setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); + } + } ); } else { updateSecretApprovalPolicy( { workspaceId, id: policy.id, - approvers: selectedApprovers, - groupApprovers: selectedGroupApprovers + approvers: selectedApprovers.concat(selectedGroupApprovers), }, - { onSettled: () => { } } + { + onError: () => { + setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); + } + } ); } } else { - setSelectedApprovers(policy.policyType === PolicyType.ChangePolicy - ? policy?.userApprovers?.map(({ userId }) => userId) || [] - : policy?.approvers || [] - ); + setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); } }} > @@ -125,13 +127,13 @@ export const ApprovalPolicyRow = ({ {members?.map(({ user }) => { const userId = user.id; - const isChecked = selectedApprovers.includes(userId); + const isChecked = selectedApprovers?.filter((el: { id: string, type: ApproverType }) => el.id === userId && el.type === ApproverType.User).length > 0; return ( { evt.preventDefault(); setSelectedApprovers((state) => - isChecked ? state.filter((el) => el !== userId) : [...state, userId] + isChecked ? state.filter((el) => el.id !== userId || el.type !== ApproverType.User) : [...state, { id: userId, type: ApproverType.User }] ); }} key={`create-policy-members-${userId}`} @@ -147,36 +149,39 @@ export const ApprovalPolicyRow = ({ { - if (!isOpen) { - if (policy.policyType === PolicyType.AccessPolicy) { - updateAccessApprovalPolicy( - { - projectSlug, - id: policy.id, - approvers: selectedApprovers, - groupApprovers: selectedGroupApprovers - }, - { onSettled: () => { } } - ); + onOpenChange={(isOpen) => { + if (!isOpen) { + if (policy.policyType === PolicyType.AccessPolicy) { + updateAccessApprovalPolicy( + { + projectSlug, + id: policy.id, + approvers: selectedApprovers.concat(selectedGroupApprovers), + }, + { + onError: () => { + setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); + } + }, + ); + } else { + updateSecretApprovalPolicy( + { + workspaceId, + id: policy.id, + approvers: selectedApprovers.concat(selectedGroupApprovers), + }, + { + onError: () => { + setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); + } + } + ); + } } else { - updateSecretApprovalPolicy( - { - workspaceId, - id: policy.id, - approvers: selectedApprovers, - groupApprovers: selectedGroupApprovers - }, - { onSettled: () => { } } - ); + setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); } - } else { - setSelectedGroupApprovers(policy.policyType === PolicyType.ChangePolicy - ? policy?.groupApprovers?.map(({ groupId }) => groupId) || [] - : policy?.groupApprovers?.map(({groupId}) => groupId) || [] - ); - } - }} + }} > {groups && groups.map(({ group }) => { const { id } = group; - const isChecked = selectedGroupApprovers?.includes(id); + const isChecked = selectedGroupApprovers?.filter((el: { id: string, type: ApproverType }) => el.id === id && el.type === ApproverType.Group).length > 0; return ( { evt.preventDefault(); setSelectedGroupApprovers( isChecked - ? selectedGroupApprovers?.filter((el: string) => el !== id) - : [...(selectedGroupApprovers || []), id] + ? selectedGroupApprovers?.filter((el) => el.id !== id || el.type !== ApproverType.Group) + : [...(selectedGroupApprovers || []), { id, type: ApproverType.Group }] ); }} key={`create-policy-groups-${id}`} From c2252c65a4a78dd66e7777f391261d614715335d Mon Sep 17 00:00:00 2001 From: Meet Date: Mon, 23 Sep 2024 10:30:49 +0530 Subject: [PATCH 6/8] chore: lint fix --- backend/src/ee/routes/v1/access-approval-policy-router.ts | 2 +- .../access-approval-request/access-approval-request-service.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/src/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index 514f01d25..464d9df4c 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -78,7 +78,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi actorOrgId: req.permission.orgId, projectSlug: req.query.projectSlug }); - + return { approvals }; } }); diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts index 684a2f77c..005ecb228 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts @@ -137,8 +137,7 @@ export const accessApprovalRequestServiceFactory = ({ approvers.forEach((approver) => { if (approver.approverUserId) { approverIds.push(approver.approverUserId); - } - else if (approver.approverGroupId) { + } else if (approver.approverGroupId) { approverGroupIds.push(approver.approverGroupId); } }); From fcbc7fcece8b7c29455f49d77d62aa03620be2e2 Mon Sep 17 00:00:00 2001 From: Meet Date: Mon, 23 Sep 2024 10:53:58 +0530 Subject: [PATCH 7/8] chore: fix test --- backend/e2e-test/routes/v1/secret-approval-policy.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/e2e-test/routes/v1/secret-approval-policy.spec.ts b/backend/e2e-test/routes/v1/secret-approval-policy.spec.ts index 3234503e5..6244cf735 100644 --- a/backend/e2e-test/routes/v1/secret-approval-policy.spec.ts +++ b/backend/e2e-test/routes/v1/secret-approval-policy.spec.ts @@ -1,6 +1,7 @@ import { seedData1 } from "@app/db/seed-data"; +import { ApproverType } from "@app/ee/services/access-approval-policy/access-approval-policy-types"; -const createPolicy = async (dto: { name: string; secretPath: string; approvers: string[]; approvals: number }) => { +const createPolicy = async (dto: { name: string; secretPath: string; approvers: {type: ApproverType.User, id: string}[]; approvals: number }) => { const res = await testServer.inject({ method: "POST", url: `/api/v1/secret-approvals`, @@ -26,7 +27,7 @@ describe("Secret approval policy router", async () => { const policy = await createPolicy({ secretPath: "/", approvals: 1, - approvers: [seedData1.id], + approvers: [{id:seedData1.id, type: ApproverType.User}], name: "test-policy" }); From 3a38e1e41395459ada52b7f38c489f642380210f Mon Sep 17 00:00:00 2001 From: Meet Date: Mon, 23 Sep 2024 19:04:57 +0530 Subject: [PATCH 8/8] chore: refactor --- backend/src/ee/routes/v1/access-approval-policy-router.ts | 4 ++-- backend/src/ee/routes/v1/secret-approval-policy-router.ts | 4 ++-- .../secret-approval-request-service.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/src/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index 464d9df4c..f8abcee89 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -20,7 +20,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi approvers: z .object({ type: z.nativeEnum(ApproverType), id: z.string() }) .array() - .min(1), + .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }), @@ -129,7 +129,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi approvers: z .object({ type: z.nativeEnum(ApproverType), id: z.string() }) .array() - .min(1), + .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).optional(), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }), diff --git a/backend/src/ee/routes/v1/secret-approval-policy-router.ts b/backend/src/ee/routes/v1/secret-approval-policy-router.ts index 34a7dc779..80b77aabf 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -30,7 +30,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi approvers: z .object({ type: z.nativeEnum(ApproverType), id: z.string() }) .array() - .min(1), + .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) }), @@ -71,7 +71,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi approvers: z .object({ type: z.nativeEnum(ApproverType), id: z.string() }) .array() - .min(1), + .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), secretPath: z .string() diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts index f9bb2ec10..ad476bbab 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts @@ -448,7 +448,7 @@ export const secretApprovalRequestServiceFactory = ({ const hasMinApproval = secretApprovalRequest.policy.approvals <= secretApprovalRequest.policy.approvers.filter(({ userId: approverId }) => - approverId ? reviewers[approverId.toString()] === ApprovalStatus.APPROVED : null + approverId ? reviewers[approverId] === ApprovalStatus.APPROVED : false ).length; const isSoftEnforcement = secretApprovalRequest.policy.enforcementLevel === EnforcementLevel.Soft;