fix(view-secret-value): backwards compatibility for read

backend/src/ee/services/permission/permission-fns.ts

@@ -1,7 +1,108 @@
+/* eslint-disable no-nested-ternary */
+import { ForbiddenError, MongoAbility, subject } from "@casl/ability";
+import { z } from "zod";
+
 import { TOrganizations } from "@app/db/schemas";
-import { ForbiddenRequestError, UnauthorizedError } from "@app/lib/errors";
+import { BadRequestError, ForbiddenRequestError, UnauthorizedError } from "@app/lib/errors";
 import { ActorAuthMethod, AuthMethod } from "@app/services/auth/auth-type";
 
+import {
+  ProjectPermissionSecretActions,
+  ProjectPermissionSet,
+  ProjectPermissionSub,
+  ProjectPermissionV2Schema,
+  SecretSubjectFields
+} from "./project-permission";
+
+export function CheckForbiddenErrorSecretsSubject(
+  permission: MongoAbility<ProjectPermissionSet>,
+  action: Extract<
+    ProjectPermissionSecretActions,
+    ProjectPermissionSecretActions.ReadValue | ProjectPermissionSecretActions.DescribeSecret
+  >,
+  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,
+        subject(ProjectPermissionSub.Secrets, subjectFields)
+      );
+    } else {
+      ForbiddenError.from(permission).throwUnlessCan(
+        ProjectPermissionSecretActions.DescribeAndReadValue,
+        ProjectPermissionSub.Secrets
+      );
+    }
+  }
+}
+
+export function CheckCanSecretsSubject(
+  permission: MongoAbility<ProjectPermissionSet>,
+  action: Extract<
+    ProjectPermissionSecretActions,
+    ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue
+  >,
+  subjectFields?: SecretSubjectFields
+) {
+  let canNewPermission = false;
+  let canOldPermission = false;
+
+  if (subjectFields) {
+    canNewPermission = permission.can(action, subject(ProjectPermissionSub.Secrets, subjectFields));
+    canOldPermission = permission.can(
+      ProjectPermissionSecretActions.DescribeAndReadValue,
+      subject(ProjectPermissionSub.Secrets, subjectFields)
+    );
+  } else {
+    canNewPermission = permission.can(action, ProjectPermissionSub.Secrets);
+    canOldPermission = permission.can(
+      ProjectPermissionSecretActions.DescribeAndReadValue,
+      ProjectPermissionSub.Secrets
+    );
+  }
+
+  return canNewPermission || canOldPermission;
+}
+
+const OptionalArrayPermissionSchema = ProjectPermissionV2Schema.array().optional();
+export function checkForInvalidPermissionCombination(permissions: z.infer<typeof OptionalArrayPermissionSchema>) {
+  if (!permissions) return;
+
+  for (const permission of permissions) {
+    if (permission.subject === ProjectPermissionSub.Secrets) {
+      if (permission.action.includes(ProjectPermissionSecretActions.DescribeAndReadValue)) {
+        const hasReadValue = permission.action.includes(ProjectPermissionSecretActions.ReadValue);
+        const hasDescribeSecret = permission.action.includes(ProjectPermissionSecretActions.DescribeSecret);
+
+        if (!hasReadValue && !hasDescribeSecret) return;
+
+        const hasBothDescribeAndReadValue =
+          permission.action.includes(ProjectPermissionSecretActions.DescribeSecret) &&
+          permission.action.includes(ProjectPermissionSecretActions.ReadValue);
+
+        throw new BadRequestError({
+          message: `You have selected Full Read Access, and ${
+            hasBothDescribeAndReadValue
+              ? "both Read Value and Describe Secret"
+              : hasReadValue
+                ? "Read Value"
+                : hasDescribeSecret
+                  ? "Describe Secret"
+                  : ""
+          }. You cannot select Read Value or Describe Secret if you have selected Full Read Access.`
+        });
+      }
+    }
+  }
+}
+
 function isAuthMethodSaml(actorAuthMethod: ActorAuthMethod) {
   if (!actorAuthMethod) return false;
 

backend/src/ee/services/permission/project-permission.ts

@@ -18,7 +18,8 @@ export enum ProjectPermissionActions {
 }
 
 export enum ProjectPermissionSecretActions {
-  DescribeSecret = "read",
+  DescribeAndReadValue = "read",
+  DescribeSecret = "describeSecret",
   ReadValue = "readValue",
   Create = "create",
   Edit = "edit",
@@ -564,6 +565,7 @@ const buildAdminPermissionRules = () => {
 
   can(
     [
+      // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue
       ProjectPermissionSecretActions.DescribeSecret,
       ProjectPermissionSecretActions.ReadValue,
       ProjectPermissionSecretActions.Create,
@@ -632,6 +634,7 @@ const buildMemberPermissionRules = () => {
 
   can(
     [
+      // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue
       ProjectPermissionSecretActions.DescribeSecret,
       ProjectPermissionSecretActions.ReadValue,
       ProjectPermissionSecretActions.Edit,
@@ -808,6 +811,7 @@ export const projectMemberPermissions = buildMemberPermissionRules();
 const buildViewerPermissionRules = () => {
   const { can, rules } = new AbilityBuilder<MongoAbility<ProjectPermissionSet>>(createMongoAbility);
 
+  // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue
   can(ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSub.Secrets);
   can(ProjectPermissionSecretActions.ReadValue, ProjectPermissionSub.Secrets);
   can(ProjectPermissionActions.Read, ProjectPermissionSub.SecretFolders);
@@ -852,7 +856,6 @@ export const buildServiceTokenProjectPermission = (
 ) => {
   const canWrite = permission.includes("write");
   const canRead = permission.includes("read");
-  const canReadValue = permission.includes("readValue");
 
   const { can, build } = new AbilityBuilder<MongoAbility<ProjectPermissionSet>>(createMongoAbility);
   scopes.forEach(({ secretPath, environment }) => {
@@ -876,7 +879,7 @@ export const buildServiceTokenProjectPermission = (
             environment
           });
         }
-        if (canRead) {
+        if (canRead && subject !== ProjectPermissionSub.Secrets) {
           can(ProjectPermissionActions.Read, subject, {
             // @ts-expect-error type
             secretPath: { $glob: secretPath },
@@ -884,12 +887,18 @@ export const buildServiceTokenProjectPermission = (
           });
         }
 
-        if (subject === ProjectPermissionSub.Secrets && canReadValue) {
+        if (subject === ProjectPermissionSub.Secrets && canRead) {
           // @ts-expect-error type
           can(ProjectPermissionSecretActions.ReadValue, subject as ProjectPermissionSub.Secrets, {
             secretPath: { $glob: secretPath },
             environment
           });
+
+          // @ts-expect-error type
+          can(ProjectPermissionSecretActions.DescribeSecret, subject as ProjectPermissionSub.Secrets, {
+            secretPath: { $glob: secretPath },
+            environment
+          });
         }
       }
     );

backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts

@@ -77,6 +77,7 @@ import {
   TSecretApprovalDetailsDTO,
   TStatusChangeDTO
 } from "./secret-approval-request-types";
+import { CheckForbiddenErrorSecretsSubject } from "../permission/permission-fns";
 
 type TSecretApprovalRequestServiceFactoryDep = {
   permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
@@ -917,10 +918,11 @@ export const secretApprovalRequestServiceFactory = ({
       actorOrgId,
       actionProjectType: ActionProjectType.SecretManager
     });
-    ForbiddenError.from(permission).throwUnlessCan(
-      ProjectPermissionSecretActions.ReadValue,
-      subject(ProjectPermissionSub.Secrets, { environment, secretPath })
-    );
+
+    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, {
+      environment,
+      secretPath
+    });
 
     await projectDAL.checkProjectUpgradeStatus(projectId);
 

backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts

@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-argument */
 // akhilmhdh: I did this, quite strange bug with eslint. Everything do have a type stil has this error
-import { ForbiddenError, subject } from "@casl/ability";
+import { ForbiddenError } from "@casl/ability";
 
 import { ActionProjectType, TableName, TSecretTagJunctionInsert, TSecretV2TagJunctionInsert } from "@app/db/schemas";
 import { decryptSymmetric128BitHexKeyUTF8 } from "@app/lib/crypto";
@@ -12,6 +12,7 @@ import { TKmsServiceFactory } from "@app/services/kms/kms-service";
 import { KmsDataKey } from "@app/services/kms/kms-types";
 import { TProjectBotServiceFactory } from "@app/services/project-bot/project-bot-service";
 import { TSecretDALFactory } from "@app/services/secret/secret-dal";
+import { INFISICAL_SECRET_VALUE_HIDDEN_MASK } from "@app/services/secret/secret-fns";
 import { TSecretVersionDALFactory } from "@app/services/secret/secret-version-dal";
 import { TSecretVersionTagDALFactory } from "@app/services/secret/secret-version-tag-dal";
 import { TSecretFolderDALFactory } from "@app/services/secret-folder/secret-folder-dal";
@@ -22,6 +23,7 @@ import { TSecretVersionV2DALFactory } from "@app/services/secret-v2-bridge/secre
 import { TSecretVersionV2TagDALFactory } from "@app/services/secret-v2-bridge/secret-version-tag-dal";
 
 import { TLicenseServiceFactory } from "../license/license-service";
+import { CheckCanSecretsSubject, CheckForbiddenErrorSecretsSubject } from "../permission/permission-fns";
 import { TPermissionServiceFactory } from "../permission/permission-service";
 import {
   ProjectPermissionActions,
@@ -39,7 +41,6 @@ import { TSnapshotFolderDALFactory } from "./snapshot-folder-dal";
 import { TSnapshotSecretDALFactory } from "./snapshot-secret-dal";
 import { TSnapshotSecretV2DALFactory } from "./snapshot-secret-v2-dal";
 import { getFullFolderPath } from "./snapshot-service-fns";
-import { INFISICAL_SECRET_VALUE_HIDDEN_MASK } from "@app/services/secret/secret-fns";
 
 type TSecretSnapshotServiceFactoryDep = {
   snapshotDAL: TSnapshotDALFactory;
@@ -102,10 +103,10 @@ 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(
-      ProjectPermissionSecretActions.DescribeSecret,
-      subject(ProjectPermissionSub.Secrets, { environment, secretPath: path })
-    );
+    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, {
+      environment,
+      secretPath: path
+    });
 
     const folder = await folderDAL.findBySecretPath(projectId, environment, path);
     if (!folder) {
@@ -139,10 +140,10 @@ 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(
-      ProjectPermissionSecretActions.DescribeSecret,
-      subject(ProjectPermissionSub.Secrets, { environment, secretPath: path })
-    );
+    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, {
+      environment,
+      secretPath: path
+    });
 
     const folder = await folderDAL.findBySecretPath(projectId, environment, path);
     if (!folder)
@@ -186,15 +187,12 @@ export const secretSnapshotServiceFactory = ({
       snapshotDetails = {
         ...encryptedSnapshotDetails,
         secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => {
-          const canReadValue = permission.can(
-            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
-            })
-          );
+          const canReadValue = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, {
+            environment: encryptedSnapshotDetails.environment.slug,
+            secretPath: fullFolderPath,
+            secretName: el.key,
+            secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined
+          });
 
           let secretValue = "";
           if (canReadValue) {
@@ -238,15 +236,12 @@ export const secretSnapshotServiceFactory = ({
             key: botKey
           });
 
-          const canReadValue = permission.can(
-            ProjectPermissionSecretActions.ReadValue,
-            subject(ProjectPermissionSub.Secrets, {
-              environment: encryptedSnapshotDetails.environment.slug,
-              secretPath: fullFolderPath,
-              secretName: secretKey,
-              secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined
-            })
-          );
+          const canReadValue = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, {
+            environment: encryptedSnapshotDetails.environment.slug,
+            secretPath: fullFolderPath,
+            secretName: secretKey,
+            secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined
+          });
 
           let secretValue = "";