improvements: address feedback

frontend/src/components/permissions/AccessTree/nodes/FolderNode/components/FolderNodeTooltipContent.tsx

@@ -21,17 +21,16 @@ type ConditionDisplayProps = {
   _key: string;
   operator: string;
   value: string | string[];
-  inverted?: boolean;
 };
 
-const Display = ({ _key: key, value, operator, inverted }: ConditionDisplayProps) => {
+const ConditionDisplay = ({ _key: key, value, operator }: ConditionDisplayProps) => {
   return (
     <li>
       <span className="font-medium capitalize text-mineshaft-100">{camelCaseToSpaces(key)}</span>{" "}
       <span className="text-mineshaft-200">
         {formatedConditionsOperatorNames[operator as PermissionConditionOperators]}
       </span>{" "}
-      <span className={inverted ? "text-red" : "text-green"}>
+      <span className="rounded bg-mineshaft-600 p-0.5 font-mono">
         {typeof value === "string" ? value : value.join(", ")}
       </span>
       .
@@ -87,9 +86,7 @@ export const FolderNodeTooltipContent = ({ action, access, actionRuleMap, subjec
               ) {
                 return (
                   <li key={`${action}_${index + 1}`}>
-                    <span className={`italic ${rule.inverted ? "text-red" : "text-green"} `}>
+                    <span className="italic">{rule.inverted ? "Forbids" : "Allows"}</span>
-                      {rule.inverted ? "Forbids" : "Allows"}
-                    </span>
                     <span> when:</span>
                     {Object.entries(rule.conditions).map(([key, condition]) => {
                       return (
@@ -100,8 +97,7 @@ export const FolderNodeTooltipContent = ({ action, access, actionRuleMap, subjec
                                 ([nestedKey, nestedCondition]) =>
                                   Object.entries(nestedCondition as object).map(
                                     ([nestedOperator, nestedValue]) => (
-                                      <Display
+                                      <ConditionDisplay
-                                        inverted={rule.inverted}
                                         _key={`${key} ${nestedKey}`}
                                         operator={nestedOperator}
                                         value={nestedValue}
@@ -113,8 +109,7 @@ export const FolderNodeTooltipContent = ({ action, access, actionRuleMap, subjec
                             }
 
                             return (
-                              <Display
+                              <ConditionDisplay
-                                inverted={rule.inverted}
                                 _key={key}
                                 operator={operator}
                                 value={value}

frontend/src/components/permissions/AccessTree/utils/createFolderNode.ts

@@ -33,6 +33,12 @@ const ACTION_MAP: Record<string, string[] | undefined> = {
   ]
 };
 
+const SUBJECT_HEIGHT_MAP: Record<string, number> = {
+  [ProjectPermissionSub.DynamicSecrets]: 130,
+  [ProjectPermissionSub.Secrets]: 85,
+  default: 64
+};
+
 const evaluateCondition = (
   value: string,
   operator: PermissionConditionOperators,
@@ -52,6 +58,105 @@ const evaluateCondition = (
   }
 };
 
+const doesConditionMatch = (
+  conditions: Record<string, any> | undefined,
+  value: string
+): boolean => {
+  if (!conditions) return true;
+
+  return Object.entries(conditions).every(([operator, comparisonValue]) =>
+    evaluateCondition(value, operator as PermissionConditionOperators, comparisonValue)
+  );
+};
+
+const doBaseConditionsApply = (
+  ruleConditions: any,
+  environment: string,
+  folderPath: string
+): boolean => {
+  return (
+    doesConditionMatch(ruleConditions?.environment, environment) &&
+    doesConditionMatch(ruleConditions?.secretPath, folderPath)
+  );
+};
+
+const shouldShowConditionalAccess = (
+  actionRuleMap: TActionRuleMap,
+  action: string,
+  environment: string,
+  folderPath: string,
+  conditionalFields: string[]
+): boolean => {
+  return actionRuleMap.some((rule) => {
+    const ruleConditions = rule[action]?.conditions;
+    if (!ruleConditions) return false;
+
+    // Check if any of the conditional fields are present
+    const hasConditionalField = conditionalFields.some((field) => ruleConditions[field]);
+    if (!hasConditionalField) return false;
+
+    // Check if base conditions (environment and secretPath) apply
+    return doBaseConditionsApply(ruleConditions, environment, folderPath);
+  });
+};
+
+const determineAccessLevel = (
+  hasPermission: boolean,
+  subject: ProjectPermissionSub,
+  action: string,
+  actionRuleMap: TActionRuleMap,
+  environment: string,
+  folderPath: string,
+  secretName: string,
+  metadata: Array<{ key: string; value: string }>
+): PermissionAccess => {
+  if (!hasPermission) {
+    return PermissionAccess.None;
+  }
+
+  if (subject === ProjectPermissionSub.Secrets) {
+    if (
+      !secretName &&
+      shouldShowConditionalAccess(actionRuleMap, action, environment, folderPath, [
+        "secretName",
+        "secretTags"
+      ])
+    ) {
+      return PermissionAccess.Partial;
+    }
+  } else if (subject === ProjectPermissionSub.DynamicSecrets) {
+    if (
+      !metadata.length &&
+      shouldShowConditionalAccess(actionRuleMap, action, environment, folderPath, ["metadata"])
+    ) {
+      return PermissionAccess.Partial;
+    }
+  }
+
+  return PermissionAccess.Full;
+};
+
+const checkPermission = (
+  permissions: MongoAbility<ProjectPermissionSet, MongoQuery>,
+  subject: ProjectPermissionSub,
+  action: string,
+  subjectFields: any
+): boolean => {
+  if (
+    subject === ProjectPermissionSub.Secrets &&
+    (action === ProjectPermissionSecretActions.ReadValue ||
+      action === ProjectPermissionSecretActions.DescribeSecret)
+  ) {
+    return hasSecretReadValueOrDescribePermission(permissions, action, subjectFields);
+  }
+
+  return permissions.can(
+    // @ts-expect-error we are not specifying which so can't resolve if valid
+    action,
+    abilitySubject(subject, subjectFields)
+  );
+};
+
 export const createFolderNode = ({
   folder,
   permissions,
@@ -75,8 +180,6 @@ export const createFolderNode = ({
 
       // wrapped in try because while editing certain conditions, if their values are empty it throws an error
       try {
-        let hasPermission: boolean;
-
         const subjectFields = {
           secretPath: folder.path,
           environment,
@@ -85,123 +188,18 @@ export const createFolderNode = ({
           metadata: metadata.length ? metadata : ["*"]
         };
 
-        if (
+        const hasPermission = checkPermission(permissions, subject, action, subjectFields);
-          subject === ProjectPermissionSub.Secrets &&
-          (action === ProjectPermissionSecretActions.ReadValue ||
-            action === ProjectPermissionSecretActions.DescribeSecret)
-        ) {
-          hasPermission = hasSecretReadValueOrDescribePermission(
-            permissions,
-            action,
-            subjectFields
-          );
-        } else {
-          hasPermission = permissions.can(
-            // @ts-expect-error we are not specifying which so can't resolve if valid
-            action,
-            abilitySubject(subject, subjectFields)
-          );
-        }
 
-        if (hasPermission) {
+        access = determineAccessLevel(
-          if (subject === ProjectPermissionSub.Secrets) {
+          hasPermission,
-            // we want to show yellow/conditional access if user hasn't specified secret name to fully resolve access
+          subject,
-            if (
+          action,
-              !secretName &&
+          actionRuleMap,
-              actionRuleMap.some((el) => {
+          environment,
-                // we only show conditional if secretName/secretTags are present - environment and path can be directly determined
+          folder.path,
-                if (!el[action]?.conditions?.secretName && !el[action]?.conditions?.secretTags)
+          secretName,
-                  return false;
+          metadata
-
+        );
-                // make sure condition applies to env
-                if (el[action]?.conditions?.environment) {
-                  if (
-                    !Object.entries(el[action]?.conditions?.environment).every(
-                      ([operator, value]) =>
-                        evaluateCondition(
-                          environment,
-                          operator as PermissionConditionOperators,
-                          value
-                        )
-                    )
-                  ) {
-                    return false;
-                  }
-                }
-
-                // and applies to path
-                if (el[action]?.conditions?.secretPath) {
-                  if (
-                    !Object.entries(el[action]?.conditions?.secretPath).every(([operator, value]) =>
-                      evaluateCondition(
-                        folder.path,
-                        operator as PermissionConditionOperators,
-                        value
-                      )
-                    )
-                  ) {
-                    return false;
-                  }
-                }
-
-                return true;
-              })
-            ) {
-              access = PermissionAccess.Partial;
-            } else {
-              access = PermissionAccess.Full;
-            }
-          } else if (subject === ProjectPermissionSub.DynamicSecrets) {
-            if (
-              !metadata.length &&
-              actionRuleMap.some((el) => {
-                // we only show conditional if metadata present - environment and path can be directly determined
-                if (!el[action]?.conditions?.metadata) return false;
-
-                // make sure condition applies to env
-                if (el[action]?.conditions?.environment) {
-                  if (
-                    !Object.entries(el[action]?.conditions?.environment).every(
-                      ([operator, value]) =>
-                        evaluateCondition(
-                          environment,
-                          operator as PermissionConditionOperators,
-                          value
-                        )
-                    )
-                  ) {
-                    return false;
-                  }
-                }
-
-                // and applies to path
-                if (el[action]?.conditions?.secretPath) {
-                  if (
-                    !Object.entries(el[action]?.conditions?.secretPath).every(([operator, value]) =>
-                      evaluateCondition(
-                        folder.path,
-                        operator as PermissionConditionOperators,
-                        value
-                      )
-                    )
-                  ) {
-                    return false;
-                  }
-                }
-
-                return true;
-              })
-            ) {
-              access = PermissionAccess.Partial;
-            } else {
-              access = PermissionAccess.Full;
-            }
-          } else {
-            access = PermissionAccess.Full;
-          }
-        } else {
-          access = PermissionAccess.None;
-        }
       } catch (e) {
         console.error(e);
         access = PermissionAccess.None;
@@ -211,18 +209,7 @@ export const createFolderNode = ({
     })
   );
 
-  let height: number;
+  const height = SUBJECT_HEIGHT_MAP[subject] ?? SUBJECT_HEIGHT_MAP.default;
-
-  switch (subject) {
-    case ProjectPermissionSub.DynamicSecrets:
-      height = 130;
-      break;
-    case ProjectPermissionSub.Secrets:
-      height = 85;
-      break;
-    default:
-      height = 64;
-  }
 
   return {
     type: PermissionNode.Folder,

frontend/src/context/ProjectPermissionContext/types.ts

@@ -163,7 +163,7 @@ export type IdentityManagementSubjectFields = {
 
 export const formatedConditionsOperatorNames: { [K in PermissionConditionOperators]: string } = {
   [PermissionConditionOperators.$EQ]: "equal to",
-  [PermissionConditionOperators.$IN]: "contains",
+  [PermissionConditionOperators.$IN]: "in",
   [PermissionConditionOperators.$ALL]: "contains all",
   [PermissionConditionOperators.$NEQ]: "not equal to",
   [PermissionConditionOperators.$GLOB]: "matches glob pattern",