From 2a0c0590f1691d862d717c231e69bec0b7abc244 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Wed, 19 Feb 2025 07:37:51 +0400 Subject: [PATCH] fix: cleanup and bug fixes --- .../secret-snapshot-service.ts | 116 +++++++++++------- .../components/v2/SecretInput/SecretInput.tsx | 1 - .../api/secretApprovalRequest/queries.tsx | 1 + .../src/hooks/api/secretSnapshots/queries.tsx | 1 + frontend/src/hooks/api/secrets/types.ts | 1 + .../CreateSecretForm/CreateSecretForm.tsx | 3 +- .../SecretRenameRow.tsx | 12 +- .../SelectionPanel/SelectionPanel.tsx | 5 +- .../MoveSecretsDialog/MoveSecretsDialog.tsx | 5 +- .../SecretDashboardPage.tsx | 2 - 10 files changed, 86 insertions(+), 61 deletions(-) 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 624b82d77..b7a972d02 100644 --- a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts +++ b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts @@ -165,6 +165,7 @@ export const secretSnapshotServiceFactory = ({ }); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); + const shouldUseBridge = snapshot.projectVersion === 3; let snapshotDetails; if (shouldUseBridge) { @@ -173,68 +174,93 @@ export const secretSnapshotServiceFactory = ({ projectId: snapshot.projectId }); const encryptedSnapshotDetails = await snapshotDAL.findSecretSnapshotV2DataById(id); + + const fullFolderPath = await getFullFolderPath({ + folderDAL, + folderId: encryptedSnapshotDetails.folderId, + envId: encryptedSnapshotDetails.environment.id + }); + snapshotDetails = { ...encryptedSnapshotDetails, - secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => ({ - ...el, - secretKey: el.key, - secretValue: el.encryptedValue - ? secretManagerDecryptor({ cipherTextBlob: el.encryptedValue }).toString() - : "", - secretComment: el.encryptedComment - ? secretManagerDecryptor({ cipherTextBlob: el.encryptedComment }).toString() - : "" - })) + secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretActions.ReadValue, + subject(ProjectPermissionSub.Secrets, { + environment: encryptedSnapshotDetails.environment.slug, + secretPath: fullFolderPath, + secretName: el.key, + secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined + }) + ); + + return { + ...el, + secretKey: el.key, + secretValue: el.encryptedValue + ? secretManagerDecryptor({ cipherTextBlob: el.encryptedValue }).toString() + : "", + secretComment: el.encryptedComment + ? secretManagerDecryptor({ cipherTextBlob: el.encryptedComment }).toString() + : "" + }; + }) }; } else { const encryptedSnapshotDetails = await snapshotDAL.findSecretSnapshotDataById(id); + + const fullFolderPath = await getFullFolderPath({ + folderDAL, + folderId: encryptedSnapshotDetails.folderId, + envId: encryptedSnapshotDetails.environment.id + }); + const { botKey } = await projectBotService.getBotKey(snapshot.projectId); if (!botKey) throw new NotFoundError({ message: `Project bot key not found for project with ID '${snapshot.projectId}'` }); snapshotDetails = { ...encryptedSnapshotDetails, - secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => ({ - ...el, - secretKey: decryptSymmetric128BitHexKeyUTF8({ + secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => { + const secretKey = decryptSymmetric128BitHexKeyUTF8({ ciphertext: el.secretKeyCiphertext, iv: el.secretKeyIV, tag: el.secretKeyTag, key: botKey - }), - secretValue: decryptSymmetric128BitHexKeyUTF8({ - ciphertext: el.secretValueCiphertext, - iv: el.secretValueIV, - tag: el.secretValueTag, - key: botKey - }), - secretComment: - el.secretCommentTag && el.secretCommentIV && el.secretCommentCiphertext - ? decryptSymmetric128BitHexKeyUTF8({ - ciphertext: el.secretCommentCiphertext, - iv: el.secretCommentIV, - tag: el.secretCommentTag, - key: botKey - }) - : "" - })) + }); + + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretActions.ReadValue, + subject(ProjectPermissionSub.Secrets, { + environment: encryptedSnapshotDetails.environment.slug, + secretPath: fullFolderPath, + secretName: secretKey, + secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined + }) + ); + + return { + ...el, + secretKey, + secretValue: decryptSymmetric128BitHexKeyUTF8({ + ciphertext: el.secretValueCiphertext, + iv: el.secretValueIV, + tag: el.secretValueTag, + key: botKey + }), + secretComment: + el.secretCommentTag && el.secretCommentIV && el.secretCommentCiphertext + ? decryptSymmetric128BitHexKeyUTF8({ + ciphertext: el.secretCommentCiphertext, + iv: el.secretCommentIV, + tag: el.secretCommentTag, + key: botKey + }) + : "" + }; + }) }; } - const fullFolderPath = await getFullFolderPath({ - folderDAL, - folderId: snapshotDetails.folderId, - envId: snapshotDetails.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( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: snapshotDetails.environment.slug, - secretPath: fullFolderPath - }) - ); - return snapshotDetails; }; diff --git a/frontend/src/components/v2/SecretInput/SecretInput.tsx b/frontend/src/components/v2/SecretInput/SecretInput.tsx index 573e1c60a..5bf6e83b7 100644 --- a/frontend/src/components/v2/SecretInput/SecretInput.tsx +++ b/frontend/src/components/v2/SecretInput/SecretInput.tsx @@ -1,6 +1,5 @@ /* eslint-disable react/no-danger */ import { forwardRef, TextareaHTMLAttributes } from "react"; - import { twMerge } from "tailwind-merge"; import { useToggle } from "@app/hooks"; diff --git a/frontend/src/hooks/api/secretApprovalRequest/queries.tsx b/frontend/src/hooks/api/secretApprovalRequest/queries.tsx index 74acf744b..39fa1058a 100644 --- a/frontend/src/hooks/api/secretApprovalRequest/queries.tsx +++ b/frontend/src/hooks/api/secretApprovalRequest/queries.tsx @@ -79,6 +79,7 @@ export const decryptSecrets = ( id: encSecret.id, env: encSecret.environment, key: secretKey, + secretValueHidden: encSecret.secretValueHidden, value: secretValue, tags: encSecret.tags, comment: secretComment, diff --git a/frontend/src/hooks/api/secretSnapshots/queries.tsx b/frontend/src/hooks/api/secretSnapshots/queries.tsx index 8a16b4180..263768777 100644 --- a/frontend/src/hooks/api/secretSnapshots/queries.tsx +++ b/frontend/src/hooks/api/secretSnapshots/queries.tsx @@ -75,6 +75,7 @@ export const useGetSnapshotSecrets = ({ snapshotId }: TSnapshotDataProps) => id: secretVersion.secretId, env: data.environment.slug, key: secretVersion.secretKey, + secretValueHidden: false, value: secretVersion.secretValue || "", tags: secretVersion.tags, comment: secretVersion.secretComment, diff --git a/frontend/src/hooks/api/secrets/types.ts b/frontend/src/hooks/api/secrets/types.ts index b2287f6fe..e3d83a203 100644 --- a/frontend/src/hooks/api/secrets/types.ts +++ b/frontend/src/hooks/api/secrets/types.ts @@ -19,6 +19,7 @@ export type EncryptedSecret = { secretValueCiphertext: string; secretValueIV: string; secretValueTag: string; + secretValueHidden: boolean; __v: number; createdAt: string; updatedAt: string; diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx index c894d6b14..4847feb69 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx @@ -19,6 +19,7 @@ import { import { getKeyValue } from "@app/helpers/parseEnvVar"; import { useCreateFolder, useCreateSecretV3, useCreateWsTag, useGetWsTags } from "@app/hooks/api"; import { SecretType } from "@app/hooks/api/types"; +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; const typeSchema = z .object({ @@ -275,7 +276,7 @@ export const CreateSecretForm = ({ secretPath = "/", onClose }: Props) => { isMulti options={environments.filter((environment) => permission.can( - ProjectPermissionActions.Create, + ProjectPermissionSecretActions.Create, subject(ProjectPermissionSub.Secrets, { environment: environment.slug, secretPath, diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx index ae1916b86..3b05f10fa 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx @@ -10,12 +10,8 @@ import { z } from "zod"; import { createNotification } from "@app/components/notifications"; import { IconButton, Input, Spinner, Tooltip } from "@app/components/v2"; -import { - ProjectPermissionActions, - ProjectPermissionSub, - useProjectPermission, - useWorkspace -} from "@app/context"; +import { ProjectPermissionSub, useProjectPermission, useWorkspace } from "@app/context"; +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { useToggle } from "@app/hooks"; import { useUpdateSecretV3 } from "@app/hooks/api"; import { SecretType, SecretV3RawSanitized } from "@app/hooks/api/types"; @@ -55,8 +51,8 @@ function SecretRenameRow({ environments, getSecretByKey, secretKey, secretPath } secretTags: (secretDetails?.tags || []).map((i) => i.slug) }); const isSecretInEnvReadOnly = - permission.can(ProjectPermissionActions.Read, secretPermissionSubject) && - permission.cannot(ProjectPermissionActions.Edit, secretPermissionSubject); + permission.can(ProjectPermissionSecretActions.DescribeSecret, secretPermissionSubject) && + permission.cannot(ProjectPermissionSecretActions.Edit, secretPermissionSubject); if (isSecretInEnvReadOnly) { return true; } diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/SelectionPanel.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/SelectionPanel.tsx index 4738e411e..4e048b941 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/SelectionPanel.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/SelectionPanel.tsx @@ -11,6 +11,7 @@ import { useProjectPermission, useWorkspace } from "@app/context"; +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { usePopUp } from "@app/hooks"; import { useDeleteFolder, useDeleteSecretBatch } from "@app/hooks/api"; import { @@ -58,7 +59,7 @@ export const SelectionPanel = ({ secretPath, resetSelectedEntries, selectedEntri // user should have the ability to delete secrets/folders in at least one of the envs const shouldShowDelete = userAvailableEnvs.some((env) => permission.can( - ProjectPermissionActions.Delete, + ProjectPermissionSecretActions.Delete, subject(ProjectPermissionSub.Secrets, { environment: env.slug, secretPath, @@ -110,7 +111,7 @@ export const SelectionPanel = ({ secretPath, resetSelectedEntries, selectedEntri (accum: TDeleteSecretBatchDTO["secrets"], secretRecord) => { const entry = secretRecord[env.slug]; const canDeleteSecret = permission.can( - ProjectPermissionActions.Delete, + ProjectPermissionSecretActions.Delete, subject(ProjectPermissionSub.Secrets, { environment: env.slug, secretPath, diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx index 8a27a24c0..a81fa3e53 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx @@ -25,7 +25,8 @@ import { Spinner, Switch } from "@app/components/v2"; -import { ProjectPermissionActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { useDebounce } from "@app/hooks"; import { useMoveSecrets } from "@app/hooks/api"; import { useGetProjectSecretsQuickSearch } from "@app/hooks/api/dashboard"; @@ -95,7 +96,7 @@ const Content = ({ env.slug, { missingPermissions: permission.cannot( - ProjectPermissionActions.Delete, + ProjectPermissionSecretActions.Delete, subject(ProjectPermissionSub.Secrets, { environment: env.slug, secretPath: sourceSecretPath, diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx index dfb7aceb0..5cbefe9c3 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx @@ -122,8 +122,6 @@ const Page = () => { }) ); - console.log("Can read secret value", canReadSecret); - const canReadSecretImports = permission.can( ProjectPermissionActions.Read, subject(ProjectPermissionSub.SecretImports, { environment, secretPath })