Merge pull request #2493 from scott-ray-wilson/secrets-overview-fix

Fix: Secrets Overview Endpoint Filter Secrets for Read Permissive Environments
2026-09-22 13:39:35 +00:00 · 2024-09-26 11:32:37 -07:00
parent adc3542750 971987c786
commit c011d99b8b
6 changed files with 193 additions and 152 deletions
--- 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" });
--- 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<TListDynamicSecretsDTO, "projectSlug" | "projectId"> & {
  projectId: string;
--- 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 {
--- 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<TGetSecretsDTO, "actorId" | "actor" | "path" | "projectId" | "actorOrgId" | "actorAuthMethod" | "search"> & {
    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<TGetSecretsDTO, "actorId" | "actor" | "path" | "projectId" | "actorOrgId" | "actorAuthMethod" | "search"> & {
    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) {
--- 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<TGetSecretsRawDTO, "environment" | "includeImports" | "expandSecretReferences" | "recursive" | "tagSlugs"> & {
    environments: string[];
+    isInternal?: boolean;
  }) => {
    const { shouldUseSecretV2Bridge } = await projectBotService.getBotKey(projectId);

--- 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 &&
@@ -254,15 +267,7 @@ export const SecretMainPage = () => {
          <NavHeader
            pageName={t("dashboard.title")}
            currentEnv={environment}
-            userAvailableEnvs={currentWorkspace?.environments.filter(({ slug }) =>
-              permission.can(
-                ProjectPermissionActions.Read,
-                subject(ProjectPermissionSub.Secrets, {
-                  environment: slug,
-                  secretPath
-                })
-              )
-            )}
+            userAvailableEnvs={currentWorkspace?.environments}
            isFolderMode
            secretPath={secretPath}
            isProjectRelated