feat(rbac): edit nested conditions (#1868)

* feat(rbac): edit nested conditions Signed-off-by: Yi Cai <yicai@redhat.com> * Fixed multi level nested not condition display Signed-off-by: Yi Cai <yicai@redhat.com> * Updated e2e tests Signed-off-by: Yi Cai <yicai@redhat.com> * Hide nested condition block on multi-level nested conditions Signed-off-by: Yi Cai <yicai@redhat.com> --------- Signed-off-by: Yi Cai <yicai@redhat.com>
janus-idp · Aug 13, 2024 · 738783b · 738783b
1 parent b2081bc
commit 738783b
Show file tree

Hide file tree

Showing 6 changed files with 269 additions and 181 deletions.
diff --git a/plugins/rbac/src/__fixtures__/mockConditions.ts b/plugins/rbac/src/__fixtures__/mockConditions.ts
@@ -111,6 +111,26 @@ export const mockConditions: RoleConditionalPolicyDecision<PermissionAction>[] =
               },
             ],
           },
+          {
+            not: {
+              allOf: [
+                {
+                  rule: 'IS_ENTITY_OWNER',
+                  resourceType: 'catalog-entity',
+                  params: {
+                    claims: ['user:default/xyz'],
+                  },
+                },
+                {
+                  rule: 'IS_ENTITY_KIND',
+                  resourceType: 'catalog-entity',
+                  params: {
+                    kinds: ['User'],
+                  },
+                },
+              ],
+            },
+          },
         ],
       },
       roleEntityRef: 'role:default/rbac_admin',

diff --git a/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx b/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx
@@ -178,18 +178,25 @@ export const ConditionsForm = ({
         const nestedConditionCriteria = Object.keys(
           firstLevelNestedCondition,
         )[0];
-        return (
+        if (
           Array.isArray(
             firstLevelNestedCondition[
               nestedConditionCriteria as keyof Condition
             ],
-          ) &&
-          (
+          )
+        ) {
+          return (
             firstLevelNestedCondition[
               nestedConditionCriteria as keyof Condition
             ] as Condition[]
-          ).some((con: Condition) => !('rule' in con))
-        );
+          ).some((con: Condition) => !('rule' in con));
+        }
+
+        return !Object.keys(
+          firstLevelNestedCondition[
+            nestedConditionCriteria as keyof Condition
+          ] as Condition[],
+        ).includes('rule');
       });
   };
 

diff --git a/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx b/plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx
@@ -16,9 +16,10 @@ import Button from '@mui/material/Button';
 import ToggleButtonGroup from '@mui/material/ToggleButtonGroup';
 
 import {
-  calculateConditionIndex,
   extractNestedConditions,
   getDefaultRule,
+  getNestedConditionSimpleRulesCount,
+  getSimpleRulesCount,
   isNestedConditionRule,
   isSimpleRule,
   makeConditionsFormRowStyles,
@@ -363,122 +364,128 @@ export const ConditionsFormRow = ({
     nestedConditionIndex: number,
   ) => {
     const selectedNestedConditionCriteria = Object.keys(nc)[0];
-    const simpleRulesCount = calculateConditionIndex(
-      conditionRow,
-      nestedConditionIndex,
-      criteria,
-    );
+    const simpleRulesCount = getSimpleRulesCount(conditionRow, criteria);
     const nestedConditionsCount = nestedConditionRow.length;
+    const nestedConditionSimpleRulesCount = getNestedConditionSimpleRulesCount(
+      nc,
+      selectedNestedConditionCriteria,
+    );
     return (
-      <Box
-        mt={2}
-        className={classes.nestedConditionRow}
-        key={`nestedCondition-${nestedConditionIndex}`}
-      >
-        <div style={{ display: 'flex', justifyContent: 'space-between' }}>
-          <ToggleButtonGroup
-            exclusive
-            value={selectedNestedConditionCriteria}
-            onChange={(_event, newNestedCriteria) =>
-              handleNestedConditionCriteriaChange(
-                newNestedCriteria,
-                nestedConditionIndex,
-              )
-            }
-            className={classes.nestedConditioncriteriaButtonGroup}
-          >
-            {nestedConditionButtons.map(({ val, label }) => (
-              <CriteriaToggleButton
-                key={`nested-criteria-${val}`}
-                val={val}
-                label={label}
-                selectedCriteria={selectedNestedConditionCriteria}
-                theme={theme}
-              />
-            ))}
-          </ToggleButtonGroup>
-          {criteria !== criterias.not && (
-            <IconButton
-              title="Remove nested condition"
-              className={classes.removeNestedRuleButton}
-              disabled={simpleRulesCount === 0 && nestedConditionsCount === 1} // 0 simple rules and this is the only 1 nested condition
-              onClick={() => handleRemoveNestedCondition(nestedConditionIndex)}
-            >
-              <RemoveIcon />
-            </IconButton>
-          )}
-        </div>
-        <Box>
-          {selectedNestedConditionCriteria !== criterias.not &&
-            (
-              nc[
-                selectedNestedConditionCriteria as keyof Condition
-              ] as PermissionCondition[]
-            ).map((c, ncrIndex) => (
-              <ComplexConditionRow
-                key={`nested-condition-${nestedConditionIndex}-${ncrIndex}`}
-                conditionRow={conditionRow}
-                nestedConditionRow={nestedConditionRow}
-                criteria={criteria}
-                onRuleChange={onRuleChange}
-                updateRules={updateRules}
-                setErrors={setErrors}
-                setRemoveAllClicked={setRemoveAllClicked}
-                conditionRulesData={conditionRulesData}
-                notConditionType={notConditionType}
-                classes={classes}
-                currentCondition={c}
-                ruleIndex={ncrIndex}
-                isNestedCondition
-                nestedConditionIndex={nestedConditionIndex}
-                activeNestedCriteria={
-                  selectedNestedConditionCriteria as 'allOf' | 'anyOf'
-                }
-              />
-            ))}
-          {selectedNestedConditionCriteria === criterias.not && (
-            <ConditionsFormRowFields
-              oldCondition={
-                (nc as ConditionsData).not ??
-                getDefaultRule(selPluginResourceType)
-              }
-              onRuleChange={onRuleChange}
-              conditionRow={conditionRow}
-              criteria={criteria}
-              conditionRulesData={conditionRulesData}
-              setErrors={setErrors}
-              optionDisabled={ruleOption =>
-                ruleOptionDisabled(
-                  ruleOption,
-                  (nc as ConditionsData).not
-                    ? [(nc as ConditionsData).not as PermissionCondition]
-                    : undefined,
-                )
-              }
-              setRemoveAllClicked={setRemoveAllClicked}
-              nestedConditionRow={nestedConditionRow}
-              nestedConditionCriteria={selectedNestedConditionCriteria}
-              nestedConditionIndex={nestedConditionIndex}
-              updateRules={updateRules}
-            />
-          )}
-          {selectedNestedConditionCriteria !== criterias.not && (
-            <Button
-              className={classes.addRuleButton}
-              size="small"
-              onClick={() =>
-                handleAddRuleInNestedCondition(
-                  selectedNestedConditionCriteria,
+      nestedConditionSimpleRulesCount > 0 && (
+        <Box
+          mt={2}
+          className={classes.nestedConditionRow}
+          key={`nestedCondition-${nestedConditionIndex}`}
+        >
+          <div style={{ display: 'flex', justifyContent: 'space-between' }}>
+            <ToggleButtonGroup
+              exclusive
+              value={selectedNestedConditionCriteria}
+              onChange={(_event, newNestedCriteria) =>
+                handleNestedConditionCriteriaChange(
+                  newNestedCriteria,
                   nestedConditionIndex,
                 )
               }
+              className={classes.nestedConditioncriteriaButtonGroup}
             >
-              <AddIcon fontSize="small" />
-              Add rule
-            </Button>
-          )}
+              {nestedConditionButtons.map(({ val, label }) => (
+                <CriteriaToggleButton
+                  key={`nested-criteria-${val}`}
+                  val={val}
+                  label={label}
+                  selectedCriteria={selectedNestedConditionCriteria}
+                  theme={theme}
+                />
+              ))}
+            </ToggleButtonGroup>
+            {criteria !== criterias.not && (
+              <IconButton
+                title="Remove nested condition"
+                className={classes.removeNestedRuleButton}
+                disabled={simpleRulesCount === 0 && nestedConditionsCount === 1} // 0 simple rules and this is the only 1 nested condition
+                onClick={() =>
+                  handleRemoveNestedCondition(nestedConditionIndex)
+                }
+              >
+                <RemoveIcon data-testid="remove-nested-condition" />
+              </IconButton>
+            )}
+          </div>
+          <Box>
+            {selectedNestedConditionCriteria !== criterias.not &&
+              (
+                nc[
+                  selectedNestedConditionCriteria as keyof Condition
+                ] as PermissionCondition[]
+              ).map((c, ncrIndex) => (
+                <ComplexConditionRow
+                  key={`nested-condition-${nestedConditionIndex}-${ncrIndex}`}
+                  conditionRow={conditionRow}
+                  nestedConditionRow={nestedConditionRow}
+                  criteria={criteria}
+                  onRuleChange={onRuleChange}
+                  updateRules={updateRules}
+                  setErrors={setErrors}
+                  setRemoveAllClicked={setRemoveAllClicked}
+                  conditionRulesData={conditionRulesData}
+                  notConditionType={notConditionType}
+                  classes={classes}
+                  currentCondition={c}
+                  ruleIndex={ncrIndex}
+                  isNestedCondition
+                  nestedConditionIndex={nestedConditionIndex}
+                  activeNestedCriteria={
+                    selectedNestedConditionCriteria as 'allOf' | 'anyOf'
+                  }
+                />
+              ))}
+            {selectedNestedConditionCriteria === criterias.not &&
+              ((nc as ConditionsData).not as PermissionCondition)
+                .resourceType && (
+                <ConditionsFormRowFields
+                  oldCondition={
+                    (nc as ConditionsData).not ??
+                    getDefaultRule(selPluginResourceType)
+                  }
+                  onRuleChange={onRuleChange}
+                  conditionRow={conditionRow}
+                  criteria={criteria}
+                  conditionRulesData={conditionRulesData}
+                  setErrors={setErrors}
+                  optionDisabled={ruleOption =>
+                    ruleOptionDisabled(
+                      ruleOption,
+                      (nc as ConditionsData).not
+                        ? [(nc as ConditionsData).not as PermissionCondition]
+                        : undefined,
+                    )
+                  }
+                  setRemoveAllClicked={setRemoveAllClicked}
+                  nestedConditionRow={nestedConditionRow}
+                  nestedConditionCriteria={selectedNestedConditionCriteria}
+                  nestedConditionIndex={nestedConditionIndex}
+                  updateRules={updateRules}
+                />
+              )}
+            {selectedNestedConditionCriteria !== criterias.not && (
+              <Button
+                className={classes.addRuleButton}
+                size="small"
+                onClick={() =>
+                  handleAddRuleInNestedCondition(
+                    selectedNestedConditionCriteria,
+                    nestedConditionIndex,
+                  )
+                }
+              >
+                <AddIcon fontSize="small" />
+                Add rule
+              </Button>
+            )}
+          </Box>
         </Box>
-      </Box>
+      )
     );
   };
 

diff --git a/plugins/rbac/src/components/CreateRole/PermissionPoliciesFormRow.tsx b/plugins/rbac/src/components/CreateRole/PermissionPoliciesFormRow.tsx
@@ -83,6 +83,15 @@ export const PermissionPoliciesFormRow = ({
     </div>
   );
 
+  const getTotalRules = (): string => {
+    let accessMessage = 'Configure access';
+
+    if (totalRules > 0) {
+      accessMessage += ` (${totalRules} ${totalRules > 1 ? 'rules' : 'rule'})`;
+    }
+    return accessMessage;
+  };
+
   return (
     <div>
       <div style={{ display: 'flex', flexFlow: 'column', gap: '15px' }}>
@@ -180,11 +189,7 @@ export const PermissionPoliciesFormRow = ({
                   disabled={!!conditionRulesError}
                 >
                   <ChecklistRtlIcon fontSize="small" />
-                  {totalRules > 0
-                    ? `Configure access (${totalRules} ${
-                        totalRules > 1 ? `rules` : 'rule'
-                      })`
-                    : 'Configure access'}
+                  {getTotalRules()}
                   &nbsp;
                   <Tooltip title={tooltipTitle()} placement="top">
                     <HelpOutlineIcon fontSize="inherit" />

diff --git a/plugins/rbac/src/utils/conditional-access-utils.ts b/plugins/rbac/src/utils/conditional-access-utils.ts
@@ -175,19 +175,22 @@ export const makeConditionsFormRowFieldsStyles = makeStyles<Theme, StyleProps>(
   }),
 );
 
-export const calculateConditionIndex = (
+export const getSimpleRulesCount = (
   conditionRow: ConditionsData,
-  nestedConditionIndex: number,
   criteria: string,
 ): number => {
-  const simpleRulesCount =
-    criteria === criterias.not
-      ? 0
-      : (
-          (conditionRow[criteria as keyof Condition] as Condition[]) || []
-        ).filter((e: Condition) => 'rule' in e).length;
-
-  return simpleRulesCount + nestedConditionIndex;
+  if (criteria === criterias.not) {
+    return (conditionRow[criteria as keyof Condition] as PermissionCondition)
+      .resourceType
+      ? 1
+      : 0;
+  }
+  if (criteria === criterias.condition) {
+    return 1;
+  }
+  return (conditionRow[criteria as keyof Condition] as Condition[]).filter(
+    (e: Condition) => 'rule' in e,
+  ).length;
 };
 
 export const initializeErrors = (
@@ -293,6 +296,21 @@ export const isNestedConditionRule = (r: Condition): boolean => {
   );
 };
 
+export const getNestedConditionSimpleRulesCount = (
+  nc: Condition,
+  c: string,
+): number => {
+  if (c === criterias.not) {
+    return (nc[c as keyof Condition] as PermissionCondition).resourceType
+      ? 1
+      : 0;
+  }
+
+  return (nc[c as keyof Condition] as Condition[]).filter(
+    r => 'resourceType' in r,
+  ).length;
+};
+
 export const getRowStyle = (c: Condition, isNestedCondition: boolean) =>
   isNestedCondition
     ? {