From bcfe1bda84553204bdec7094f2f7aed314e06ca4 Mon Sep 17 00:00:00 2001 From: Akhil Mohan Date: Tue, 29 Aug 2023 20:14:24 +0530 Subject: [PATCH] feat(rbac): made new permission check for v3 secrets and v2 batch --- .../src/controllers/v2/secretsController.ts | 58 +++++++--- .../src/controllers/v3/secretsController.ts | 104 ++++++++++++++++-- backend/src/helpers/secrets.ts | 29 +---- backend/src/routes/v2/secrets.ts | 4 - backend/src/routes/v3/secrets.ts | 84 +------------- backend/src/validation/serviceTokenData.ts | 8 +- 6 files changed, 150 insertions(+), 137 deletions(-) diff --git a/backend/src/controllers/v2/secretsController.ts b/backend/src/controllers/v2/secretsController.ts index 24be055aa..b703c9d50 100644 --- a/backend/src/controllers/v2/secretsController.ts +++ b/backend/src/controllers/v2/secretsController.ts @@ -35,7 +35,17 @@ import { isValidScope } from "../../helpers/secrets"; import path from "path"; import { getAllImportedSecrets } from "../../services/SecretImportService"; import { validateRequest } from "../../helpers/validation"; -import { BatchSecretsV2, GetSecretsV2 } from "../../validation"; +import { + BatchSecretsV2, + GetSecretsV2, + validateServiceTokenDataClientForWorkspace +} from "../../validation"; +import { + ProjectPermissionActions, + ProjectPermissionSub, + getUserProjectPermissions +} from "../../services/ProjectRoleService"; +import { ForbiddenError, subject } from "@casl/ability"; /** * Peform a batch of any specified CUD secret operations @@ -68,17 +78,13 @@ export const batchSecrets = async (req: Request, res: Response) => { const folders = await Folder.findOne({ workspace: workspaceId, environment }); if (req.authData.authPayload instanceof ServiceTokenData) { - const isValidScopeAccess = isValidScope( - req.authData.authPayload, + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload, + workspaceId: new Types.ObjectId(workspaceId), environment, - secretPath || "/" - ); - - // in service token when not giving secretpath folderid must be root - // this is to avoid giving folderid when service tokens are used - if ((!secretPath && folderId !== "root") || (secretPath && !isValidScopeAccess)) { - throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); - } + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } if (secretPath) { @@ -94,6 +100,22 @@ export const batchSecrets = async (req: Request, res: Response) => { ); } + if (req.user?._id) { + 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) { // do a validation @@ -977,17 +999,17 @@ export const getSecrets = async (req: Request, res: Response) => { const postHogClient = await TelemetryService.getPostHogClient(); // reduce the number of events captured - let shouldRecordK8Event = false + let shouldRecordK8Event = false; if (req.authData.userAgent == K8_USER_AGENT_NAME) { const randomNumber = Math.random(); if (randomNumber > 0.9) { - shouldRecordK8Event = true + shouldRecordK8Event = true; } } if (postHogClient) { const shouldCapture = req.authData.userAgent !== K8_USER_AGENT_NAME || shouldRecordK8Event; - const approximateForNoneCapturedEvents = secrets.length * 10 + const approximateForNoneCapturedEvents = secrets.length * 10; if (shouldCapture) { postHogClient.capture({ @@ -1111,10 +1133,10 @@ export const updateSecrets = async (req: Request, res: Response) => { tags, ...(secretCommentCiphertext !== undefined && secretCommentIV && secretCommentTag ? { - secretCommentCiphertext, - secretCommentIV, - secretCommentTag - } + secretCommentCiphertext, + secretCommentIV, + secretCommentTag + } : {}) } } diff --git a/backend/src/controllers/v3/secretsController.ts b/backend/src/controllers/v3/secretsController.ts index 93e3da885..ca94bc9ce 100644 --- a/backend/src/controllers/v3/secretsController.ts +++ b/backend/src/controllers/v3/secretsController.ts @@ -17,6 +17,8 @@ import { getUserProjectPermissions } from "../../services/ProjectRoleService"; import { ForbiddenError, subject } from "@casl/ability"; +import { validateServiceTokenDataClientForWorkspace } from "../../validation"; +import { PERMISSION_READ_SECRETS, PERMISSION_WRITE_SECRETS } from "../../variables"; /** * Return secrets for workspace with id [workspaceId] and environment @@ -40,12 +42,22 @@ export const getSecretsRaw = async (req: Request, res: Response) => { secretPath = scope.secretPath; environment = scope.environment; workspaceId = serviceTokenDetails.workspace.toString(); - } else { + } + + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Read, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_READ_SECRETS] + }); } const secrets = await SecretService.getSecrets({ @@ -108,12 +120,20 @@ export const getSecretByNameRaw = async (req: Request, res: Response) => { params: { secretName } } = await validateRequest(reqValidator.GetSecretByNameRawV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Read, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_READ_SECRETS] + }); } const secret = await SecretService.getSecret({ @@ -148,12 +168,20 @@ export const createSecretRaw = async (req: Request, res: Response) => { body: { secretPath, environment, workspaceId, type, secretValue, secretComment } } = await validateRequest(reqValidator.CreateSecretRawV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Create, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const key = await BotService.getWorkspaceKeyWithBot({ @@ -223,12 +251,20 @@ export const updateSecretByNameRaw = async (req: Request, res: Response) => { body: { secretValue, environment, secretPath, type, workspaceId } } = await validateRequest(reqValidator.UpdateSecretByNameRawV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Edit, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const key = await BotService.getWorkspaceKeyWithBot({ @@ -279,12 +315,20 @@ export const deleteSecretByNameRaw = async (req: Request, res: Response) => { body: { environment, secretPath, type, workspaceId } } = await validateRequest(reqValidator.DeleteSecretByNameRawV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Delete, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const { secret } = await SecretService.deleteSecret({ @@ -327,12 +371,20 @@ export const getSecrets = async (req: Request, res: Response) => { query: { secretPath, environment, workspaceId, include_imports: includeImports,folderId } } = await validateRequest(reqValidator.GetSecretsV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Read, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_READ_SECRETS] + }); } const secrets = await SecretService.getSecrets({ @@ -377,12 +429,20 @@ export const getSecretByName = async (req: Request, res: Response) => { params: { secretName } } = await validateRequest(reqValidator.GetSecretByNameV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Read, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_READ_SECRETS] + }); } const secret = await SecretService.getSecret({ @@ -425,12 +485,20 @@ export const createSecret = async (req: Request, res: Response) => { } } = await validateRequest(reqValidator.CreateSecretV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Create, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const secret = await SecretService.createSecret({ @@ -487,12 +555,20 @@ export const updateSecretByName = async (req: Request, res: Response) => { params: { secretName } } = await validateRequest(reqValidator.UpdateSecretByNameV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Edit, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const secret = await SecretService.updateSecret({ @@ -531,12 +607,20 @@ export const deleteSecretByName = async (req: Request, res: Response) => { params: { secretName } } = await validateRequest(reqValidator.DeleteSecretByNameV3, req); - if (req.user._id) { + if (req.user?._id) { const { permission } = await getUserProjectPermissions(req.user._id, workspaceId); ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Delete, subject(ProjectPermissionSub.Secrets, { environment, secretPath }) ); + } else { + await validateServiceTokenDataClientForWorkspace({ + serviceTokenData: req.authData.authPayload as IServiceTokenData, + workspaceId: new Types.ObjectId(workspaceId), + environment, + secretPath, + requiredPermissions: [PERMISSION_WRITE_SECRETS] + }); } const { secret } = await SecretService.deleteSecret({ diff --git a/backend/src/helpers/secrets.ts b/backend/src/helpers/secrets.ts index ad6296adf..510da7dd9 100644 --- a/backend/src/helpers/secrets.ts +++ b/backend/src/helpers/secrets.ts @@ -504,11 +504,6 @@ export const getSecretsHelper = async ({ }: GetSecretsParams) => { let secrets: ISecret[] = []; // if using service token filter towards the folderId by secretpath - if (authData.authPayload instanceof ServiceTokenData) { - if (!isValidScope(authData.authPayload, environment, secretPath)) { - throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); - } - } if (!folderId) { folderId = await getFolderIdFromServiceToken(workspaceId, environment, secretPath); @@ -575,11 +570,11 @@ export const getSecretsHelper = async ({ const postHogClient = await TelemetryService.getPostHogClient(); // reduce the number of events captured - let shouldRecordK8Event = false + let shouldRecordK8Event = false; if (authData.userAgent == K8_USER_AGENT_NAME) { const randomNumber = Math.random(); if (randomNumber > 0.9) { - shouldRecordK8Event = true + shouldRecordK8Event = true; } } @@ -588,7 +583,7 @@ export const getSecretsHelper = async ({ if (postHogClient && atLeastOneNonSignUpSecret) { const shouldCapture = authData.userAgent !== K8_USER_AGENT_NAME || shouldRecordK8Event; - const approximateForNoneCapturedEvents = secrets.length * 10 + const approximateForNoneCapturedEvents = secrets.length * 10; if (shouldCapture) { postHogClient.capture({ @@ -633,11 +628,7 @@ export const getSecretHelper = async ({ }); let secret: ISecret | null = null; // if using service token filter towards the folderId by secretpath - if (authData.authPayload instanceof ServiceTokenData) { - if (!isValidScope(authData.authPayload, environment, secretPath)) { - throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); - } - } + const folderId = await getFolderIdFromServiceToken(workspaceId, environment, secretPath); // try getting personal secret first (if exists) @@ -751,12 +742,6 @@ export const updateSecretHelper = async ({ }); let secret: ISecret | null = null; - // if using service token filter towards the folderId by secretpath - if (authData.authPayload instanceof ServiceTokenData) { - if (!isValidScope(authData.authPayload, environment, secretPath)) { - throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); - } - } const folderId = await getFolderIdFromServiceToken(workspaceId, environment, secretPath); if (type === SECRET_SHARED) { @@ -916,12 +901,6 @@ export const deleteSecretHelper = async ({ workspaceId: new Types.ObjectId(workspaceId) }); - // if using service token filter towards the folderId by secretpath - if (authData.authPayload instanceof ServiceTokenData) { - if (!isValidScope(authData.authPayload, environment, secretPath)) { - throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); - } - } const folderId = await getFolderIdFromServiceToken(workspaceId, environment, secretPath); let secrets: ISecret[] = []; diff --git a/backend/src/routes/v2/secrets.ts b/backend/src/routes/v2/secrets.ts index 3a8395bc6..c175335ff 100644 --- a/backend/src/routes/v2/secrets.ts +++ b/backend/src/routes/v2/secrets.ts @@ -24,10 +24,6 @@ router.post( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body" - }), secretsController.batchSecrets ); diff --git a/backend/src/routes/v3/secrets.ts b/backend/src/routes/v3/secrets.ts index 338c72fe7..1765c2e33 100644 --- a/backend/src/routes/v3/secrets.ts +++ b/backend/src/routes/v3/secrets.ts @@ -1,22 +1,13 @@ import express from "express"; const router = express.Router(); -import { - requireAuth, - requireBlindIndicesEnabled, - requireE2EEOff, - requireWorkspaceAuth, - validateRequest +import { + requireAuth, + requireBlindIndicesEnabled, + requireE2EEOff } from "../../middleware"; -import { body, param, query } from "express-validator"; import { secretsController } from "../../controllers/v3"; import { - ADMIN, - AuthMode, - MEMBER, - PERMISSION_READ_SECRETS, - PERMISSION_WRITE_SECRETS, - SECRET_PERSONAL, - SECRET_SHARED + AuthMode } from "../../variables"; router.get( @@ -32,12 +23,6 @@ router.get( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "query", - locationEnvironment: "query", - requiredPermissions: [PERMISSION_READ_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "query" }), @@ -52,12 +37,6 @@ router.post( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), @@ -72,12 +51,6 @@ router.patch( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), @@ -92,12 +65,6 @@ router.delete( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), @@ -109,20 +76,9 @@ router.delete( router.get( "/", - query("workspaceId").exists().isString().trim(), - query("environment").exists().isString().trim(), - query("folderId").optional().isString().trim(), - query("secretPath").default("/").isString().trim(), - validateRequest, requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "query", - locationEnvironment: "query", - requiredPermissions: [PERMISSION_READ_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "query" }), @@ -134,12 +90,6 @@ router.post( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), @@ -151,12 +101,6 @@ router.get( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "query", - locationEnvironment: "query", - requiredPermissions: [PERMISSION_READ_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "query" }), @@ -168,12 +112,6 @@ router.patch( requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), @@ -182,21 +120,9 @@ router.patch( router.delete( "/:secretName", - param("secretName").exists().isString().trim(), - body("workspaceId").exists().isString().trim(), - body("environment").exists().isString().trim(), - body("secretPath").default("/").isString().trim(), - body("type").exists().isIn([SECRET_SHARED, SECRET_PERSONAL]), - validateRequest, requireAuth({ acceptedAuthModes: [AuthMode.JWT, AuthMode.API_KEY, AuthMode.SERVICE_TOKEN] }), - requireWorkspaceAuth({ - acceptedRoles: [ADMIN, MEMBER], - locationWorkspaceId: "body", - locationEnvironment: "body", - requiredPermissions: [PERMISSION_WRITE_SECRETS] - }), requireBlindIndicesEnabled({ locationWorkspaceId: "body" }), diff --git a/backend/src/validation/serviceTokenData.ts b/backend/src/validation/serviceTokenData.ts index e7702306f..1379b8f97 100644 --- a/backend/src/validation/serviceTokenData.ts +++ b/backend/src/validation/serviceTokenData.ts @@ -5,6 +5,7 @@ import { validateUserClientForWorkspace } from "./user"; import { ActorType } from "../ee/models"; import { AuthData } from "../interfaces/middleware"; import { z } from "zod"; +import { isValidScope } from "../helpers"; /** * Validate authenticated clients for service token with id [serviceTokenId] based @@ -62,11 +63,13 @@ export const validateServiceTokenDataClientForWorkspace = async ({ serviceTokenData, workspaceId, environment, + secretPath = "/", requiredPermissions }: { serviceTokenData: IServiceTokenData; workspaceId: Types.ObjectId; environment?: string; + secretPath?: string; requiredPermissions?: string[]; }) => { if (!serviceTokenData.workspace.equals(workspaceId)) { @@ -78,7 +81,6 @@ export const validateServiceTokenDataClientForWorkspace = async ({ if (environment) { // case: environment is specified - if (!serviceTokenData.scopes.find(({ environment: tkEnv }) => tkEnv === environment)) { // case: invalid environment passed throw UnauthorizedRequestError({ @@ -86,6 +88,10 @@ export const validateServiceTokenDataClientForWorkspace = async ({ }); } + if (!isValidScope(serviceTokenData, environment, secretPath)) { + throw UnauthorizedRequestError({ message: "Folder Permission Denied" }); + } + requiredPermissions?.forEach((permission) => { if (!serviceTokenData.permissions.includes(permission)) { throw UnauthorizedRequestError({