From 43735b818302fafb432edb71303c032e0ca2a98d Mon Sep 17 00:00:00 2001 From: Akhil Mohan Date: Fri, 8 Sep 2023 13:28:54 +0530 Subject: [PATCH] feat(rbac): updated role controller to check permissions and batch v2 selectively permission check --- backend/src/controllers/v1/roleController.ts | 61 ++++++++++++++----- .../src/controllers/v2/secretsController.ts | 61 +++++++++++++------ .../src/views/DashboardPage/DashboardPage.tsx | 2 + .../SecretInputRow/SecretInputRow.tsx | 4 ++ 4 files changed, 94 insertions(+), 34 deletions(-) diff --git a/backend/src/controllers/v1/roleController.ts b/backend/src/controllers/v1/roleController.ts index e2e4e30e6..77ff7cadf 100644 --- a/backend/src/controllers/v1/roleController.ts +++ b/backend/src/controllers/v1/roleController.ts @@ -8,6 +8,8 @@ import { UpdateRoleSchema } from "../../validation"; import { + ProjectPermissionActions, + ProjectPermissionSub, adminProjectPermissions, getUserProjectPermissions, memberProjectPermissions, @@ -30,12 +32,18 @@ export const createRole = async (req: Request, res: Response) => { body: { workspaceId, name, description, slug, permissions, orgId } } = await validateRequest(CreateRoleSchema, req); - const { permission } = await getUserOrgPermissions(req.user.id, orgId); - if (permission.cannot(OrgPermissionActions.Create, OrgPermissionSubjects.Role)) { - throw BadRequestError({ message: "User doesn't have the permission." }); - } - const isOrgRole = !workspaceId; // if workspaceid is provided then its a workspace rule + if (isOrgRole) { + const { permission } = await getUserOrgPermissions(req.user.id, orgId); + if (permission.cannot(OrgPermissionActions.Create, OrgPermissionSubjects.Role)) { + throw BadRequestError({ message: "user doesn't have the permission." }); + } + } else { + const { permission } = await getUserProjectPermissions(req.user.id, workspaceId); + if (permission.cannot(ProjectPermissionActions.Create, ProjectPermissionSub.Role)) { + throw BadRequestError({ message: "User doesn't have the permission." }); + } + } const existingRole = await Role.findOne({ organization: orgId, workspace: workspaceId, slug }); if (existingRole) { @@ -68,9 +76,16 @@ export const updateRole = async (req: Request, res: Response) => { } = await validateRequest(UpdateRoleSchema, req); const isOrgRole = !workspaceId; // if workspaceid is provided then its a workspace rule - const { permission } = await getUserOrgPermissions(req.user.id, orgId); - if (permission.cannot(OrgPermissionActions.Edit, OrgPermissionSubjects.Role)) { - throw BadRequestError({ message: "User doesn't have the permission." }); + if (isOrgRole) { + const { permission } = await getUserOrgPermissions(req.user.id, orgId); + if (permission.cannot(OrgPermissionActions.Edit, OrgPermissionSubjects.Role)) { + throw BadRequestError({ message: "User doesn't have the org permission." }); + } + } else { + const { permission } = await getUserProjectPermissions(req.user.id, workspaceId); + if (permission.cannot(ProjectPermissionActions.Edit, ProjectPermissionSub.Role)) { + throw BadRequestError({ message: "User doesn't have the workspace permission." }); + } } if (slug) { @@ -112,10 +127,19 @@ export const deleteRole = async (req: Request, res: Response) => { throw BadRequestError({ message: "Role not found" }); } - const { permission } = await getUserOrgPermissions(req.user.id, role.organization.toString()); - if (permission.cannot(OrgPermissionActions.Delete, OrgPermissionSubjects.Role)) { - throw BadRequestError({ message: "User doesn't have the permission." }); + const isOrgRole = !role.workspace; + if (isOrgRole) { + const { permission } = await getUserOrgPermissions(req.user.id, role.organization.toString()); + if (permission.cannot(OrgPermissionActions.Delete, OrgPermissionSubjects.Role)) { + throw BadRequestError({ message: "User doesn't have the org permission." }); + } + } else { + const { permission } = await getUserProjectPermissions(req.user.id, role.workspace.toString()); + if (permission.cannot(ProjectPermissionActions.Delete, ProjectPermissionSub.Role)) { + throw BadRequestError({ message: "User doesn't have the workspace permission." }); + } } + await Role.findByIdAndDelete(role.id); res.status(200).json({ @@ -130,11 +154,18 @@ export const getRoles = async (req: Request, res: Response) => { const { query: { workspaceId, orgId } } = await validateRequest(GetRoleSchema, req); - const isOrgRole = !workspaceId; - const { permission } = await getUserOrgPermissions(req.user.id, orgId); - if (permission.cannot(OrgPermissionActions.Read, OrgPermissionSubjects.Role)) { - throw BadRequestError({ message: "User doesn't have the permission." }); + const isOrgRole = !workspaceId; + if (isOrgRole) { + const { permission } = await getUserOrgPermissions(req.user.id, orgId); + if (permission.cannot(OrgPermissionActions.Read, OrgPermissionSubjects.Role)) { + throw BadRequestError({ message: "User doesn't have the org permission." }); + } + } else { + const { permission } = await getUserProjectPermissions(req.user.id, workspaceId); + if (permission.cannot(ProjectPermissionActions.Read, ProjectPermissionSub.Role)) { + throw BadRequestError({ message: "User doesn't have the workspace permission." }); + } } const customRoles = await Role.find({ organization: orgId, isOrgRole, workspace: workspaceId }); diff --git a/backend/src/controllers/v2/secretsController.ts b/backend/src/controllers/v2/secretsController.ts index 4aa95d574..84b3af681 100644 --- a/backend/src/controllers/v2/secretsController.ts +++ b/backend/src/controllers/v2/secretsController.ts @@ -65,6 +65,20 @@ export const batchSecrets = async (req: Request, res: Response) => { body: { secretPath, folderId } } = validatedData; + const secretIds = requests + .filter(({ method }) => method !== "POST") + // akhilmhdh: ts is dumb + .map((el) => new Types.ObjectId((el.secret as any)._id)); + + const oldSecrets = await Secret.find({ + _id: { + $in: secretIds + } + }); + if (oldSecrets.length != secretIds.length) { + throw BadRequestError({ message: "Failed to validate non-existent secrets" }); + } + const createSecrets: any[] = []; const updateSecrets: any[] = []; const deleteSecrets: { _id: Types.ObjectId; secretName: string }[] = []; @@ -98,20 +112,6 @@ export const batchSecrets = async (req: Request, res: Response) => { secretPath, requiredPermissions: [PERMISSION_WRITE_SECRETS] }); - } else { - const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Create, - subject(ProjectPermissionSub.Secrets, { environment, secretPath }) - ); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Edit, - subject(ProjectPermissionSub.Secrets, { environment, secretPath }) - ); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Delete, - subject(ProjectPermissionSub.Secrets, { environment, secretPath }) - ); } for await (const request of requests) { @@ -160,6 +160,27 @@ export const batchSecrets = async (req: Request, res: Response) => { break; } } + // not using service token using auth + if (!(req.authData.authPayload instanceof ServiceTokenData)) { + const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); + if (!createSecrets.length) + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Create, + subject(ProjectPermissionSub.Secrets, { environment, secretPath }) + ); + + if (!updateSecrets.length) + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Edit, + subject(ProjectPermissionSub.Secrets, { environment, secretPath }) + ); + + if (!deleteSecrets.length) + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Delete, + subject(ProjectPermissionSub.Secrets, { environment, secretPath }) + ); + } // handle create secrets let createdSecrets: ISecret[] = []; @@ -229,7 +250,7 @@ export const batchSecrets = async (req: Request, res: Response) => { // handle update secrets let updatedSecrets: ISecret[] = []; - if (updateSecrets.length > 0 && req.secrets) { + if (updateSecrets.length > 0 && oldSecrets) { // construct object containing all secrets let listedSecretsObj: { [key: string]: { @@ -238,7 +259,7 @@ export const batchSecrets = async (req: Request, res: Response) => { }; } = {}; - listedSecretsObj = req.secrets.reduce( + listedSecretsObj = oldSecrets.reduce( (obj: any, secret: ISecret) => ({ ...obj, [secret._id.toString()]: secret @@ -250,7 +271,8 @@ export const batchSecrets = async (req: Request, res: Response) => { updateOne: { filter: { _id: new Types.ObjectId(u._id), - workspace: new Types.ObjectId(workspaceId) + workspace: new Types.ObjectId(workspaceId), + environment }, update: { $inc: { @@ -264,7 +286,6 @@ export const batchSecrets = async (req: Request, res: Response) => { } } })); - await Secret.bulkWrite(updateOperations); const secretVersions = updateSecrets.map( @@ -372,7 +393,9 @@ export const batchSecrets = async (req: Request, res: Response) => { await Secret.deleteMany({ _id: { $in: deleteSecretIds - } + }, + workspace: new Types.ObjectId(workspaceId), + environment }); await EESecretService.markDeletedSecretVersions({ diff --git a/frontend/src/views/DashboardPage/DashboardPage.tsx b/frontend/src/views/DashboardPage/DashboardPage.tsx index d8b04eac0..46cb89835 100644 --- a/frontend/src/views/DashboardPage/DashboardPage.tsx +++ b/frontend/src/views/DashboardPage/DashboardPage.tsx @@ -451,6 +451,7 @@ export const DashboardPage = () => { await onSecretRollback(); return; } + console.log(userSec); // just closing this if save is triggered from drawer handlePopUpClose("secretDetails"); // encrypt and format the secrets to batch api format @@ -466,6 +467,7 @@ export const DashboardPage = () => { reset(); return; } + console.log(batchedSecret); try { await batchSecretOp({ requests: batchedSecret, diff --git a/frontend/src/views/DashboardPage/components/SecretInputRow/SecretInputRow.tsx b/frontend/src/views/DashboardPage/components/SecretInputRow/SecretInputRow.tsx index c31c6df2e..745e98878 100644 --- a/frontend/src/views/DashboardPage/components/SecretInputRow/SecretInputRow.tsx +++ b/frontend/src/views/DashboardPage/components/SecretInputRow/SecretInputRow.tsx @@ -304,6 +304,10 @@ export const SecretInputRow = memo( (isOverridden ? isAddOnly : shouldBeBlockedInAddOnly) } {...field} + onChange={(val) => { + console.log(val); + field.onChange(val); + }} /> )} />