From cd71a13bb72f14123b53dc0abb3d8f90d1c70c9d Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Thu, 26 Sep 2024 09:24:29 -0700 Subject: [PATCH 1/2] fix: refactor secrets overview endpoint to filter envs for secrets with read permissions --- .../dynamic-secret/dynamic-secret-service.ts | 64 +++--- .../dynamic-secret/dynamic-secret-types.ts | 2 +- .../src/server/routes/v3/dashboard-router.ts | 186 ++++++++++-------- .../secret-v2-bridge-service.ts | 67 ++++--- backend/src/services/secret/secret-service.ts | 3 +- .../views/SecretMainPage/SecretMainPage.tsx | 13 ++ 6 files changed, 192 insertions(+), 143 deletions(-) diff --git a/backend/src/ee/services/dynamic-secret/dynamic-secret-service.ts b/backend/src/ee/services/dynamic-secret/dynamic-secret-service.ts index 3f6492571..64883cbb5 100644 --- a/backend/src/ee/services/dynamic-secret/dynamic-secret-service.ts +++ b/backend/src/ee/services/dynamic-secret/dynamic-secret-service.ts @@ -313,23 +313,26 @@ export const dynamicSecretServiceFactory = ({ projectId, path, environmentSlugs, - search + search, + isInternal }: TListDynamicSecretsMultiEnvDTO) => { - const { permission } = await permissionService.getProjectPermission( - actor, - actorId, - projectId, - actorAuthMethod, - actorOrgId - ); + if (!isInternal) { + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + projectId, + actorAuthMethod, + actorOrgId + ); - // verify user has access to each env in request - environmentSlugs.forEach((environmentSlug) => - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { environment: environmentSlug, secretPath: path }) - ) - ); + // verify user has access to each env in request + environmentSlugs.forEach((environmentSlug) => + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment: environmentSlug, secretPath: path }) + ) + ); + } const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environmentSlugs, path); if (!folders.length) throw new BadRequestError({ message: "Folders not found" }); @@ -434,23 +437,26 @@ export const dynamicSecretServiceFactory = ({ path, environmentSlugs, projectId, + isInternal, ...params }: TListDynamicSecretsMultiEnvDTO) => { - const { permission } = await permissionService.getProjectPermission( - actor, - actorId, - projectId, - actorAuthMethod, - actorOrgId - ); + if (!isInternal) { + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + projectId, + actorAuthMethod, + actorOrgId + ); - // verify user has access to each env in request - environmentSlugs.forEach((environmentSlug) => - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { environment: environmentSlug, secretPath: path }) - ) - ); + // verify user has access to each env in request + environmentSlugs.forEach((environmentSlug) => + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment: environmentSlug, secretPath: path }) + ) + ); + } const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environmentSlugs, path); if (!folders.length) throw new BadRequestError({ message: "Folders not found" }); diff --git a/backend/src/ee/services/dynamic-secret/dynamic-secret-types.ts b/backend/src/ee/services/dynamic-secret/dynamic-secret-types.ts index 426135a4c..208290db1 100644 --- a/backend/src/ee/services/dynamic-secret/dynamic-secret-types.ts +++ b/backend/src/ee/services/dynamic-secret/dynamic-secret-types.ts @@ -63,7 +63,7 @@ export type TListDynamicSecretsDTO = { export type TListDynamicSecretsMultiEnvDTO = Omit< TListDynamicSecretsDTO, "projectId" | "environmentSlug" | "projectSlug" -> & { projectId: string; environmentSlugs: string[] }; +> & { projectId: string; environmentSlugs: string[]; isInternal?: boolean }; export type TGetDynamicSecretsCountDTO = Omit & { projectId: string; diff --git a/backend/src/server/routes/v3/dashboard-router.ts b/backend/src/server/routes/v3/dashboard-router.ts index 8cce92e31..c66e5e912 100644 --- a/backend/src/server/routes/v3/dashboard-router.ts +++ b/backend/src/server/routes/v3/dashboard-router.ts @@ -1,8 +1,9 @@ -import { ForbiddenError } from "@casl/ability"; +import { ForbiddenError, subject } from "@casl/ability"; import { z } from "zod"; import { SecretFoldersSchema, SecretImportsSchema, SecretTagsSchema } from "@app/db/schemas"; import { EventType, UserAgentType } from "@app/ee/services/audit-log/audit-log-types"; +import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; import { DASHBOARD } from "@app/lib/api-docs"; import { BadRequestError } from "@app/lib/errors"; import { removeTrailingSlash } from "@app/lib/fn"; @@ -174,114 +175,135 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => { } } - try { - if (includeDynamicSecrets) { - // this is the unique count, ie duplicate secrets across envs only count as 1 - totalDynamicSecretCount = await server.services.dynamicSecret.getCountMultiEnv({ + if (!includeDynamicSecrets && !includeSecrets) + return { + folders, + totalFolderCount, + totalCount: totalFolderCount ?? 0 + }; + + const { permission } = await server.services.permission.getProjectPermission( + req.permission.type, + req.permission.id, + projectId, + req.permission.authMethod, + req.permission.orgId + ); + + const permissiveEnvs = // filter envs user has access to + environments.filter((environment) => + permission.can( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment, secretPath }) + ) + ); + + if (includeDynamicSecrets) { + // this is the unique count, ie duplicate secrets across envs only count as 1 + totalDynamicSecretCount = await server.services.dynamicSecret.getCountMultiEnv({ + actor: req.permission.type, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId, + projectId, + search, + environmentSlugs: permissiveEnvs, + path: secretPath, + isInternal: true + }); + + if (remainingLimit > 0 && totalDynamicSecretCount > adjustedOffset) { + dynamicSecrets = await server.services.dynamicSecret.listDynamicSecretsByFolderIds({ actor: req.permission.type, actorId: req.permission.id, actorAuthMethod: req.permission.authMethod, actorOrgId: req.permission.orgId, projectId, search, - environmentSlugs: environments, - path: secretPath + orderBy, + orderDirection, + environmentSlugs: permissiveEnvs, + path: secretPath, + limit: remainingLimit, + offset: adjustedOffset, + isInternal: true }); - if (remainingLimit > 0 && totalDynamicSecretCount > adjustedOffset) { - dynamicSecrets = await server.services.dynamicSecret.listDynamicSecretsByFolderIds({ - actor: req.permission.type, - actorId: req.permission.id, - actorAuthMethod: req.permission.authMethod, - actorOrgId: req.permission.orgId, - projectId, - search, - orderBy, - orderDirection, - environmentSlugs: environments, - path: secretPath, - limit: remainingLimit, - offset: adjustedOffset - }); + // get the count of unique dynamic secret names to properly adjust remaining limit + const uniqueDynamicSecretsCount = new Set(dynamicSecrets.map((dynamicSecret) => dynamicSecret.name)).size; - // get the count of unique dynamic secret names to properly adjust remaining limit - const uniqueDynamicSecretsCount = new Set(dynamicSecrets.map((dynamicSecret) => dynamicSecret.name)).size; - - remainingLimit -= uniqueDynamicSecretsCount; - adjustedOffset = 0; - } else { - adjustedOffset = Math.max(0, adjustedOffset - totalDynamicSecretCount); - } + remainingLimit -= uniqueDynamicSecretsCount; + adjustedOffset = 0; + } else { + adjustedOffset = Math.max(0, adjustedOffset - totalDynamicSecretCount); } + } - if (includeSecrets) { - // this is the unique count, ie duplicate secrets across envs only count as 1 - totalSecretCount = await server.services.secret.getSecretsCountMultiEnv({ + if (includeSecrets) { + // this is the unique count, ie duplicate secrets across envs only count as 1 + totalSecretCount = await server.services.secret.getSecretsCountMultiEnv({ + actorId: req.permission.id, + actor: req.permission.type, + actorOrgId: req.permission.orgId, + environments: permissiveEnvs, + actorAuthMethod: req.permission.authMethod, + projectId, + path: secretPath, + search, + isInternal: true + }); + + if (remainingLimit > 0 && totalSecretCount > adjustedOffset) { + secrets = await server.services.secret.getSecretsRawMultiEnv({ actorId: req.permission.id, actor: req.permission.type, actorOrgId: req.permission.orgId, - environments, + environments: permissiveEnvs, actorAuthMethod: req.permission.authMethod, projectId, path: secretPath, - search + orderBy, + orderDirection, + search, + limit: remainingLimit, + offset: adjustedOffset, + isInternal: true }); - if (remainingLimit > 0 && totalSecretCount > adjustedOffset) { - secrets = await server.services.secret.getSecretsRawMultiEnv({ - actorId: req.permission.id, - actor: req.permission.type, - actorOrgId: req.permission.orgId, - environments, - actorAuthMethod: req.permission.authMethod, - projectId, - path: secretPath, - orderBy, - orderDirection, - search, - limit: remainingLimit, - offset: adjustedOffset - }); + for await (const environment of permissiveEnvs) { + const secretCountFromEnv = secrets.filter((secret) => secret.environment === environment).length; - for await (const environment of environments) { - const secretCountFromEnv = secrets.filter((secret) => secret.environment === environment).length; + if (secretCountFromEnv) { + await server.services.auditLog.createAuditLog({ + projectId, + ...req.auditLogInfo, + event: { + type: EventType.GET_SECRETS, + metadata: { + environment, + secretPath, + numberOfSecrets: secretCountFromEnv + } + } + }); - if (secretCountFromEnv) { - await server.services.auditLog.createAuditLog({ - projectId, - ...req.auditLogInfo, - event: { - type: EventType.GET_SECRETS, - metadata: { - environment, - secretPath, - numberOfSecrets: secretCountFromEnv - } + if (getUserAgentType(req.headers["user-agent"]) !== UserAgentType.K8_OPERATOR) { + await server.services.telemetry.sendPostHogEvents({ + event: PostHogEventTypes.SecretPulled, + distinctId: getTelemetryDistinctId(req), + properties: { + numberOfSecrets: secretCountFromEnv, + workspaceId: projectId, + environment, + secretPath, + channel: getUserAgentType(req.headers["user-agent"]), + ...req.auditLogInfo } }); - - if (getUserAgentType(req.headers["user-agent"]) !== UserAgentType.K8_OPERATOR) { - await server.services.telemetry.sendPostHogEvents({ - event: PostHogEventTypes.SecretPulled, - distinctId: getTelemetryDistinctId(req), - properties: { - numberOfSecrets: secretCountFromEnv, - workspaceId: projectId, - environment, - secretPath, - channel: getUserAgentType(req.headers["user-agent"]), - ...req.auditLogInfo - } - }); - } } } } } - } catch (error) { - if (!(error instanceof ForbiddenError)) { - throw error; - } } return { 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 5dbb32e7d..c50a5a25a 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 @@ -455,31 +455,34 @@ export const secretV2BridgeServiceFactory = ({ const getSecretsCountMultiEnv = async ({ actorId, path, - projectId, actor, actorOrgId, actorAuthMethod, environments, + isInternal, ...params }: Pick & { environments: string[]; + isInternal?: boolean; }) => { - const { permission } = await permissionService.getProjectPermission( - actor, - actorId, - projectId, - actorAuthMethod, - actorOrgId - ); + if (!isInternal) { + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + projectId, + actorAuthMethod, + actorOrgId + ); - // verify user has access to all environments - environments.forEach((environment) => - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ) - ); + // verify user has access to all environments + environments.forEach((environment) => + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) + ) + ); + } const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environments, path); if (!folders.length) return 0; @@ -546,28 +549,32 @@ export const secretV2BridgeServiceFactory = ({ actor, actorOrgId, actorAuthMethod, + isInternal, ...params }: Pick & { environments: string[]; + isInternal?: boolean; }) => { - const { permission } = await permissionService.getProjectPermission( - actor, - actorId, - projectId, - actorAuthMethod, - actorOrgId - ); + if (!isInternal) { + const { permission } = await permissionService.getProjectPermission( + actor, + actorId, + projectId, + actorAuthMethod, + actorOrgId + ); + + // verify user has access to all environments + environments.forEach((environment) => + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) + ) + ); + } let paths: { folderId: string; path: string; environment: string }[] = []; - // verify user has access to all environments - environments.forEach((environment) => - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ) - ); - const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environments, path); if (!folders.length) { diff --git a/backend/src/services/secret/secret-service.ts b/backend/src/services/secret/secret-service.ts index 0bf1f1171..566598238 100644 --- a/backend/src/services/secret/secret-service.ts +++ b/backend/src/services/secret/secret-service.ts @@ -1011,7 +1011,7 @@ export const secretServiceFactory = ({ }: Pick< TGetSecretsRawDTO, "projectId" | "path" | "actor" | "actorId" | "actorOrgId" | "actorAuthMethod" | "search" - > & { environments: string[] }) => { + > & { environments: string[]; isInternal?: boolean }) => { const { shouldUseSecretV2Bridge } = await projectBotService.getBotKey(projectId); if (!shouldUseSecretV2Bridge) @@ -1045,6 +1045,7 @@ export const secretServiceFactory = ({ ...params }: Omit & { environments: string[]; + isInternal?: boolean; }) => { const { shouldUseSecretV2Bridge } = await projectBotService.getBotKey(projectId); diff --git a/frontend/src/views/SecretMainPage/SecretMainPage.tsx b/frontend/src/views/SecretMainPage/SecretMainPage.tsx index aa6804511..d68a66a6b 100644 --- a/frontend/src/views/SecretMainPage/SecretMainPage.tsx +++ b/frontend/src/views/SecretMainPage/SecretMainPage.tsx @@ -91,6 +91,19 @@ export const SecretMainPage = () => { }); const debouncedSearchFilter = useDebounce(filter.searchFilter); + // change filters if permissions change at different paths/env + useEffect(() => { + setFilter((prev) => ({ + ...prev, + include: { + [RowType.Folder]: true, + [RowType.Import]: canReadSecret, + [RowType.DynamicSecret]: canReadSecret, + [RowType.Secret]: canReadSecret + } + })); + }, [canReadSecret]); + useEffect(() => { if ( !isWorkspaceLoading && From 971987c786a7b267c53dd7b8900dafeb02dd1a05 Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Thu, 26 Sep 2024 09:32:15 -0700 Subject: [PATCH 2/2] fix: display all envs in secrets overview header --- frontend/src/views/SecretMainPage/SecretMainPage.tsx | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/frontend/src/views/SecretMainPage/SecretMainPage.tsx b/frontend/src/views/SecretMainPage/SecretMainPage.tsx index d68a66a6b..20dd42ab9 100644 --- a/frontend/src/views/SecretMainPage/SecretMainPage.tsx +++ b/frontend/src/views/SecretMainPage/SecretMainPage.tsx @@ -267,15 +267,7 @@ export const SecretMainPage = () => { - permission.can( - ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { - environment: slug, - secretPath - }) - ) - )} + userAvailableEnvs={currentWorkspace?.environments} isFolderMode secretPath={secretPath} isProjectRelated