From 6733349af03b6f4f9885b71cb72405f49d092f7f Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Wed, 25 Sep 2024 00:07:11 +0800 Subject: [PATCH 1/5] misc: updated secret approval policy api to support TF usecase --- .../v1/secret-approval-policy-router.ts | 48 +++++++- .../secret-approval-policy-dal.ts | 27 ++++- .../secret-approval-policy-service.ts | 103 +++++++++++++++++- .../secret-approval-policy-types.ts | 6 +- 4 files changed, 170 insertions(+), 14 deletions(-) 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 80b77aabf..49a080e84 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -28,7 +28,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .default("/") .transform((val) => (val ? removeTrailingSlash(val) : val)), approvers: z - .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .discriminatedUnion("type", [ + z.object({ type: z.literal(ApproverType.Group), id: z.string() }), + z.object({ type: z.literal(ApproverType.User), id: z.string().optional(), name: z.string().optional() }) + ]) .array() .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), @@ -69,7 +72,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi body: z.object({ name: z.string().optional(), approvers: z - .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .discriminatedUnion("type", [ + z.object({ type: z.literal(ApproverType.Group), id: z.string() }), + z.object({ type: z.literal(ApproverType.User), id: z.string().optional(), name: z.string().optional() }) + ]) .array() .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), @@ -168,6 +174,44 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi } }); + server.route({ + url: "/:sapId", + method: "GET", + config: { + rateLimit: readLimit + }, + schema: { + params: z.object({ + sapId: z.string() + }), + response: { + 200: z.object({ + approval: sapPubSchema.extend({ + approvers: z + .object({ + id: z.string().nullable().optional(), + type: z.nativeEnum(ApproverType), + name: z.string().nullable().optional() + }) + .array() + }) + }) + } + }, + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), + handler: async (req) => { + const approval = await server.services.secretApprovalPolicy.getSecretApprovalPolicyById({ + actor: req.permission.type, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId, + ...req.params + }); + + return { approval }; + } + }); + server.route({ url: "/board", method: "GET", 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 651e19b64..e3526b0e3 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 @@ -12,10 +12,21 @@ export type TSecretApprovalPolicyDALFactory = ReturnType { const secretApprovalPolicyOrm = ormify(db, TableName.SecretApprovalPolicy); - const secretApprovalPolicyFindQuery = (tx: Knex, filter: TFindFilter) => + const secretApprovalPolicyFindQuery = ( + tx: Knex, + filter: TFindFilter, + customFilter?: { + sapId?: string; + } + ) => tx(TableName.SecretApprovalPolicy) // eslint-disable-next-line .where(buildFindFilter(filter)) + .where((qb) => { + if (customFilter?.sapId) { + void qb.where(`${TableName.SecretApprovalPolicy}.id`, "=", customFilter.sapId); + } + }) .join(TableName.Environment, `${TableName.SecretApprovalPolicy}.envId`, `${TableName.Environment}.id`) .leftJoin( TableName.SecretApprovalPolicyApprover, @@ -37,6 +48,7 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { tx.ref("id").withSchema("secretApprovalPolicyApproverUser").as("approverUserId"), tx.ref("email").withSchema("secretApprovalPolicyApproverUser").as("approverEmail"), tx.ref("firstName").withSchema("secretApprovalPolicyApproverUser").as("approverFirstName"), + tx.ref("username").withSchema("secretApprovalPolicyApproverUser").as("approverUsername"), tx.ref("lastName").withSchema("secretApprovalPolicyApproverUser").as("approverLastName") ) .select( @@ -108,9 +120,15 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { } }; - const find = async (filter: TFindFilter, tx?: Knex) => { + const find = async ( + filter: TFindFilter, + customFilter?: { + sapId?: string; + }, + tx?: Knex + ) => { try { - const docs = await secretApprovalPolicyFindQuery(tx || db.replicaNode(), filter); + const docs = await secretApprovalPolicyFindQuery(tx || db.replicaNode(), filter, customFilter); const formatedDoc = sqlNestRelationships({ data: docs, key: "id", @@ -123,8 +141,9 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId: id }) => ({ + mapper: ({ approverUserId: id, approverUsername }) => ({ type: ApproverType.User, + name: approverUsername, id }) }, 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 c1f7cce31..5af05a71f 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 @@ -3,10 +3,11 @@ import picomatch from "picomatch"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; -import { BadRequestError } from "@app/lib/errors"; +import { BadRequestError, NotFoundError } from "@app/lib/errors"; import { removeTrailingSlash } from "@app/lib/fn"; import { containsGlobPatterns } from "@app/lib/picomatch"; import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal"; +import { TUserDALFactory } from "@app/services/user/user-dal"; import { ApproverType } from "../access-approval-policy/access-approval-policy-types"; import { TLicenseServiceFactory } from "../license/license-service"; @@ -16,6 +17,7 @@ import { TCreateSapDTO, TDeleteSapDTO, TGetBoardSapDTO, + TGetSapByIdDTO, TListSapDTO, TUpdateSapDTO } from "./secret-approval-policy-types"; @@ -29,6 +31,7 @@ type TSecretApprovalPolicyServiceFactoryDep = { permissionService: Pick; secretApprovalPolicyDAL: TSecretApprovalPolicyDALFactory; projectEnvDAL: Pick; + userDAL: Pick; secretApprovalPolicyApproverDAL: TSecretApprovalPolicyApproverDALFactory; licenseService: Pick; }; @@ -40,6 +43,7 @@ export const secretApprovalPolicyServiceFactory = ({ permissionService, secretApprovalPolicyApproverDAL, projectEnvDAL, + userDAL, licenseService }: TSecretApprovalPolicyServiceFactoryDep) => { const createSecretApprovalPolicy = async ({ @@ -60,7 +64,12 @@ export const secretApprovalPolicyServiceFactory = ({ .map((approver) => approver.id); const userApprovers = approvers ?.filter((approver) => approver.type === ApproverType.User) - .map((approver) => approver.id); + .map((approver) => approver.id) + .filter(Boolean) as string[]; + + const userApproverNames = approvers + .map((approver) => (approver.type === ApproverType.User ? approver.name : undefined)) + .filter(Boolean) as string[]; if (!groupApprovers && approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); @@ -100,8 +109,31 @@ export const secretApprovalPolicyServiceFactory = ({ tx ); + let userApproverIds = userApprovers; + if (userApproverNames.length) { + const approverUsers = await userDAL.find( + { + $in: { + username: userApproverNames + } + }, + { tx } + ); + + const approverNamesFromDb = approverUsers.map((user) => user.username); + const invalidUsernames = userApproverNames?.filter((username) => !approverNamesFromDb.includes(username)); + + if (invalidUsernames?.length) { + throw new BadRequestError({ + message: `Invalid approver user: ${invalidUsernames.join(", ")}` + }); + } + + userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id)); + } + await secretApprovalPolicyApproverDAL.insertMany( - userApprovers.map((approverUserId) => ({ + userApproverIds.map((approverUserId) => ({ approverUserId, policyId: doc.id })), @@ -117,6 +149,7 @@ export const secretApprovalPolicyServiceFactory = ({ ); return doc; }); + return { ...secretApproval, environment: env, projectId }; }; @@ -137,7 +170,12 @@ export const secretApprovalPolicyServiceFactory = ({ .map((approver) => approver.id); const userApprovers = approvers ?.filter((approver) => approver.type === ApproverType.User) - .map((approver) => approver.id); + .map((approver) => approver.id) + .filter(Boolean) as string[]; + + const userApproverNames = approvers + .map((approver) => (approver.type === ApproverType.User ? approver.name : undefined)) + .filter(Boolean) as string[]; const secretApprovalPolicy = await secretApprovalPolicyDAL.findById(secretPolicyId); if (!secretApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" }); @@ -174,8 +212,31 @@ export const secretApprovalPolicyServiceFactory = ({ await secretApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); if (approvers) { + let userApproverIds = userApprovers; + if (userApproverNames) { + const approverUsers = await userDAL.find( + { + $in: { + username: userApproverNames + } + }, + { tx } + ); + + const approverNamesFromDb = approverUsers.map((user) => user.username); + const invalidUsernames = userApproverNames?.filter((username) => !approverNamesFromDb.includes(username)); + + if (invalidUsernames?.length) { + throw new BadRequestError({ + message: `Invalid approver user: ${invalidUsernames.join(", ")}` + }); + } + + userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id)); + } + await secretApprovalPolicyApproverDAL.insertMany( - userApprovers.map((approverUserId) => ({ + userApproverIds.map((approverUserId) => ({ approverUserId, policyId: doc.id })), @@ -192,6 +253,7 @@ export const secretApprovalPolicyServiceFactory = ({ tx ); } + return doc; }); return { @@ -296,12 +358,41 @@ export const secretApprovalPolicyServiceFactory = ({ return getSecretApprovalPolicy(projectId, environment, secretPath); }; + const getSecretApprovalPolicyById = async ({ + actorId, + actor, + actorOrgId, + actorAuthMethod, + sapId + }: TGetSapByIdDTO) => { + const [sapPolicy] = await secretApprovalPolicyDAL.find({}, { sapId }); + + if (!sapPolicy) { + throw new NotFoundError({ + message: "Cannot find secret approval policy" + }); + } + + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + sapPolicy.projectId, + actorAuthMethod, + actorOrgId + ); + + ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval); + + return sapPolicy; + }; + return { createSecretApprovalPolicy, updateSecretApprovalPolicy, deleteSecretApprovalPolicy, getSecretApprovalPolicy, getSecretApprovalPolicyByProjectId, - getSecretApprovalPolicyOfFolder + getSecretApprovalPolicyOfFolder, + getSecretApprovalPolicyById }; }; 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 78b77c152..863f1c926 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 @@ -6,7 +6,7 @@ export type TCreateSapDTO = { approvals: number; secretPath?: string | null; environment: string; - approvers: { type: ApproverType; id: string }[]; + approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; name?: string })[]; projectId: string; name: string; enforcementLevel: EnforcementLevel; @@ -16,7 +16,7 @@ export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; secretPath?: string | null; - approvers: { type: ApproverType; id: string }[]; + approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; name?: string })[]; name?: string; enforcementLevel?: EnforcementLevel; } & Omit; @@ -27,6 +27,8 @@ export type TDeleteSapDTO = { export type TListSapDTO = TProjectPermission; +export type TGetSapByIdDTO = Omit & { sapId: string }; + export type TGetBoardSapDTO = { projectId: string; environment: string; From 974e21d856367ba92312542aa47b5cf7872fa274 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Wed, 25 Sep 2024 14:30:22 +0800 Subject: [PATCH 2/5] fix: addressed bugs --- backend/src/ee/routes/v1/secret-approval-policy-router.ts | 6 +++--- .../secret-approval-policy-service.ts | 2 +- backend/src/server/routes/index.ts | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) 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 49a080e84..40f0a71bd 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -43,7 +43,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { const approval = await server.services.secretApprovalPolicy.createSecretApprovalPolicy({ actor: req.permission.type, @@ -93,7 +93,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { const approval = await server.services.secretApprovalPolicy.updateSecretApprovalPolicy({ actor: req.permission.type, @@ -123,7 +123,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { const approval = await server.services.secretApprovalPolicy.deleteSecretApprovalPolicy({ actor: req.permission.type, 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 5af05a71f..522989fc0 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 @@ -71,7 +71,7 @@ export const secretApprovalPolicyServiceFactory = ({ .map((approver) => (approver.type === ApproverType.User ? approver.name : undefined)) .filter(Boolean) as string[]; - if (!groupApprovers && approvals > approvers.length) + if (!groupApprovers.length && approvals > approvers.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 3946c841a..f1c51f8b5 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -380,7 +380,8 @@ export const registerRoutes = async ( secretApprovalPolicyApproverDAL: sapApproverDAL, permissionService, secretApprovalPolicyDAL, - licenseService + licenseService, + userDAL }); const tokenService = tokenServiceFactory({ tokenDAL: authTokenDAL, userDAL, orgMembershipDAL }); From cf6ff58f168cb33a71c56d8af6ea3153e02655b1 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Wed, 25 Sep 2024 18:38:06 +0800 Subject: [PATCH 3/5] misc: access approval prerequisites --- .../v1/access-approval-policy-router.ts | 57 +++++++- .../access-approval-policy-dal.ts | 30 ++++- .../access-approval-policy-service.ts | 124 +++++++++++++++--- .../access-approval-policy-types.ts | 8 +- backend/src/server/routes/index.ts | 3 +- 5 files changed, 192 insertions(+), 30 deletions(-) diff --git a/backend/src/ee/routes/v1/access-approval-policy-router.ts b/backend/src/ee/routes/v1/access-approval-policy-router.ts index f8abcee89..49d1c7765 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { ApproverType } from "@app/ee/services/access-approval-policy/access-approval-policy-types"; import { EnforcementLevel } from "@app/lib/types"; +import { readLimit } from "@app/server/config/rateLimiter"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { sapPubSchema } from "@app/server/routes/sanitizedSchemas"; import { AuthMode } from "@app/services/auth/auth-type"; @@ -18,7 +19,10 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi secretPath: z.string().trim().default("/"), environment: z.string(), approvers: z - .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .discriminatedUnion("type", [ + z.object({ type: z.literal(ApproverType.Group), id: z.string() }), + z.object({ type: z.literal(ApproverType.User), id: z.string().optional(), name: z.string().optional() }) + ]) .array() .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).default(1), @@ -30,7 +34,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { const approval = await server.services.accessApprovalPolicy.createAccessApprovalPolicy({ actor: req.permission.type, @@ -127,7 +131,10 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi .optional() .transform((val) => (val === "" ? "/" : val)), approvers: z - .object({ type: z.nativeEnum(ApproverType), id: z.string() }) + .discriminatedUnion("type", [ + z.object({ type: z.literal(ApproverType.Group), id: z.string() }), + z.object({ type: z.literal(ApproverType.User), id: z.string().optional(), name: z.string().optional() }) + ]) .array() .min(1, { message: "At least one approver should be provided" }), approvals: z.number().min(1).optional(), @@ -139,7 +146,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { await server.services.accessApprovalPolicy.updateAccessApprovalPolicy({ policyId: req.params.policyId, @@ -165,7 +172,7 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi }) } }, - onRequest: verifyAuth([AuthMode.JWT]), + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { const approval = await server.services.accessApprovalPolicy.deleteAccessApprovalPolicy({ actor: req.permission.type, @@ -177,4 +184,44 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi return { approval }; } }); + + server.route({ + url: "/:policyId", + method: "GET", + config: { + rateLimit: readLimit + }, + schema: { + params: z.object({ + policyId: z.string() + }), + response: { + 200: z.object({ + approval: sapPubSchema.extend({ + approvers: z + .object({ + type: z.nativeEnum(ApproverType), + id: z.string().nullable().optional(), + name: z.string().nullable().optional() + }) + .array() + .nullable() + .optional() + }) + }) + } + }, + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), + handler: async (req) => { + const approval = await server.services.accessApprovalPolicy.getAccessApprovalPolicyById({ + actor: req.permission.type, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId, + ...req.params + }); + + return { approval }; + } + }); }; 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 51303dcfd..569580507 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 @@ -12,16 +12,29 @@ export type TAccessApprovalPolicyDALFactory = ReturnType { const accessApprovalPolicyOrm = ormify(db, TableName.AccessApprovalPolicy); - const accessApprovalPolicyFindQuery = async (tx: Knex, filter: TFindFilter) => { + const accessApprovalPolicyFindQuery = async ( + tx: Knex, + filter: TFindFilter, + customFilter?: { + policyId?: string; + } + ) => { const result = await tx(TableName.AccessApprovalPolicy) // eslint-disable-next-line .where(buildFindFilter(filter)) + .where((qb) => { + if (customFilter?.policyId) { + void qb.where(`${TableName.AccessApprovalPolicy}.id`, "=", customFilter.policyId); + } + }) .join(TableName.Environment, `${TableName.AccessApprovalPolicy}.envId`, `${TableName.Environment}.id`) .leftJoin( TableName.AccessApprovalPolicyApprover, `${TableName.AccessApprovalPolicy}.id`, `${TableName.AccessApprovalPolicyApprover}.policyId` ) + .leftJoin(TableName.Users, `${TableName.AccessApprovalPolicyApprover}.approverUserId`, `${TableName.Users}.id`) + .select(tx.ref("username").withSchema(TableName.Users).as("approverUsername")) .select(tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover)) .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyApprover)) .select(tx.ref("name").withSchema(TableName.Environment).as("envName")) @@ -76,9 +89,15 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { } }; - const find = async (filter: TFindFilter, tx?: Knex) => { + const find = async ( + filter: TFindFilter, + customFilter?: { + policyId?: string; + }, + tx?: Knex + ) => { try { - const docs = await accessApprovalPolicyFindQuery(tx || db.replicaNode(), filter); + const docs = await accessApprovalPolicyFindQuery(tx || db.replicaNode(), filter, customFilter); const formattedDocs = sqlNestRelationships({ data: docs, @@ -97,9 +116,10 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => { { key: "approverUserId", label: "approvers" as const, - mapper: ({ approverUserId: id }) => ({ + mapper: ({ approverUserId: id, approverUsername }) => ({ id, - type: ApproverType.User + type: ApproverType.User, + name: approverUsername }) }, { 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 fdb242314..f947f267c 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 @@ -2,10 +2,11 @@ import { ForbiddenError } from "@casl/ability"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; -import { BadRequestError } from "@app/lib/errors"; +import { BadRequestError, NotFoundError } from "@app/lib/errors"; import { TProjectDALFactory } from "@app/services/project/project-dal"; import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal"; import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal"; +import { TUserDALFactory } from "@app/services/user/user-dal"; import { TGroupDALFactory } from "../group/group-dal"; import { TAccessApprovalPolicyApproverDALFactory } from "./access-approval-policy-approver-dal"; @@ -15,6 +16,7 @@ import { ApproverType, TCreateAccessApprovalPolicy, TDeleteAccessApprovalPolicy, + TGetAccessApprovalPolicyByIdDTO, TGetAccessPolicyCountByEnvironmentDTO, TListAccessApprovalPoliciesDTO, TUpdateAccessApprovalPolicy @@ -28,6 +30,7 @@ type TSecretApprovalPolicyServiceFactoryDep = { accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory; projectMembershipDAL: Pick; groupDAL: TGroupDALFactory; + userDAL: Pick; }; export type TAccessApprovalPolicyServiceFactory = ReturnType; @@ -38,7 +41,8 @@ export const accessApprovalPolicyServiceFactory = ({ groupDAL, permissionService, projectEnvDAL, - projectDAL + projectDAL, + userDAL }: TSecretApprovalPolicyServiceFactoryDep) => { const createAccessApprovalPolicy = async ({ name, @@ -59,12 +63,18 @@ export const accessApprovalPolicyServiceFactory = ({ // 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); + .map((approver) => approver.id) as string[]; + const userApprovers = approvers .filter((approver) => approver.type === ApproverType.User) - .map((approver) => approver.id); + .map((approver) => approver.id) + .filter(Boolean) as string[]; - if (!groupApprovers && approvals > userApprovers.length) + const userApproverNames = approvers + .map((approver) => (approver.type === ApproverType.User ? approver.name : undefined)) + .filter(Boolean) as string[]; + + if (!groupApprovers && approvals > userApprovers.length + userApproverNames.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -81,7 +91,26 @@ 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; + let approverUserIds = userApprovers; + if (userApproverNames.length) { + const approverUsers = await userDAL.find({ + $in: { + username: userApproverNames + } + }); + + const approverNamesFromDb = approverUsers.map((user) => user.username); + const invalidUsernames = userApproverNames.filter((username) => !approverNamesFromDb.includes(username)); + + if (invalidUsernames.length) { + throw new BadRequestError({ + message: `Invalid approver user: ${invalidUsernames.join(", ")}` + }); + } + + approverUserIds = approverUserIds.concat(approverUsers.map((user) => user.id)); + } + const usersPromises: Promise< { id: string; @@ -92,6 +121,7 @@ export const accessApprovalPolicyServiceFactory = ({ isPartOfGroup: boolean; }[] >[] = []; + const verifyAllApprovers = [...approverUserIds]; for (const groupId of groupApprovers) { usersPromises.push(groupDAL.findAllGroupMembers({ orgId: actorOrgId, groupId, offset: 0 })); @@ -120,9 +150,9 @@ export const accessApprovalPolicyServiceFactory = ({ }, tx ); - if (userApprovers) { + if (approverUserIds.length) { await accessApprovalPolicyApproverDAL.insertMany( - userApprovers.map((userId) => ({ + approverUserIds.map((userId) => ({ approverUserId: userId, policyId: doc.id })), @@ -182,15 +212,25 @@ export const accessApprovalPolicyServiceFactory = ({ enforcementLevel }: TUpdateAccessApprovalPolicy) => { const groupApprovers = approvers - ?.filter((approver) => approver.type === ApproverType.Group) - .map((approver) => approver.id); + .filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id) as string[]; + const userApprovers = approvers - ?.filter((approver) => approver.type === ApproverType.User) - .map((approver) => approver.id); + .filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id) + .filter(Boolean) as string[]; + + const userApproverNames = approvers + .map((approver) => (approver.type === ApproverType.User ? approver.name : undefined)) + .filter(Boolean) as string[]; const accessApprovalPolicy = await accessApprovalPolicyDAL.findById(policyId); const currentAppovals = approvals || accessApprovalPolicy.approvals; - if (groupApprovers?.length === 0 && userApprovers && currentAppovals > userApprovers.length) { + if ( + groupApprovers?.length === 0 && + userApprovers && + currentAppovals > userApprovers.length + userApproverNames.length + ) { throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); } @@ -219,7 +259,27 @@ export const accessApprovalPolicyServiceFactory = ({ await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); - if (userApprovers) { + if (userApprovers.length || userApproverNames.length) { + let userApproverIds = userApprovers; + if (userApproverNames.length) { + const approverUsers = await userDAL.find({ + $in: { + username: userApproverNames + } + }); + + const approverNamesFromDb = approverUsers.map((user) => user.username); + const invalidUsernames = userApproverNames.filter((username) => !approverNamesFromDb.includes(username)); + + if (invalidUsernames.length) { + throw new BadRequestError({ + message: `Invalid approver user: ${invalidUsernames.join(", ")}` + }); + } + + userApproverIds = userApproverIds.concat(approverUsers.map((user) => user.id)); + } + await verifyApprovers({ projectId: accessApprovalPolicy.projectId, orgId: actorOrgId, @@ -227,10 +287,11 @@ export const accessApprovalPolicyServiceFactory = ({ secretPath: doc.secretPath!, actorAuthMethod, permissionService, - userIds: userApprovers + userIds: userApproverIds }); + await accessApprovalPolicyApproverDAL.insertMany( - userApprovers.map((userId) => ({ + userApproverIds.map((userId) => ({ approverUserId: userId, policyId: doc.id })), @@ -338,11 +399,40 @@ export const accessApprovalPolicyServiceFactory = ({ return { count: policies.length }; }; + const getAccessApprovalPolicyById = async ({ + actorId, + actor, + actorOrgId, + actorAuthMethod, + policyId + }: TGetAccessApprovalPolicyByIdDTO) => { + const [policy] = await accessApprovalPolicyDAL.find({}, { policyId }); + + if (!policy) { + throw new NotFoundError({ + message: "Cannot find access approval policy" + }); + } + + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + policy.projectId, + actorAuthMethod, + actorOrgId + ); + + ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval); + + return policy; + }; + return { getAccessPolicyCountByEnvSlug, createAccessApprovalPolicy, deleteAccessApprovalPolicy, updateAccessApprovalPolicy, - getAccessApprovalPolicyByProjectSlug + getAccessApprovalPolicyByProjectSlug, + getAccessApprovalPolicyById }; }; 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 c20aabfad..bc8f23572 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 @@ -22,7 +22,7 @@ export type TCreateAccessApprovalPolicy = { approvals: number; secretPath: string; environment: string; - approvers: { type: ApproverType; id: string }[]; + approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; name?: string })[]; projectSlug: string; name: string; enforcementLevel: EnforcementLevel; @@ -31,7 +31,7 @@ export type TCreateAccessApprovalPolicy = { export type TUpdateAccessApprovalPolicy = { policyId: string; approvals?: number; - approvers?: { type: ApproverType; id: string }[]; + approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; name?: string })[]; secretPath?: string; name?: string; enforcementLevel?: EnforcementLevel; @@ -46,6 +46,10 @@ export type TGetAccessPolicyCountByEnvironmentDTO = { projectSlug: string; } & Omit; +export type TGetAccessApprovalPolicyByIdDTO = { + policyId: string; +} & Omit; + export type TListAccessApprovalPoliciesDTO = { projectSlug: string; } & Omit; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index f1c51f8b5..45a08b36b 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -928,7 +928,8 @@ export const registerRoutes = async ( permissionService, projectEnvDAL, projectMembershipDAL, - projectDAL + projectDAL, + userDAL }); const accessApprovalRequestService = accessApprovalRequestServiceFactory({ From adaddad3706c4802ead9c7aed123b78a5e1bf308 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Wed, 25 Sep 2024 18:46:44 +0800 Subject: [PATCH 4/5] misc: added rate limiting --- .../ee/routes/v1/access-approval-policy-router.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) 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 49d1c7765..814d19841 100644 --- a/backend/src/ee/routes/v1/access-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/access-approval-policy-router.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import { ApproverType } from "@app/ee/services/access-approval-policy/access-approval-policy-types"; import { EnforcementLevel } from "@app/lib/types"; -import { readLimit } from "@app/server/config/rateLimiter"; +import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { sapPubSchema } from "@app/server/routes/sanitizedSchemas"; import { AuthMode } from "@app/services/auth/auth-type"; @@ -12,6 +12,9 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi server.route({ url: "/", method: "POST", + config: { + rateLimit: writeLimit + }, schema: { body: z.object({ projectSlug: z.string().trim(), @@ -53,6 +56,9 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi server.route({ url: "/", method: "GET", + config: { + rateLimit: readLimit + }, schema: { querystring: z.object({ projectSlug: z.string().trim() @@ -119,6 +125,9 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi server.route({ url: "/:policyId", method: "PATCH", + config: { + rateLimit: writeLimit + }, schema: { params: z.object({ policyId: z.string() @@ -162,6 +171,9 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi server.route({ url: "/:policyId", method: "DELETE", + config: { + rateLimit: writeLimit + }, schema: { params: z.object({ policyId: z.string() From bf8e1f2bfd1904dd45dea57776f54a7d41aeedc7 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Wed, 25 Sep 2024 21:36:28 +0800 Subject: [PATCH 5/5] misc: added missing filter --- .../access-approval-policy/access-approval-policy-service.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 f3c9c6aaa..b9019b130 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 @@ -317,7 +317,10 @@ export const accessApprovalPolicyServiceFactory = ({ for (const groupId of groupApprovers) { usersPromises.push(groupDAL.findAllGroupPossibleMembers({ orgId: actorOrgId, groupId, offset: 0 })); } - const verifyGroupApprovers = (await Promise.all(usersPromises)).flat().map((user) => user.id); + const verifyGroupApprovers = (await Promise.all(usersPromises)) + .flat() + .filter((user) => user.isPartOfGroup) + .map((user) => user.id); await verifyApprovers({ projectId: accessApprovalPolicy.projectId,