From 52cf38449b12a2f1837a11f3c3ce9f33e09ff766 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Fri, 19 Apr 2024 03:08:55 +0200 Subject: [PATCH] Chore: Documentation --- .../secret-snapshot-service.ts | 30 +++++++++++++++++-- .../services/secret-snapshot/snapshot-dal.ts | 2 ++ .../secret-snapshot/snapshot-service-fns.ts | 28 +++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 backend/src/ee/services/secret-snapshot/snapshot-service-fns.ts diff --git a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts index de2f6efcb..0e71ad126 100644 --- a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts +++ b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts @@ -1,4 +1,4 @@ -import { ForbiddenError } from "@casl/ability"; +import { ForbiddenError, subject } from "@casl/ability"; import { TableName, TSecretTagJunctionInsert } from "@app/db/schemas"; import { BadRequestError, InternalServerError } from "@app/lib/errors"; @@ -23,6 +23,7 @@ import { import { TSnapshotDALFactory } from "./snapshot-dal"; import { TSnapshotFolderDALFactory } from "./snapshot-folder-dal"; import { TSnapshotSecretDALFactory } from "./snapshot-secret-dal"; +import { getFullFolderPath } from "./snapshot-service-fns"; type TSecretSnapshotServiceFactoryDep = { snapshotDAL: TSnapshotDALFactory; @@ -33,7 +34,7 @@ type TSecretSnapshotServiceFactoryDep = { secretDAL: Pick; secretTagDAL: Pick; secretVersionTagDAL: Pick; - folderDAL: Pick; + folderDAL: Pick; permissionService: Pick; licenseService: Pick; }; @@ -71,6 +72,12 @@ export const secretSnapshotServiceFactory = ({ ); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); + // We need to check if the user has access to the secrets in the folder. If we don't do this, a user could theoretically access snapshot secret values even if they don't have read access to the secrets in the folder. + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) + ); + const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) throw new BadRequestError({ message: "Folder not found" }); @@ -98,6 +105,12 @@ export const secretSnapshotServiceFactory = ({ ); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); + // We need to check if the user has access to the secrets in the folder. If we don't do this, a user could theoretically access snapshot secret values even if they don't have read access to the secrets in the folder. + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) + ); + const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) throw new BadRequestError({ message: "Folder not found" }); @@ -116,6 +129,19 @@ export const secretSnapshotServiceFactory = ({ actorOrgId ); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); + + const fullFolderPath = await getFullFolderPath({ + folderDAL, + folderId: snapshot.folderId, + envId: snapshot.environment.id + }); + + // We need to check if the user has access to the secrets in the folder. If we don't do this, a user could theoretically access snapshot secret values even if they don't have read access to the secrets in the folder. + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment: snapshot.environment.slug, secretPath: fullFolderPath }) + ); + return snapshot; }; diff --git a/backend/src/ee/services/secret-snapshot/snapshot-dal.ts b/backend/src/ee/services/secret-snapshot/snapshot-dal.ts index 41524c6eb..cdd5a999b 100644 --- a/backend/src/ee/services/secret-snapshot/snapshot-dal.ts +++ b/backend/src/ee/services/secret-snapshot/snapshot-dal.ts @@ -101,6 +101,7 @@ export const snapshotDALFactory = (db: TDbClient) => { key: "snapshotId", parentMapper: ({ snapshotId: id, + folderId, projectId, envId, envSlug, @@ -109,6 +110,7 @@ export const snapshotDALFactory = (db: TDbClient) => { snapshotUpdatedAt: updatedAt }) => ({ id, + folderId, projectId, createdAt, updatedAt, diff --git a/backend/src/ee/services/secret-snapshot/snapshot-service-fns.ts b/backend/src/ee/services/secret-snapshot/snapshot-service-fns.ts new file mode 100644 index 000000000..51cb9c056 --- /dev/null +++ b/backend/src/ee/services/secret-snapshot/snapshot-service-fns.ts @@ -0,0 +1,28 @@ +import { TSecretFolderDALFactory } from "@app/services/secret-folder/secret-folder-dal"; + +type GetFullFolderPath = { + folderDAL: Pick; // Added findAllInEnv + folderId: string; + envId: string; +}; + +export const getFullFolderPath = async ({ folderDAL, folderId, envId }: GetFullFolderPath): Promise => { + // Helper function to remove duplicate slashes + const removeDuplicateSlashes = (path: string) => path.replace(/\/{2,}/g, "/"); + + // Fetch all folders at once based on environment ID to avoid multiple queries + const folders = await folderDAL.find({ envId }); + const folderMap = new Map(folders.map((folder) => [folder.id, folder])); + + const buildPath = (currFolderId: string): string => { + const folder = folderMap.get(currFolderId); + if (!folder) return ""; + const folderPathSegment = !folder.parentId && folder.name === "root" ? "/" : `/${folder.name}`; + if (folder.parentId) { + return removeDuplicateSlashes(`${buildPath(folder.parentId)}${folderPathSegment}`); + } + return removeDuplicateSlashes(folderPathSegment); + }; + + return buildPath(folderId); +};