From 483fb458dd7681af59f656d714ca8452ae93c9c3 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Tue, 11 Mar 2025 04:52:12 +0400 Subject: [PATCH] requested changes --- .../ee/services/permission/permission-fns.ts | 12 +- .../src/server/routes/v1/dashboard-router.ts | 45 ++++ .../secret-v2-bridge-service.ts | 90 +++++++- .../secret-v2-bridge-types.ts | 7 + backend/src/services/secret/secret-service.ts | 37 ++- backend/src/services/secret/secret-types.ts | 7 + frontend/src/hooks/api/dashboard/index.ts | 1 + frontend/src/hooks/api/dashboard/queries.tsx | 57 ++++- frontend/src/hooks/api/dashboard/types.ts | 10 + .../SecretOverviewTableRow/SecretEditRow.tsx | 89 +++---- .../SecretDropzone/CopySecretsFromBoard.tsx | 60 ++--- .../SecretListView/SecretDetailSidebar.tsx | 217 +++++++++--------- 12 files changed, 441 insertions(+), 191 deletions(-) diff --git a/backend/src/ee/services/permission/permission-fns.ts b/backend/src/ee/services/permission/permission-fns.ts index 9397df939..cccadb86e 100644 --- a/backend/src/ee/services/permission/permission-fns.ts +++ b/backend/src/ee/services/permission/permission-fns.ts @@ -23,12 +23,6 @@ export function throwIfMissingSecretReadValueOrDescribePermission( subjectFields?: SecretSubjectFields ) { try { - if (subjectFields) { - ForbiddenError.from(permission).throwUnlessCan(action, subject(ProjectPermissionSub.Secrets, subjectFields)); - } else { - ForbiddenError.from(permission).throwUnlessCan(action, ProjectPermissionSub.Secrets); - } - } catch { if (subjectFields) { ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionSecretActions.DescribeAndReadValue, @@ -40,6 +34,12 @@ export function throwIfMissingSecretReadValueOrDescribePermission( ProjectPermissionSub.Secrets ); } + } catch { + if (subjectFields) { + ForbiddenError.from(permission).throwUnlessCan(action, subject(ProjectPermissionSub.Secrets, subjectFields)); + } else { + ForbiddenError.from(permission).throwUnlessCan(action, ProjectPermissionSub.Secrets); + } } } diff --git a/backend/src/server/routes/v1/dashboard-router.ts b/backend/src/server/routes/v1/dashboard-router.ts index 27ba3f8a3..392fcf5d9 100644 --- a/backend/src/server/routes/v1/dashboard-router.ts +++ b/backend/src/server/routes/v1/dashboard-router.ts @@ -834,6 +834,51 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => { } }); + server.route({ + method: "GET", + url: "/accessible-secrets", + config: { + rateLimit: secretsLimit + }, + schema: { + querystring: z.object({ + projectId: z.string().trim(), + environment: z.string().trim(), + secretPath: z.string().trim().default("/").transform(removeTrailingSlash), + filterByAction: z + .enum([ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSecretActions.ReadValue]) + .default(ProjectPermissionSecretActions.ReadValue) + }), + response: { + 200: z.object({ + secrets: secretRawSchema + .extend({ + secretPath: z.string().optional() + }) + .array() + .optional() + }) + } + }, + onRequest: verifyAuth([AuthMode.JWT]), + handler: async (req) => { + const { projectId, environment, secretPath, filterByAction } = req.query; + + const { secrets } = await server.services.secret.getAccessibleSecrets({ + actorId: req.permission.id, + actor: req.permission.type, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId, + environment, + secretPath, + projectId, + filterByAction + }); + + return { secrets }; + } + }); + server.route({ method: "GET", url: "/secrets-by-keys", diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts index 45c6493d6..c66fdddf1 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts @@ -60,6 +60,7 @@ import { TCreateSecretDTO, TDeleteManySecretDTO, TDeleteSecretDTO, + TGetAccessibleSecretsDTO, TGetASecretDTO, TGetSecretReferencesTreeDTO, TGetSecretsDTO, @@ -200,7 +201,7 @@ export const secretV2BridgeServiceFactory = ({ const referredSecretsGroupBySecretKey = groupBy(referredSecrets, (i) => i.key); references.forEach((el) => { - throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, { + throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, { environment: el.environment, secretPath: el.secretPath, secretName: el.secretKey, @@ -1291,7 +1292,7 @@ export const secretV2BridgeServiceFactory = ({ folderDAL, secretImportDAL, decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : ""), - expandSecretReferences: shouldExpandSecretReferences ? expandSecretReferences : undefined, + expandSecretReferences: shouldExpandSecretReferences && viewSecretValue ? expandSecretReferences : undefined, hasSecretAccess: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => { return hasSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, { environment: expandEnvironment, @@ -1347,7 +1348,7 @@ export const secretV2BridgeServiceFactory = ({ let secretValue = secret.encryptedValue ? secretManagerDecryptor({ cipherTextBlob: secret.encryptedValue }).toString() : ""; - if (shouldExpandSecretReferences && secretValue) { + if (shouldExpandSecretReferences && secretValue && viewSecretValue) { // eslint-disable-next-line const expandedSecretValue = await expandSecretReferences({ environment, @@ -2613,6 +2614,86 @@ export const secretV2BridgeServiceFactory = ({ return { tree: stackTrace, value: expandedValue }; }; + const getAccessibleSecrets = async ({ + projectId, + secretPath, + environment, + filterByAction, + actorId, + actor, + actorAuthMethod, + actorOrgId + }: TGetAccessibleSecretsDTO) => { + const { permission } = await permissionService.getProjectPermission({ + actor, + actorId, + projectId, + actorAuthMethod, + actorOrgId, + actionProjectType: ActionProjectType.SecretManager + }); + throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath + }); + + const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); + if (!folder) return { secrets: [] }; + + const secrets = await secretDAL.findByFolderIds([folder.id]); + + const { decryptor: secretManagerDecryptor } = await kmsService.createCipherPairWithDataKey({ + type: KmsDataKey.SecretManager, + projectId + }); + + const decryptedSecrets = secrets + .filter((el) => { + if ( + !hasSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }) + ) { + return false; + } + + if (filterByAction === ProjectPermissionSecretActions.ReadValue) { + return hasSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }); + } + + return true; + }) + .map((secret) => { + return reshapeBridgeSecret( + projectId, + environment, + secretPath, + { + ...secret, + value: secret.encryptedValue + ? secretManagerDecryptor({ cipherTextBlob: secret.encryptedValue }).toString() + : "", + comment: secret.encryptedComment + ? secretManagerDecryptor({ cipherTextBlob: secret.encryptedComment }).toString() + : "" + }, + false + ); + }); + + return { + secrets: decryptedSecrets + }; + }; + return { createSecret, deleteSecret, @@ -2630,6 +2711,7 @@ export const secretV2BridgeServiceFactory = ({ getSecretsMultiEnv, getSecretReferenceTree, getSecretsByFolderMappings, - getSecretById + getSecretById, + getAccessibleSecrets }; }; diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-types.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-types.ts index 473d1ed83..7f415bff8 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-types.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-types.ts @@ -351,3 +351,10 @@ export type TGetSecretsRawByFolderMappingsDTO = { filters: TFindSecretsByFolderIdsFilter; filterByAction?: ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue; }; + +export type TGetAccessibleSecretsDTO = { + environment: string; + projectId: string; + secretPath: string; + filterByAction: ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue; +} & TProjectPermission; diff --git a/backend/src/services/secret/secret-service.ts b/backend/src/services/secret/secret-service.ts index 3593adf88..13a5c9736 100644 --- a/backend/src/services/secret/secret-service.ts +++ b/backend/src/services/secret/secret-service.ts @@ -81,6 +81,7 @@ import { TDeleteManySecretRawDTO, TDeleteSecretDTO, TDeleteSecretRawDTO, + TGetAccessibleSecretsDTO, TGetASecretByIdRawDTO, TGetASecretDTO, TGetASecretRawDTO, @@ -1312,6 +1313,39 @@ export const secretServiceFactory = ({ return { users: usersWithAccess, identities: identitiesWithAccess, groups: groupsWithAccess }; }; + const getAccessibleSecrets = async ({ + projectId, + secretPath, + actor, + actorId, + actorOrgId, + actorAuthMethod, + environment, + filterByAction + }: TGetAccessibleSecretsDTO) => { + const { shouldUseSecretV2Bridge } = await projectBotService.getBotKey(projectId); + + if (!shouldUseSecretV2Bridge) { + throw new BadRequestError({ + message: "Project version does not support this endpoint.", + name: "ProjectVersionNotSupported" + }); + } + + const secrets = await secretV2BridgeService.getAccessibleSecrets({ + projectId, + secretPath, + environment, + filterByAction, + actor, + actorId, + actorOrgId, + actorAuthMethod + }); + + return secrets; + }; + const getSecretsRaw = async ({ projectId, path, @@ -3261,6 +3295,7 @@ export const secretServiceFactory = ({ getSecretReferenceTree, getSecretsRawByFolderMappings, getSecretAccessList, - getSecretByIdRaw + getSecretByIdRaw, + getAccessibleSecrets }; }; diff --git a/backend/src/services/secret/secret-types.ts b/backend/src/services/secret/secret-types.ts index c7b8a067d..f42f70b78 100644 --- a/backend/src/services/secret/secret-types.ts +++ b/backend/src/services/secret/secret-types.ts @@ -20,6 +20,7 @@ import { TSecretV2BridgeDALFactory } from "../secret-v2-bridge/secret-v2-bridge- import { SecretUpdateMode } from "../secret-v2-bridge/secret-v2-bridge-types"; import { TSecretVersionV2DALFactory } from "../secret-v2-bridge/secret-version-dal"; import { TSecretVersionV2TagDALFactory } from "../secret-v2-bridge/secret-version-tag-dal"; +import { ProjectPermissionSecretActions } from "@app/ee/services/permission/project-permission"; type TPartialSecret = Pick; @@ -180,6 +181,12 @@ export enum SecretsOrderBy { Name = "name" // "key" for secrets but using name for use across resources } +export type TGetAccessibleSecretsDTO = { + secretPath: string; + environment: string; + filterByAction: ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue; +} & TProjectPermission; + export type TGetSecretsRawDTO = { expandSecretReferences?: boolean; path: string; diff --git a/frontend/src/hooks/api/dashboard/index.ts b/frontend/src/hooks/api/dashboard/index.ts index 83206bdf8..bbd70e306 100644 --- a/frontend/src/hooks/api/dashboard/index.ts +++ b/frontend/src/hooks/api/dashboard/index.ts @@ -1,4 +1,5 @@ export { + useGetAccessibleSecrets, useGetProjectSecretsDetails, useGetProjectSecretsOverview, useGetProjectSecretsQuickSearch diff --git a/frontend/src/hooks/api/dashboard/queries.tsx b/frontend/src/hooks/api/dashboard/queries.tsx index f704081a9..049238202 100644 --- a/frontend/src/hooks/api/dashboard/queries.tsx +++ b/frontend/src/hooks/api/dashboard/queries.tsx @@ -11,6 +11,7 @@ import { DashboardSecretsOrderBy, TDashboardProjectSecretsQuickSearch, TDashboardProjectSecretsQuickSearchResponse, + TGetAccessibleSecretsDTO, TGetDashboardProjectSecretsByKeys, TGetDashboardProjectSecretsDetailsDTO, TGetDashboardProjectSecretsOverviewDTO, @@ -20,6 +21,8 @@ import { OrderByDirection } from "@app/hooks/api/generic/types"; import { mergePersonalSecrets } from "@app/hooks/api/secrets/queries"; import { groupBy, unique } from "@app/lib/fn/array"; +import { SecretV3Raw } from "../types"; + export const dashboardKeys = { all: () => ["dashboard"] as const, getDashboardSecrets: ({ @@ -58,7 +61,14 @@ export const dashboardKeys = { ...dashboardKeys.getDashboardSecrets({ projectId, secretPath }), "quick-search", params - ] as const + ] as const, + getAccessibleSecrets: ({ + projectId, + secretPath, + environment, + filterByAction + }: TGetAccessibleSecretsDTO) => + [...dashboardKeys.all(), { projectId, secretPath, environment, filterByAction }] as const }; export const fetchProjectSecretsOverview = async ({ @@ -295,6 +305,22 @@ export const fetchProjectSecretsQuickSearch = async ({ return data; }; +const fetchAccessibleSecrets = async ({ + projectId, + secretPath, + environment, + filterByAction +}: TGetAccessibleSecretsDTO) => { + const { data } = await apiRequest.get<{ secrets: SecretV3Raw[] }>( + "/api/v1/dashboard/accessible-secrets", + { + params: { projectId, secretPath, environment, filterByAction } + } + ); + + return data.secrets; +}; + export const useGetProjectSecretsQuickSearch = ( { projectId, @@ -357,3 +383,32 @@ export const useGetProjectSecretsQuickSearch = ( placeholderData: (previousData) => previousData }); }; + +export const useGetAccessibleSecrets = ({ + projectId, + secretPath, + environment, + filterByAction, + options +}: TGetAccessibleSecretsDTO & { + options?: Omit< + UseQueryOptions< + SecretV3Raw[], + unknown, + SecretV3Raw[], + ReturnType + >, + "queryKey" | "queryFn" + >; +}) => { + return useQuery({ + ...options, + queryKey: dashboardKeys.getAccessibleSecrets({ + projectId, + secretPath, + environment, + filterByAction + }), + queryFn: () => fetchAccessibleSecrets({ projectId, secretPath, environment, filterByAction }) + }); +}; diff --git a/frontend/src/hooks/api/dashboard/types.ts b/frontend/src/hooks/api/dashboard/types.ts index 4808b463c..9540c4ae6 100644 --- a/frontend/src/hooks/api/dashboard/types.ts +++ b/frontend/src/hooks/api/dashboard/types.ts @@ -1,3 +1,4 @@ +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { TDynamicSecret } from "@app/hooks/api/dynamicSecret/types"; import { OrderByDirection } from "@app/hooks/api/generic/types"; import { TSecretFolder } from "@app/hooks/api/secretFolders/types"; @@ -101,3 +102,12 @@ export type TGetDashboardProjectSecretsByKeys = { environment: string; keys: string[]; }; + +export type TGetAccessibleSecretsDTO = { + projectId: string; + secretPath: string; + environment: string; + filterByAction: + | ProjectPermissionSecretActions.DescribeSecret + | ProjectPermissionSecretActions.ReadValue; +}; diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretEditRow.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretEditRow.tsx index 4887d4fd7..705d7e177 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretEditRow.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretEditRow.tsx @@ -27,9 +27,11 @@ import { } from "@app/components/v2"; import { Blur } from "@app/components/v2/Blur"; import { InfisicalSecretInput } from "@app/components/v2/InfisicalSecretInput"; -import { ProjectPermissionActions, ProjectPermissionSub } from "@app/context"; +import { ProjectPermissionActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { useToggle } from "@app/hooks"; import { SecretType } from "@app/hooks/api/types"; +import { hasSecretReadValueOrDescribePermission } from "@app/lib/fn/permission"; type Props = { defaultValue?: string | null; @@ -79,6 +81,9 @@ export const SecretEditRow = ({ value: defaultValue || null } }); + + const { permission } = useProjectPermission(); + const [isDeleting, setIsDeleting] = useToggle(); const [isModalOpen, setIsModalOpen] = useState(false); @@ -120,6 +125,11 @@ export const SecretEditRow = ({ reset({ value }); }; + const canReadSecretValue = hasSecretReadValueOrDescribePermission( + permission, + ProjectPermissionSecretActions.ReadValue + ); + const handleDeleteSecret = useCallback(async () => { setIsDeleting.on(); setIsModalOpen(false); @@ -229,48 +239,43 @@ export const SecretEditRow = ({ - - {(isAllowed) => ( -
- - -
- - - - - -
- - e.preventDefault()} // prevents secret input from displaying value on open + +
+ + +
+ - - - -
- )} - + + + + +
+ + e.preventDefault()} // prevents secret input from displaying value on open + > + + + +
+ 1 ? val.slice(0, -1) : val ), secrets: z - .object({ key: z.string(), value: z.string().optional() }) + .object({ secretKey: z.string(), secretValue: z.string().optional() }) .array() .min(1, "Select one or more secrets to copy") }); @@ -78,34 +79,38 @@ export const CopySecretsFromBoard = ({ const selectedEnvSlug = watch("environment"); const [debouncedEnvCopySecretPath] = useDebounce(envCopySecPath); - const { data: secrets, isPending: isSecretsLoading } = useGetProjectSecrets({ - workspaceId, - environment: selectedEnvSlug.slug, - secretPath: debouncedEnvCopySecretPath, - options: { - enabled: - Boolean(workspaceId) && - Boolean(selectedEnvSlug) && - Boolean(debouncedEnvCopySecretPath) && - isOpen - } - }); + const { data: accessibleSecrets, isPending: isAccessibleSecretsLoading } = + useGetAccessibleSecrets({ + projectId: workspaceId, + secretPath: debouncedEnvCopySecretPath, + environment: selectedEnvSlug.slug, + filterByAction: shouldIncludeValues + ? ProjectPermissionSecretActions.ReadValue + : ProjectPermissionSecretActions.DescribeSecret, + options: { + enabled: + Boolean(workspaceId) && + Boolean(selectedEnvSlug) && + Boolean(debouncedEnvCopySecretPath) && + isOpen + } + }); useEffect(() => { setValue("secrets", []); }, [debouncedEnvCopySecretPath, selectedEnvSlug]); const handleSecSelectAll = () => { - if (secrets) { - setValue("secrets", secrets, { shouldDirty: true }); + if (accessibleSecrets) { + setValue("secrets", accessibleSecrets, { shouldDirty: true }); } }; const handleFormSubmit = async (data: TFormSchema) => { const secretsToBePulled: Record = {}; - data.secrets.forEach(({ key, value }) => { - secretsToBePulled[key] = { - value: (shouldIncludeValues && value) || "", + data.secrets.forEach(({ secretKey, secretValue }) => { + secretsToBePulled[secretKey] = { + value: (shouldIncludeValues && secretValue) || "", comments: [""] }; }); @@ -202,19 +207,19 @@ export const CopySecretsFromBoard = ({ option.key} - getOptionLabel={(option) => option.key} + getOptionValue={(option) => option.secretKey} + getOptionLabel={(option) => option.secretKey} /> )} @@ -235,7 +240,10 @@ export const CopySecretsFromBoard = ({ setShouldIncludeValues(isChecked as boolean)} + onCheckedChange={(isChecked) => { + setValue("secrets", []); + setShouldIncludeValues(isChecked as boolean); + }} > Include secret values diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx index 410d49e5f..641a4a723 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx @@ -13,8 +13,8 @@ import { faShare, faTag, faTrash, - faUser, - faTriangleExclamation + faTriangleExclamation, + faUser } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { zodResolver } from "@hookform/resolvers/zod"; @@ -728,8 +728,8 @@ export const SecretDetailSidebar = ({
Version History
{secretVersion?.map( - ({ createdAt, secretValue, version, id, secretValueHidden, actor }, index) => ( -
+ ({ createdAt, secretValue, secretValueHidden, version, id, actor }) => ( +
@@ -739,72 +739,50 @@ export const SecretDetailSidebar = ({
{format(new Date(createdAt), "Pp")}
-
{format(new Date(createdAt), "Pp")}
-
-
-
-
-
-
- {actor && ( -
-
- Modified by: - - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} -
- onModifyHistoryClick( - actor.actorId, - actor.actorType, - actor.membershipId - ) - } - className="cursor-pointer" +
+
+
+
+
+ {actor && ( +
+
+ Modified by: + - -
- + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
+ onModifyHistoryClick( + actor.actorId, + actor.actorType, + actor.membershipId + ) + } + className="cursor-pointer" + > + +
+ +
-
- )} -
-
- Value: -
-
-
- - + -
- - {secretValueHidden ? "******" : secretValue?.replace(/./g, "*")} - +
+ + {secretValueHidden ? "******" : secretValue?.replace(/./g, "*")} + - + }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.currentTarget + .closest(".group") + ?.classList.add("show-value"); + } + }} + > + + + +