From 9806587d2f21ccf7b4b0cb4a5b5801f8cfa3f6b1 Mon Sep 17 00:00:00 2001 From: Yi Cai Date: Fri, 28 Jun 2024 17:32:00 -0400 Subject: [PATCH] Addressed review comments Signed-off-by: Yi Cai --- .../rbac/src/__fixtures__/mockConditions.ts | 21 ++ .../ConditionalAccess/ConditionsForm.tsx | 1 + .../ConditionalAccess/ConditionsFormRow.tsx | 215 ++++++++++++------ .../CreateRole/PermissionPoliciesFormRow.tsx | 16 +- 4 files changed, 174 insertions(+), 79 deletions(-) diff --git a/plugins/rbac/src/__fixtures__/mockConditions.ts b/plugins/rbac/src/__fixtures__/mockConditions.ts index 5770d53790..ddcacd266f 100644 --- a/plugins/rbac/src/__fixtures__/mockConditions.ts +++ b/plugins/rbac/src/__fixtures__/mockConditions.ts @@ -77,6 +77,27 @@ export const mockConditions: RoleConditionalPolicyDecision[] = kinds: ['User'], }, }, + { + not: { + rule: 'HAS_LABEL', + resourceType: 'catalog-entity', + params: { label: 'temp' }, + }, + }, + { + anyOf: [ + { + rule: 'HAS_TAG', + resourceType: 'catalog-entity', + params: { tag: 'dev' }, + }, + { + rule: 'HAS_TAG', + resourceType: 'catalog-entity', + params: { tag: 'test' }, + }, + ], + }, ], }, ], diff --git a/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx b/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx index 3e632240d9..f2108cb2f5 100644 --- a/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx +++ b/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx @@ -22,6 +22,7 @@ const useStyles = makeStyles(theme => ({ paddingTop: theme.spacing(1), paddingBottom: theme.spacing(1), flexGrow: 1, + overflowY: 'auto', }, addConditionButton: { color: theme.palette.primary.light, diff --git a/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx b/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx index b0a2c1eb66..4930ad4a85 100644 --- a/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx +++ b/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx @@ -8,9 +8,11 @@ import { makeStyles, Radio, RadioGroup, + Tooltip, useTheme, } from '@material-ui/core'; import AddIcon from '@mui/icons-material/Add'; +import HelpOutlineIcon from '@mui/icons-material/HelpOutline'; import RemoveIcon from '@mui/icons-material/Remove'; import Box from '@mui/material/Box'; import Button from '@mui/material/Button'; @@ -34,7 +36,7 @@ const useStyles = makeStyles(theme => ({ }, nestedConditionRow: { padding: '20px', - marginLeft: '20px', + marginLeft: '1.5rem', border: `1px solid ${theme.palette.border}`, borderRadius: '4px', backgroundColor: theme.palette.background.default, @@ -60,13 +62,19 @@ const useStyles = makeStyles(theme => ({ display: 'flex', color: theme.palette.primary.light, textTransform: 'none', - marginTop: theme.spacing(1), + marginTop: theme.spacing(2), }, addNestedConditionButton: { display: 'flex', color: theme.palette.primary.light, textTransform: 'none', marginTop: theme.spacing(1), + marginBottom: theme.spacing(2), + }, + addNestedConditionLabel: { + display: 'flex', + justifyContent: 'center', + alignItems: 'center', }, removeRuleButton: { color: theme.palette.grey[500], @@ -463,6 +471,34 @@ export const ConditionsFormRow = ({ ); }; + const addNestedConditionLabel = () => { + const tooltipTitle = () => ( +
+

+ Nested conditions are 1 layer rules within a main condition. It + lets you allow appropriate access by using detailed permissions based + on various conditions. You can add multiple nested conditions. +

+

+ For example, you can allow access to all entity types in the main + condition and use a nested condition to limit the access to entities + owned by the user. +

+
+ ); + return ( + + Add Nested Condition + + + + + ); + }; + let showMultilevelNestedConditionWarning = false; return ( } - label="Add nested condition" + label={addNestedConditionLabel()} className={classes.radioLabel} /> @@ -652,7 +688,7 @@ export const ConditionsFormRow = ({ onClick={() => handleAddNestedCondition(criteria)} > - Add nested condition + {addNestedConditionLabel()} )} {nestedConditionRow?.length > 0 && @@ -712,79 +748,101 @@ export const ConditionsFormRow = ({ {Object.keys(nc)[0] === criterias.allOf && - nc.allOf?.map((c, index) => ( -
- - - handleRemoveNestedConditionRule( - criterias.allOf, - nestedConditionIndex, - index, - ) - } - > - - -
- ))} + nc.allOf?.map((c, index) => { + if ((c as PermissionCondition).rule === undefined) { + showMultilevelNestedConditionWarning = true; + } + return ( + (c as PermissionCondition).rule !== undefined && ( +
+ + + handleRemoveNestedConditionRule( + criterias.allOf, + nestedConditionIndex, + index, + ) + } + > + + +
+ ) + ); + })} {Object.keys(nc)[0] === criterias.anyOf && - nc.anyOf?.map((c, index) => ( -
- - - handleRemoveNestedConditionRule( - criterias.anyOf, - nestedConditionIndex, - index, - ) - } + nc.anyOf?.map((c, index) => { + if ((c as PermissionCondition).rule === undefined) { + showMultilevelNestedConditionWarning = true; + } + return ( +
- - -
- ))} + + + handleRemoveNestedConditionRule( + criterias.anyOf, + nestedConditionIndex, + index, + ) + } + > + + +
+ ); + })} {Object.keys(nc)[0] === criterias.not && ( )} + {showMultilevelNestedConditionWarning && ( +
+ This condition contains multiple nested levels that the UI + does not yet support. Please use the CLI to view + additional levels of nested conditions. +
+ )} {Object.keys(nc)[0] !== criterias.not && (