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 3234503e54..6244cf735a 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" }); 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 0000000000..ed4534eb5f --- /dev/null +++ b/backend/src/db/migrations/20240918005344_add-group-approvals.ts @@ -0,0 +1,36 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + 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(); + }); + } +} + +export async function down(knex: Knex): Promise { + 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/schemas/access-approval-policies-approvers.ts b/backend/src/db/schemas/access-approval-policies-approvers.ts index 8795c486e7..1ecd805138 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/secret-approval-policies-approvers.ts b/backend/src/db/schemas/secret-approval-policies-approvers.ts index 3af5614f4d..f9aebf0194 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/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index a57a26d812..f8abcee898 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,20 +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().min(1), - 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." - }), + 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, { message: "At least one approver should be provided" }), + approvals: z.number().min(1).default(1), + enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) + }), response: { 200: z.object({ approval: sapPubSchema @@ -58,14 +57,15 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi 200: z.object({ approvals: sapPubSchema .extend({ - userApprovers: z - .object({ - userId: 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() }) } }, @@ -119,22 +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().min(1), - 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." - }), + 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, { message: "At least one approver should be provided" }), + 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 25c1bb0b50..80b77aabf1 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,25 +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().min(1), - 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." - }), + 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, { message: "At least one approver should be provided" }), + approvals: z.number().min(1).default(1), + enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard) + }), response: { 200: z.object({ approval: sapPubSchema @@ -67,23 +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().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() - }) - .refine((data) => data.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." - }), + body: z.object({ + name: z.string().optional(), + approvers: z + .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .array() + .min(1, { message: "At least one approver should be provided" }), + 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 @@ -147,9 +144,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi 200: z.object({ approvals: sapPubSchema .extend({ - userApprovers: z + approvers: z .object({ - userId: z.string() + id: z.string().nullable().optional(), + type: z.nativeEnum(ApproverType) }) .array() }) @@ -186,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 bcc4a36c5e..5fbf784f61 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, @@ -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().nullable().optional() + }) + .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().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 c224bd3ca7..51303dcfdb 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) => { @@ -21,6 +23,7 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicyApprover}.policyId` ) .select(tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover)) + .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")) @@ -30,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, @@ -50,9 +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: "approvers" as const, + mapper: ({ approverGroupId: id }) => ({ + id, + type: "group" }) } ] @@ -84,9 +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: "approvers" as const, + mapper: ({ approverGroupId: id }) => ({ + id, + type: ApproverType.Group }) } ] 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 153941771d..fdb242314b 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 @@ -7,10 +7,12 @@ 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 { + ApproverType, TCreateAccessApprovalPolicy, TDeleteAccessApprovalPolicy, TGetAccessPolicyCountByEnvironmentDTO, @@ -25,6 +27,7 @@ type TSecretApprovalPolicyServiceFactoryDep = { projectEnvDAL: Pick; accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory; projectMembershipDAL: Pick; + groupDAL: TGroupDALFactory; }; export type TAccessApprovalPolicyServiceFactory = ReturnType; @@ -32,6 +35,7 @@ export type TAccessApprovalPolicyServiceFactory = ReturnType approvers.length) + // If there is a group approver people might be added to the group later to meet the approvers quota + 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( @@ -69,6 +81,24 @@ export const accessApprovalPolicyServiceFactory = ({ const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id }); if (!env) throw new BadRequestError({ message: "Environment not found" }); + const verifyAllApprovers = userApprovers; + 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); + await verifyApprovers({ projectId: project.id, orgId: actorOrgId, @@ -76,7 +106,7 @@ export const accessApprovalPolicyServiceFactory = ({ secretPath, actorAuthMethod, permissionService, - userIds: approvers + userIds: verifyAllApprovers }); const accessApproval = await accessApprovalPolicyDAL.transaction(async (tx) => { @@ -90,13 +120,26 @@ 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 + ); + } + + if (groupApprovers) { + await accessApprovalPolicyApproverDAL.insertMany( + groupApprovers.map((groupId) => ({ + approverGroupId: groupId, + policyId: doc.id + })), + tx + ); + } + return doc; }); return { ...accessApproval, environment: env, projectId: project.id }; @@ -138,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, @@ -161,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, @@ -169,18 +227,52 @@ 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 })), 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 accessApprovalPolicyApproverDAL.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 fdb6fc8bbb..c20aabfadc 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,11 +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[]; + approvers: { type: ApproverType; id: string }[]; projectSlug: string; name: string; enforcementLevel: EnforcementLevel; @@ -26,7 +31,7 @@ export type TCreateAccessApprovalPolicy = { export type TUpdateAccessApprovalPolicy = { policyId: string; approvals?: number; - approvers?: 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 48e2d88bf9..8784d05e2c 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,12 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.AccessApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + .leftJoin(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`) .join( db(TableName.Users).as("requestedByUser"), @@ -59,6 +65,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 +149,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,17 +184,28 @@ 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.UserGroupMembership, + `${TableName.AccessApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + + .leftJoin( + db(TableName.Users).as("accessApprovalPolicyGroupApproverUser"), + `${TableName.UserGroupMembership}.userId`, + "accessApprovalPolicyGroupApproverUser.id" + ) .leftJoin( TableName.AccessApprovalRequestReviewer, @@ -200,10 +223,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 +310,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 2a7953ead8..005ecb2286 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,7 @@ 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 { 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"; @@ -57,6 +58,7 @@ type TSecretApprovalRequestServiceFactoryDep = { TAccessApprovalRequestReviewerDALFactory, "create" | "find" | "findOne" | "transaction" >; + groupDAL: Pick; projectMembershipDAL: Pick; smtpService: Pick; userDAL: Pick< @@ -70,6 +72,7 @@ type TSecretApprovalRequestServiceFactoryDep = { export type TAccessApprovalRequestServiceFactory = ReturnType; export const accessApprovalRequestServiceFactory = ({ + groupDAL, projectDAL, projectEnvDAL, permissionService, @@ -124,13 +127,36 @@ export const accessApprovalRequestServiceFactory = ({ }); if (!policy) throw new UnauthorizedError({ message: "No policy matching criteria was found." }); + const approverIds: string[] = []; + const approverGroupIds: string[] = []; + const approvers = await accessApprovalPolicyApproverDAL.find({ policyId: policy.id }); + approvers.forEach((approver) => { + if (approver.approverUserId) { + approverIds.push(approver.approverUserId); + } else if (approver.approverGroupId) { + approverGroupIds.push(approver.approverGroupId); + } + }); + + const groupUsers = ( + await Promise.all( + approverGroupIds.map((groupApproverId) => + groupDAL.findAllGroupMembers({ + orgId: actorOrgId, + groupId: groupApproverId + }) + ) + ) + ).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/ee/services/secret-approval-policy/secret-approval-policy-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts index 6d5168f264..651e19b643 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,14 +22,29 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - - .leftJoin(TableName.Users, `${TableName.SecretApprovalPolicyApprover}.approverUserId`, `${TableName.Users}.id`) - + .leftJoin( + TableName.UserGroupMembership, + `${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") + 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"), + tx.ref("lastName").withSchema(TableName.Users).as("approverGroupLastName") ) .select( tx.ref("name").withSchema(TableName.Environment).as("envName"), @@ -55,11 +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: userId, + approverGroupEmail: email, + approverGroupFirstName: firstName, + approverGroupLastName: lastName + }) => ({ + userId, + email, + firstName, + lastName }) } ] @@ -85,9 +122,32 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { childrenMapper: [ { key: "approverUserId", + label: "approvers" as const, + mapper: ({ approverUserId: id }) => ({ + type: ApproverType.User, + id + }) + }, + { + key: "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: ({ approverUserId }) => ({ - userId: approverUserId + mapper: ({ approverGroupUserId: userId }) => ({ + userId }) } ] 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 8c974b05d5..c1f7cce311 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,6 +8,7 @@ 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"; @@ -54,7 +55,14 @@ export const secretApprovalPolicyServiceFactory = ({ environment, enforcementLevel }: TCreateSapDTO) => { - if (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 > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -91,13 +99,22 @@ export const secretApprovalPolicyServiceFactory = ({ }, tx ); + await secretApprovalPolicyApproverDAL.insertMany( - approvers.map((approverUserId) => ({ + userApprovers.map((approverUserId) => ({ approverUserId, policyId: doc.id })), tx ); + + await secretApprovalPolicyApproverDAL.insertMany( + groupApprovers.map((approverGroupId) => ({ + approverGroupId, + policyId: doc.id + })), + tx + ); return doc; }); return { ...secretApproval, environment: env, projectId }; @@ -115,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" }); @@ -146,16 +170,28 @@ 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 secretApprovalPolicyApproverDAL.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 8e7099c981..78b77c152b 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,10 +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[]; + approvers: { type: ApproverType; id: string }[]; projectId: string; name: string; enforcementLevel: EnforcementLevel; @@ -14,7 +16,7 @@ export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; secretPath?: string | null; - approvers: 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 54c7563f62..803b9464c2 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,26 @@ 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.UserGroupMembership, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) + .leftJoin( + db(TableName.Users).as("secretApprovalPolicyGroupApproverUser"), + `${TableName.UserGroupMembership}.userId`, + `secretApprovalPolicyGroupApproverUser.id` + ) .leftJoin( TableName.SecretApprovalRequestReviewer, `${TableName.SecretApprovalRequest}.id`, @@ -71,10 +81,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"), @@ -152,13 +167,30 @@ 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, + username + }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ + approverGroupUserId: userId, + approverGroupEmail: email, + approverGroupUsername: username, + approverGroupLastName: lastName, + approverGroupFirstName: firstName + }) => ({ + userId, email, firstName, lastName, @@ -236,11 +268,16 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalRequest}.policyId`, `${TableName.SecretApprovalPolicy}.id` ) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) .join( db(TableName.Users).as("committerUser"), `${TableName.SecretApprovalRequest}.committerUserId`, @@ -269,6 +306,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 +327,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 +373,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId }) => approverUserId + mapper: ({ approverUserId }) => ({ userId: approverUserId }) }, { key: "commitId", @@ -344,6 +383,11 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { id, secretId }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + mapper: ({ approverGroupUserId }) => ({ userId: approverGroupUserId }) } ] }); @@ -371,11 +415,16 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalRequest}.policyId`, `${TableName.SecretApprovalPolicy}.id` ) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.SecretApprovalPolicyApprover}.approverGroupId`, + `${TableName.UserGroupMembership}.groupId` + ) .join( db(TableName.Users).as("committerUser"), `${TableName.SecretApprovalRequest}.committerUserId`, @@ -404,6 +453,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 +474,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 +520,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId }) => approverUserId + mapper: ({ approverUserId }) => ({ userId: approverUserId }) }, { key: "commitId", @@ -479,6 +530,13 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { id, secretId }) + }, + { + key: "approverGroupUserId", + label: "approvers" as const, + 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 1b606f617b..ad476bbab7 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] === ApprovalStatus.APPROVED : false ).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 3eb6b00310..3946c841a8 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -923,6 +923,7 @@ export const registerRoutes = async ( const accessApprovalPolicyService = accessApprovalPolicyServiceFactory({ accessApprovalPolicyDAL, accessApprovalPolicyApproverDAL, + groupDAL, permissionService, projectEnvDAL, projectMembershipDAL, @@ -942,7 +943,8 @@ export const registerRoutes = async ( smtpService, accessApprovalPolicyApproverDAL, projectSlackConfigDAL, - kmsService + kmsService, + groupDAL }); const secretReplicationService = secretReplicationServiceFactory({ diff --git a/frontend/src/hooks/api/accessApproval/types.ts b/frontend/src/hooks/api/accessApproval/types.ts index 47917716de..6df2575903 100644 --- a/frontend/src/hooks/api/accessApproval/types.ts +++ b/frontend/src/hooks/api/accessApproval/types.ts @@ -11,14 +11,23 @@ export type TAccessApprovalPolicy = { workspace: string; environment: WorkspaceEnv; projectId: string; - approvers: string[]; policyType: PolicyType; approversRequired: boolean; enforcementLevel: EnforcementLevel; updatedAt: Date; - userApprovers?: { userId: string }[]; + approvers?: Approver[]; }; +export enum ApproverType{ + User = "user", + Group = "group" +} + +export type Approver ={ + id: string; + type: ApproverType; +} + export type TAccessApprovalRequest = { id: string; policyId: string; @@ -130,7 +139,7 @@ export type TCreateAccessPolicyDTO = { projectSlug: string; name?: string; environment: string; - approvers?: string[]; + approvers?: Approver[]; approvals?: number; secretPath?: string; enforcementLevel?: EnforcementLevel; @@ -139,7 +148,7 @@ export type TCreateAccessPolicyDTO = { export type TUpdateAccessPolicyDTO = { id: string; name?: string; - approvers?: string[]; + approvers?: Approver[]; secretPath?: string; environment?: string; approvals?: number; diff --git a/frontend/src/hooks/api/secretApproval/types.ts b/frontend/src/hooks/api/secretApproval/types.ts index 06ffad432e..fcf83086c7 100644 --- a/frontend/src/hooks/api/secretApproval/types.ts +++ b/frontend/src/hooks/api/secretApproval/types.ts @@ -9,11 +9,21 @@ export type TSecretApprovalPolicy = { environment: WorkspaceEnv; secretPath?: string; approvals: number; - userApprovers: { userId: 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; }; @@ -29,7 +39,7 @@ export type TCreateSecretPolicyDTO = { name?: string; environment: string; secretPath?: string | null; - approvers?: string[]; + approvers?: Approver[]; approvals?: number; enforcementLevel: EnforcementLevel; }; @@ -37,7 +47,7 @@ export type TCreateSecretPolicyDTO = { export type TUpdateSecretPolicyDTO = { id: string; name?: string; - approvers?: string[]; + approvers?: Approver[]; secretPath?: string | null; approvals?: number; enforcementLevel?: EnforcementLevel; diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx index 7234c18ce4..0653a92e84 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 24a470bcc6..f2da726034 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx @@ -22,12 +22,12 @@ 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 } 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,13 @@ const formSchema = z name: z.string().optional(), secretPath: z.string().optional(), approvals: z.number().min(1), - approvers: z.string().array().min(1), + 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.approvals <= data.approvers.length, { - path: ["approvals"], - message: "The number of approvals should be lower than the number of approvers." + .refine((data) => data.approvers, { + path: ["approvers"], + message: "At least one approver should be provided." }); type TFormSchema = z.infer; @@ -75,11 +75,13 @@ export const AccessPolicyForm = ({ ? { ...editValues, environment: editValues.environment.slug, - approvers: editValues?.userApprovers?.map((user) => user.userId) || editValues?.approvers + approvers: editValues?.approvers || [], + approvals: editValues?.approvals } : undefined }); const { currentWorkspace } = useWorkspace(); + const { data: groups } = useListWorkspaceGroups(projectSlug); const environments = currentWorkspace?.environments || []; const isEditMode = Boolean(editValues); @@ -266,8 +268,7 @@ export const AccessPolicyForm = ({ name="approvers" render={({ field: { value, onChange }, fieldState: { error } }) => ( @@ -288,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}`} @@ -312,6 +313,57 @@ export const AccessPolicyForm = ({ )} /> + ( + + + + + + + + Select groups that are allowed to approve requests + + {groups && groups.map(({ group }) => { + const { id } = group; + const isChecked = value?.includes({id, type: ApproverType.Group}); + + return ( + { + evt.preventDefault(); + onChange( + isChecked + ? value?.filter((el: {id: string, type: ApproverType}) => el.id !== id && el.type !== ApproverType.Group) + : [...(value || []), {id, type: ApproverType.Group}] + ); + }} + key={`create-policy-members-${id}`} + iconPos="right" + icon={isChecked && } + > + {group.name} + + ); + })} + + + + )} + /> void; @@ -48,12 +50,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 [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; @@ -74,28 +78,33 @@ export const ApprovalPolicyRow = ({ { projectSlug, id: policy.id, - approvers: selectedApprovers + approvers: selectedApprovers.concat(selectedGroupApprovers), }, - { onSettled: () => {} } + { + onError: () => { + setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); + } + } ); } else { updateSecretApprovalPolicy( { workspaceId, id: policy.id, - approvers: selectedApprovers + 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) || []); } }} - > + > Select members that are allowed to approve changes - {members?.map(({ id, user }) => { - const userId = policy.policyType === PolicyType.ChangePolicy ? user.id : id; - const isChecked = selectedApprovers.includes(userId); + {members?.map(({ user }) => { + const userId = user.id; + 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}`} @@ -138,6 +147,80 @@ export const ApprovalPolicyRow = ({ + + { + 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 { + setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); + } + }} + > + + + + + + Select groups that are allowed to approve requests + + {groups && groups.map(({ group }) => { + const { id } = group; + 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) => el.id !== id || el.type !== ApproverType.Group) + : [...(selectedGroupApprovers || []), { id, type: ApproverType.Group }] + ); + }} + key={`create-policy-groups-${id}`} + iconPos="right" + icon={isChecked && } + > + {group.name} + + ); + })} + + + {policy.approvals}