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}`}