From da792d144d621ef36a1a674eac8c0e9856e80dfe Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Tue, 17 Jun 2025 09:29:17 -0700 Subject: [PATCH] improvements: address feedback --- .../components/FolderNodeTooltipContent.tsx | 15 +- .../AccessTree/utils/createFolderNode.ts | 247 +++++++++--------- .../context/ProjectPermissionContext/types.ts | 2 +- 3 files changed, 123 insertions(+), 141 deletions(-) diff --git a/frontend/src/components/permissions/AccessTree/nodes/FolderNode/components/FolderNodeTooltipContent.tsx b/frontend/src/components/permissions/AccessTree/nodes/FolderNode/components/FolderNodeTooltipContent.tsx index e96c6862e..8af3241f0 100644 --- a/frontend/src/components/permissions/AccessTree/nodes/FolderNode/components/FolderNodeTooltipContent.tsx +++ b/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 (
  • {camelCaseToSpaces(key)}{" "} {formatedConditionsOperatorNames[operator as PermissionConditionOperators]} {" "} - + {typeof value === "string" ? value : value.join(", ")} . @@ -87,9 +86,7 @@ export const FolderNodeTooltipContent = ({ action, access, actionRuleMap, subjec ) { return (
  • - - {rule.inverted ? "Forbids" : "Allows"} - + {rule.inverted ? "Forbids" : "Allows"} when: {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]) => ( - = { ] }; +const SUBJECT_HEIGHT_MAP: Record = { + [ProjectPermissionSub.DynamicSecrets]: 130, + [ProjectPermissionSub.Secrets]: 85, + default: 64 +}; + const evaluateCondition = ( value: string, operator: PermissionConditionOperators, @@ -52,6 +58,105 @@ const evaluateCondition = ( } }; +const doesConditionMatch = ( + conditions: Record | 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, + 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 ( - 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) - ); - } + const hasPermission = checkPermission(permissions, subject, action, subjectFields); - if (hasPermission) { - if (subject === ProjectPermissionSub.Secrets) { - // we want to show yellow/conditional access if user hasn't specified secret name to fully resolve access - if ( - !secretName && - actionRuleMap.some((el) => { - // we only show conditional if secretName/secretTags are present - environment and path can be directly determined - if (!el[action]?.conditions?.secretName && !el[action]?.conditions?.secretTags) - 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 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; - } + access = determineAccessLevel( + hasPermission, + subject, + action, + actionRuleMap, + environment, + folder.path, + secretName, + metadata + ); } catch (e) { console.error(e); access = PermissionAccess.None; @@ -211,18 +209,7 @@ export const createFolderNode = ({ }) ); - let height: number; - - switch (subject) { - case ProjectPermissionSub.DynamicSecrets: - height = 130; - break; - case ProjectPermissionSub.Secrets: - height = 85; - break; - default: - height = 64; - } + const height = SUBJECT_HEIGHT_MAP[subject] ?? SUBJECT_HEIGHT_MAP.default; return { type: PermissionNode.Folder, diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index f9b2a828a..9927d6de7 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/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",