Merge pull request #2458 from meetcshah19/meet/eng-1443-add-groups-as-eligible-approvers

feat: allow access approvals with user groups

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"
     });
 

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<void> {
+  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<void> {
+  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();
+    });
+  }
+}

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<typeof AccessApprovalPoliciesApproversSchema>;

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<typeof SecretApprovalPoliciesApproversSchema>;

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
+      body: z.object({
-        .object({
+        projectSlug: z.string().trim(),
-          projectSlug: z.string().trim(),
+        name: z.string().optional(),
-          name: z.string().optional(),
+        secretPath: z.string().trim().default("/"),
-          secretPath: z.string().trim().default("/"),
+        environment: z.string(),
-          environment: z.string(),
+        approvers: z
-          approvers: z.string().array().min(1),
+          .object({ type: z.nativeEnum(ApproverType), id: z.string() })
-          approvals: z.number().min(1).default(1),
+          .array()
-          enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
+          .min(1, { message: "At least one approver should be provided" }),
-        })
+        approvals: z.number().min(1).default(1),
-        .refine((data) => data.approvals <= data.approvers.length, {
+        enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
-          path: ["approvals"],
+      }),
-          message: "The number of approvals should be lower than the number of approvers."
-        }),
       response: {
         200: z.object({
           approval: sapPubSchema
@@ -58,14 +57,15 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi
         200: z.object({
           approvals: sapPubSchema
             .extend({
-              userApprovers: z
+              approvers: z
-                .object({
+                .object({ type: z.nativeEnum(ApproverType), id: z.string().nullable().optional() })
-                  userId: z.string()
+                .array()
-                })
+                .nullable()
-                .array(),
+                .optional()
-              secretPath: z.string().optional().nullable()
             })
             .array()
+            .nullable()
+            .optional()
         })
       }
     },
@@ -119,22 +119,20 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi
       params: z.object({
         policyId: z.string()
       }),
-      body: z
+      body: z.object({
-        .object({
+        name: z.string().optional(),
-          name: z.string().optional(),
+        secretPath: z
-          secretPath: z
+          .string()
-            .string()
+          .trim()
-            .trim()
+          .optional()
-            .optional()
+          .transform((val) => (val === "" ? "/" : val)),
-            .transform((val) => (val === "" ? "/" : val)),
+        approvers: z
-          approvers: z.string().array().min(1),
+          .object({ type: z.nativeEnum(ApproverType), id: z.string() })
-          approvals: z.number().min(1).default(1),
+          .array()
-          enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
+          .min(1, { message: "At least one approver should be provided" }),
-        })
+        approvals: z.number().min(1).optional(),
-        .refine((data) => data.approvals <= data.approvers.length, {
+        enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
-          path: ["approvals"],
+      }),
-          message: "The number of approvals should be lower than the number of approvers."
-        }),
       response: {
         200: z.object({
           approval: sapPubSchema

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
+      body: z.object({
-        .object({
+        workspaceId: z.string(),
-          workspaceId: z.string(),
+        name: z.string().optional(),
-          name: z.string().optional(),
+        environment: z.string(),
-          environment: z.string(),
+        secretPath: z
-          secretPath: z
+          .string()
-            .string()
+          .optional()
-            .optional()
+          .nullable()
-            .nullable()
+          .default("/")
-            .default("/")
+          .transform((val) => (val ? removeTrailingSlash(val) : val)),
-            .transform((val) => (val ? removeTrailingSlash(val) : val)),
+        approvers: z
-          approvers: z.string().array().min(1),
+          .object({ type: z.nativeEnum(ApproverType), id: z.string() })
-          approvals: z.number().min(1).default(1),
+          .array()
-          enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
+          .min(1, { message: "At least one approver should be provided" }),
-        })
+        approvals: z.number().min(1).default(1),
-        .refine((data) => data.approvals <= data.approvers.length, {
+        enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
-          path: ["approvals"],
+      }),
-          message: "The number of approvals should be lower than the number of approvers."
-        }),
       response: {
         200: z.object({
           approval: sapPubSchema
@@ -67,23 +66,21 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi
       params: z.object({
         sapId: z.string()
       }),
-      body: z
+      body: z.object({
-        .object({
+        name: z.string().optional(),
-          name: z.string().optional(),
+        approvers: z
-          approvers: z.string().array().min(1),
+          .object({ type: z.nativeEnum(ApproverType), id: z.string() })
-          approvals: z.number().min(1).default(1),
+          .array()
-          secretPath: z
+          .min(1, { message: "At least one approver should be provided" }),
-            .string()
+        approvals: z.number().min(1).default(1),
-            .optional()
+        secretPath: z
-            .nullable()
+          .string()
-            .transform((val) => (val ? removeTrailingSlash(val) : val))
+          .optional()
-            .transform((val) => (val === "" ? "/" : val)),
+          .nullable()
-          enforcementLevel: z.nativeEnum(EnforcementLevel).optional()
+          .transform((val) => (val ? removeTrailingSlash(val) : val))
-        })
+          .transform((val) => (val === "" ? "/" : val)),
-        .refine((data) => data.approvals <= data.approvers.length, {
+        enforcementLevel: z.nativeEnum(EnforcementLevel).optional()
-          path: ["approvals"],
+      }),
-          message: "The number of approvals should be lower than the number of approvers."
-        }),
       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()
         })

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()
         })
       }

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<typeof accessApprovalPolicyDALFactory>;
 
 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,
+            label: "approvers" as const,
-            mapper: ({ approverUserId }) => ({
+            mapper: ({ approverUserId: id }) => ({
-              userId: approverUserId
+              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,
+            label: "approvers" as const,
-            mapper: ({ approverUserId }) => ({
+            mapper: ({ approverUserId: id }) => ({
-              userId: approverUserId
+              id,
+              type: ApproverType.User
+            })
+          },
+          {
+            key: "approverGroupId",
+            label: "approvers" as const,
+            mapper: ({ approverGroupId: id }) => ({
+              id,
+              type: ApproverType.Group
             })
           }
         ]

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<TProjectEnvDALFactory, "find" | "findOne">;
   accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory;
   projectMembershipDAL: Pick<TProjectMembershipDALFactory, "find">;
+  groupDAL: TGroupDALFactory;
 };
 
 export type TAccessApprovalPolicyServiceFactory = ReturnType<typeof accessApprovalPolicyServiceFactory>;
@@ -32,6 +35,7 @@ export type TAccessApprovalPolicyServiceFactory = ReturnType<typeof accessApprov
 export const accessApprovalPolicyServiceFactory = ({
   accessApprovalPolicyDAL,
   accessApprovalPolicyApproverDAL,
+  groupDAL,
   permissionService,
   projectEnvDAL,
   projectDAL
@@ -52,7 +56,15 @@ export const accessApprovalPolicyServiceFactory = ({
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
     if (!project) throw new BadRequestError({ message: "Project not found" });
 
-    if (approvals > 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(
+      if (userApprovers) {
-        approvers.map((userId) => ({
+        await accessApprovalPolicyApproverDAL.insertMany(
-          approverUserId: userId,
+          userApprovers.map((userId) => ({
-          policyId: doc.id
+            approverUserId: userId,
-        })),
+            policyId: doc.id
-        tx
+          })),
-      );
+          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 {

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;

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<TUsers>(
           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<TUsers>(
+      .leftJoin<TUsers>(
         db(TableName.Users).as("accessApprovalPolicyApproverUser"),
         `${TableName.AccessApprovalPolicyApprover}.approverUserId`,
         "accessApprovalPolicyApproverUser.id"
       )
+      .leftJoin(
+        TableName.UserGroupMembership,
+        `${TableName.AccessApprovalPolicyApprover}.approverGroupId`,
+        `${TableName.UserGroupMembership}.groupId`
+      )
+
+      .leftJoin<TUsers>(
+        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
+            })
           }
         ]
       });

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<TGroupDALFactory, "findAllGroupMembers">;
   projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findById">;
   smtpService: Pick<TSmtpService, "sendMail">;
   userDAL: Pick<
@@ -70,6 +72,7 @@ type TSecretApprovalRequestServiceFactoryDep = {
 export type TAccessApprovalRequestServiceFactory = ReturnType<typeof accessApprovalRequestServiceFactory>;
 
 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)]
       }
     });
 

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<typeof secretApprovalPolicyDALFactory>;
 
 export const secretApprovalPolicyDALFactory = (db: TDbClient) => {
@@ -20,14 +22,29 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => {
         `${TableName.SecretApprovalPolicy}.id`,
         `${TableName.SecretApprovalPolicyApprover}.policyId`
       )
-
+      .leftJoin(
-      .leftJoin(TableName.Users, `${TableName.SecretApprovalPolicyApprover}.approverUserId`, `${TableName.Users}.id`)
+        TableName.UserGroupMembership,
-
+        `${TableName.SecretApprovalPolicyApprover}.approverGroupId`,
+        `${TableName.UserGroupMembership}.groupId`
+      )
+      .leftJoin<TUsers>(
+        db(TableName.Users).as("secretApprovalPolicyApproverUser"),
+        `${TableName.SecretApprovalPolicyApprover}.approverUserId`,
+        "secretApprovalPolicyApproverUser.id"
+      )
+      .leftJoin<TUsers>(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`)
       .select(
-        tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover),
+        tx.ref("id").withSchema("secretApprovalPolicyApproverUser").as("approverUserId"),
-        tx.ref("email").withSchema(TableName.Users).as("approverEmail"),
+        tx.ref("email").withSchema("secretApprovalPolicyApproverUser").as("approverEmail"),
-        tx.ref("firstName").withSchema(TableName.Users).as("approverFirstName"),
+        tx.ref("firstName").withSchema("secretApprovalPolicyApproverUser").as("approverFirstName"),
-        tx.ref("lastName").withSchema(TableName.Users).as("approverLastName")
+        tx.ref("lastName").withSchema("secretApprovalPolicyApproverUser").as("approverLastName")
+      )
+      .select(
+        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 }) => ({
+            mapper: ({
-              userId: approverUserId,
+              approverUserId: userId,
-              email: approverEmail,
+              approverEmail: email,
-              firstName: approverFirstName,
+              approverFirstName: firstName,
-              lastName: approverLastName
+              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
             })
           }
         ]
@@ -83,11 +120,34 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => {
           ...SecretApprovalPoliciesSchema.parse(data)
         }),
         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 }) => ({
+            mapper: ({ approverUserId: userId }) => ({
-              userId: approverUserId
+              userId
+            })
+          },
+          {
+            key: "approverGroupUserId",
+            label: "userApprovers" as const,
+            mapper: ({ approverGroupUserId: userId }) => ({
+              userId
             })
           }
         ]

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 {

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<TProjectPermission, "projectId">;

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<TUsers>(
+      .leftJoin<TUsers>(
         db(TableName.Users).as("secretApprovalPolicyApproverUser"),
         `${TableName.SecretApprovalPolicyApprover}.approverUserId`,
         "secretApprovalPolicyApproverUser.id"
       )
+      .leftJoin(
+        TableName.UserGroupMembership,
+        `${TableName.SecretApprovalPolicyApprover}.approverGroupId`,
+        `${TableName.UserGroupMembership}.groupId`
+      )
+      .leftJoin<TUsers>(
+        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<TUsers>(
           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<TUsers>(
           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
+            })
           }
         ]
       });

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(
+      secretApprovalRequest.policy.approvers.filter(({ userId: approverId }) =>
-        ({ userId: approverId }) => reviewers[approverId.toString()] === ApprovalStatus.APPROVED
+        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!)
         }
       });
 

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({

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;

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;

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) => {
               <Th>Environment</Th>
               <Th>Secret Path</Th>
               <Th>Eligible Approvers</Th>
+              <Th>Eligible Group Approvers</Th>
               <Th>Approval Required</Th>
               <Th>
                 <DropdownMenu>
@@ -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)}
                 />

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, {
+  .refine((data) => data.approvers, {
-    path: ["approvals"],
+    path: ["approvers"],
-    message: "The number of approvals should be lower than the number of approvers."
+    message: "At least one approver should be provided."
   });
 
 type TFormSchema = z.infer<typeof formSchema>;
@@ -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 } }) => (
                 <FormControl
-                  label="Required Approvers"
+                  label="Required User Approvers"
-                  isRequired
                   isError={Boolean(error)}
                   errorText={error?.message}
                 >
@@ -288,15 +289,15 @@ export const AccessPolicyForm = ({
                       </DropdownMenuLabel>
                       {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 (
                           <DropdownMenuItem
                             onClick={(evt) => {
                               evt.preventDefault();
                               onChange(
                                 isChecked
-                                  ? value?.filter((el: string) => el !== userId)
+                                  ? value?.filter((el: {id: string, type: ApproverType}) => el.id !== userId && el.type !== ApproverType.User)
-                                  : [...(value || []), userId]
+                                  : [...(value || []), {id:userId, type: ApproverType.User}]
                               );
                             }}
                             key={`create-policy-members-${userId}`}
@@ -312,6 +313,57 @@ export const AccessPolicyForm = ({
                 </FormControl>
               )}
             />
+            <Controller
+              control={control}
+              name="approvers"
+              render={({ field: { value, onChange }, fieldState: { error } }) => (
+                <FormControl
+                  label="Required Group Approvers"
+                  isError={Boolean(error)}
+                  errorText={error?.message}
+                >
+                  <DropdownMenu>
+                    <DropdownMenuTrigger asChild>
+                      <Input
+                        isReadOnly
+                        value={value?.length ? `${value.length} selected` : "None"}
+                        className="text-left"
+                      />
+                    </DropdownMenuTrigger>
+                    <DropdownMenuContent
+                      style={{ width: "var(--radix-dropdown-menu-trigger-width)" }}
+                      align="start"
+                    >
+                      <DropdownMenuLabel>
+                        Select groups that are allowed to approve requests
+                      </DropdownMenuLabel>
+                      {groups && groups.map(({ group }) => {
+                        const { id } = group;
+                        const isChecked = value?.includes({id, type: ApproverType.Group});
+
+                        return (
+                          <DropdownMenuItem
+                            onClick={(evt) => {
+                              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 && <FontAwesomeIcon icon={faCheckCircle} />}
+                          >
+                            {group.name}
+                          </DropdownMenuItem>
+                        );
+                      })}
+                    </DropdownMenuContent>
+                  </DropdownMenu>
+                </FormControl>
+              )}
+            />
             <Controller
               control={control}
               name="approvals"

frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx

@@ -18,6 +18,8 @@ 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";
 import { TWorkspaceUser } from "@app/hooks/api/users/types";
@@ -29,8 +31,7 @@ interface IPolicy {
   projectId?: string;
   secretPath?: string;
   approvals: number;
-  approvers?: string[];
+  approvers?: Approver[];
-  userApprovers?: { userId: string }[];
   updatedAt: Date;
   policyType: PolicyType;
   enforcementLevel: EnforcementLevel;
@@ -39,6 +40,7 @@ interface IPolicy {
 type Props = {
   policy: IPolicy;
   members?: TWorkspaceUser[];
+  groups?: TGroupMembership[];
   projectSlug: string;
   workspaceId: string;
   onEdit: () => void;
@@ -48,12 +50,14 @@ type Props = {
 export const ApprovalPolicyRow = ({
   policy,
   members = [],
+  groups = [],
   projectSlug,
   workspaceId,
   onEdit,
   onDelete
 }: Props) => {
-  const [selectedApprovers, setSelectedApprovers] = useState<string[]>(policy.userApprovers?.map(({ userId }) => userId) || policy.approvers || []);
+  const [selectedApprovers, setSelectedApprovers] = useState<Approver[]>(policy.approvers?.filter((approver) => approver.type === ApproverType.User) || []);
+  const [selectedGroupApprovers, setSelectedGroupApprovers] = useState<Approver[]>(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
+              setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []);
-                ? policy?.userApprovers?.map(({ userId }) => userId) || []
-                : policy?.approvers || []
-              );
             }
           }}
-      >
+        >
           <DropdownMenuTrigger
             asChild
             disabled={
@@ -116,15 +125,15 @@ export const ApprovalPolicyRow = ({
             <DropdownMenuLabel>
               Select members that are allowed to approve changes
             </DropdownMenuLabel>
-            {members?.map(({ id, user }) => {
+            {members?.map(({ user }) => {
-              const userId = policy.policyType === PolicyType.ChangePolicy ? user.id : id;
+              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 (
                 <DropdownMenuItem
                   onClick={(evt) => {
                     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 = ({
           </DropdownMenuContent>
         </DropdownMenu>
       </Td>
+      <Td>
+        <DropdownMenu
+          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 {
+              setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []);
+            }
+          }}
+        >
+          <DropdownMenuTrigger asChild>
+            <Input
+              isReadOnly
+              value={selectedGroupApprovers?.length ? `${selectedGroupApprovers.length} selected` : "None"}
+              className="text-left"
+            />
+          </DropdownMenuTrigger>
+          <DropdownMenuContent
+            style={{ width: "var(--radix-dropdown-menu-trigger-width)" }}
+            align="start"
+          >
+            <DropdownMenuLabel>
+              Select groups that are allowed to approve requests
+            </DropdownMenuLabel>
+            {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 (
+                <DropdownMenuItem
+                  onClick={(evt) => {
+                    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 && <FontAwesomeIcon icon={faCheckCircle} />}
+                >
+                  {group.name}
+                </DropdownMenuItem>
+              );
+            })}
+          </DropdownMenuContent>
+        </DropdownMenu>
+      </Td>
       <Td>{policy.approvals}</Td>
       <Td>
         <Badge className={policyDetails[policy.policyType].className}>