fix(view-secret-value): requested changes

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

@@ -14,7 +14,7 @@ import {
   SecretSubjectFields
 } from "./project-permission";
 
-export function CheckForbiddenErrorSecretsSubject(
+export function throwIfMissingSecretReadValueOrDescribePermission(
   permission: MongoAbility<ProjectPermissionSet> | PureAbility,
   action: Extract<
     ProjectPermissionSecretActions,
@@ -43,7 +43,7 @@ export function CheckForbiddenErrorSecretsSubject(
   }
 }
 
-export function CheckCanSecretsSubject(
+export function hasSecretReadValueOrDescribePermission(
   permission: MongoAbility<ProjectPermissionSet>,
   action: Extract<
     ProjectPermissionSecretActions,
@@ -83,12 +83,10 @@ export function checkForInvalidPermissionCombination(permissions: z.infer<typeof
 
         if (!hasReadValue && !hasDescribeSecret) return;
 
-        const hasBothDescribeAndReadValue =
-          permission.action.includes(ProjectPermissionSecretActions.DescribeSecret) &&
-          permission.action.includes(ProjectPermissionSecretActions.ReadValue);
+        const hasBothDescribeAndReadValue = hasReadValue && hasDescribeSecret;
 
         throw new BadRequestError({
-          message: `You have selected Full Read Access, and ${
+          message: `You have selected Read, and ${
             hasBothDescribeAndReadValue
               ? "both Read Value and Describe Secret"
               : hasReadValue
@@ -96,7 +94,7 @@ export function checkForInvalidPermissionCombination(permissions: z.infer<typeof
                 : hasDescribeSecret
                   ? "Describe Secret"
                   : ""
-          }. You cannot select Read Value or Describe Secret if you have selected Full Read Access.`
+          }. You cannot select Read Value or Describe Secret if you have selected Read. The Read permission is a legacy action which has been replaced by Describe Secret and Read Value.`
         });
       }
     }

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

@@ -565,7 +565,7 @@ const buildAdminPermissionRules = () => {
 
   can(
     [
-      // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue
+      ProjectPermissionSecretActions.DescribeAndReadValue,
       ProjectPermissionSecretActions.DescribeSecret,
       ProjectPermissionSecretActions.ReadValue,
       ProjectPermissionSecretActions.Create,
@@ -634,7 +634,7 @@ const buildMemberPermissionRules = () => {
 
   can(
     [
-      // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue
+      ProjectPermissionSecretActions.DescribeAndReadValue,
       ProjectPermissionSecretActions.DescribeSecret,
       ProjectPermissionSecretActions.ReadValue,
       ProjectPermissionSecretActions.Edit,
@@ -811,7 +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.DescribeAndReadValue, ProjectPermissionSub.Secrets);
   can(ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSub.Secrets);
   can(ProjectPermissionSecretActions.ReadValue, ProjectPermissionSub.Secrets);
   can(ProjectPermissionActions.Read, ProjectPermissionSub.SecretFolders);

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

@@ -77,7 +77,7 @@ import {
   TSecretApprovalDetailsDTO,
   TStatusChangeDTO
 } from "./secret-approval-request-types";
-import { CheckForbiddenErrorSecretsSubject } from "../permission/permission-fns";
+import { throwIfMissingSecretReadValueOrDescribePermission } from "../permission/permission-fns";
 
 type TSecretApprovalRequestServiceFactoryDep = {
   permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
@@ -919,7 +919,7 @@ export const secretApprovalRequestServiceFactory = ({
       actionProjectType: ActionProjectType.SecretManager
     });
 
-    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, {
+    throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, {
       environment,
       secretPath
     });

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

@@ -23,7 +23,10 @@ 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 {
+  hasSecretReadValueOrDescribePermission,
+  throwIfMissingSecretReadValueOrDescribePermission
+} from "../permission/permission-fns";
 import { TPermissionServiceFactory } from "../permission/permission-service";
 import {
   ProjectPermissionActions,
@@ -103,7 +106,7 @@ 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.
-    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, {
+    throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, {
       environment,
       secretPath: path
     });
@@ -140,7 +143,7 @@ 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.
-    CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, {
+    throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, {
       environment,
       secretPath: path
     });
@@ -187,12 +190,16 @@ export const secretSnapshotServiceFactory = ({
       snapshotDetails = {
         ...encryptedSnapshotDetails,
         secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => {
-          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
-          });
+          const canReadValue = hasSecretReadValueOrDescribePermission(
+            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) {
@@ -236,12 +243,16 @@ export const secretSnapshotServiceFactory = ({
             key: botKey
           });
 
-          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
-          });
+          const canReadValue = hasSecretReadValueOrDescribePermission(
+            permission,
+            ProjectPermissionSecretActions.ReadValue,
+            {
+              environment: encryptedSnapshotDetails.environment.slug,
+              secretPath: fullFolderPath,
+              secretName: secretKey,
+              secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined
+            }
+          );
 
           let secretValue = "";