Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(rbac): edit nested conditions #1868

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+153 −88
Open

feat(rbac): edit nested conditions #1868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac3856d
feat(rbac): edit nested conditions
ciiay Aug 6, 2024
ee2b488
Fixed multi level nested not condition display
ciiay Aug 8, 2024
c23d845
Updated e2e tests
ciiay Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions plugins/rbac/src/__fixtures__/mockConditions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
17 changes: 12 additions & 5 deletions plugins/rbac/src/components/ConditionalAccess/ConditionsForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
};

Expand Down
56 changes: 29 additions & 27 deletions plugins/rbac/src/components/ConditionalAccess/ConditionsFormRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ export const ConditionsFormRow = ({
disabled={simpleRulesCount === 0 && nestedConditionsCount === 1} // 0 simple rules and this is the only 1 nested condition
onClick={() => handleRemoveNestedCondition(nestedConditionIndex)}
>
<RemoveIcon />
<RemoveIcon data-testid="remove-nested-condition" />
</IconButton>
)}
</div>
Expand Down Expand Up @@ -436,32 +436,34 @@ export const ConditionsFormRow = ({
}
/>
))}
{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 &&
((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}
Expand Down
15 changes: 10 additions & 5 deletions plugins/rbac/src/components/CreateRole/PermissionPoliciesFormRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}>
Expand Down Expand Up @@ -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" />
Expand Down
133 changes: 82 additions & 51 deletions plugins/rbac/tests/rbac.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,33 @@ test.describe('RBAC plugin', () => {
updateMembers: 'span[data-testid="update-members"]',
};

const navigateToRole = async (roleName: string) => {
await expect(
page.getByRole('heading', { name: 'All roles (2)' }),
).toBeVisible({ timeout: 20000 });
await page
.locator(`a`)
.filter({ hasText: `role:default/${roleName}` })
.click();
await expect(
page.getByRole('heading', { name: `role:default/${roleName}` }),
).toBeVisible({ timeout: 20000 });
await page.getByRole('tab', { name: 'Overview' }).click();
await page.locator(RoleOverviewPO.updatePolicies).click();
await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({
timeout: 20000,
});
};

const finishAndVerifyUpdate = async (button: string, message: string) => {
await common.clickButton('Next');
await common.clickButton(button);
await verifyText(message, page);
if (button === 'Save') {
await page.locator(`a`).filter({ hasText: 'RBAC' }).click();
}
};

test.beforeAll(async ({ browser }) => {
const context = await browser.newContext();
page = await context.newPage();
Expand Down Expand Up @@ -233,34 +260,14 @@ test.describe('RBAC plugin', () => {
await expect(page.getByText('Configure access (2 rules)')).toBeVisible({
timeout: 20000,
});

await common.clickButton('Next');

await common.clickButton('Create');
await verifyText(
await finishAndVerifyUpdate(
'Create',
'Role role:default/sample-role-1 created successfully',
page,
);
});

test('Edit role to convert simple policy into conditional policy', async () => {
await expect(
page.getByRole('heading', { name: 'All roles (2)' }),
).toBeVisible({ timeout: 20000 });

// edit/update policies
await page.locator(`a`).filter({ hasText: 'role:default/guests' }).click();
await expect(
page.getByRole('heading', { name: 'role:default/guests' }),
).toBeVisible({
timeout: 20000,
});
await page.getByRole('tab', { name: 'Overview' }).click();

await page.locator(RoleOverviewPO.updatePolicies).click();
await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({
timeout: 20000,
});
navigateToRole('guests');

// update simple policy to add conditions
await page.getByText('Configure access', { exact: true }).click();
Expand All @@ -273,33 +280,15 @@ test.describe('RBAC plugin', () => {
page.getByText('Configure access (1 rule)', { exact: true }),
).toBeVisible();

await common.clickButton('Next');
await common.clickButton('Save');
await verifyText('Role role:default/guests updated successfully', page);

await page.locator(`a`).filter({ hasText: 'RBAC' }).click();
finishAndVerifyUpdate(
'Save',
'Role role:default/guests updated successfully',
);
});

test('Edit role to convert conditional policy into nested conditional policy', async () => {
await expect(
page.getByRole('heading', { name: 'All roles (2)' }),
).toBeVisible({ timeout: 20000 });

// edit/update policies
await page.locator(`a`).filter({ hasText: 'role:default/guests' }).click();
await expect(
page.getByRole('heading', { name: 'role:default/guests' }),
).toBeVisible({
timeout: 20000,
});
await page.getByRole('tab', { name: 'Overview' }).click();

await page.locator(RoleOverviewPO.updatePolicies).click();
await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({
timeout: 20000,
});
await navigateToRole('guests');

// update conditional policy to nested conditions
await page.getByText('Configure access', { exact: true }).click();
await page.getByText('AllOf', { exact: true }).click();
await page.getByPlaceholder('Select a rule').first().click();
Expand All @@ -311,14 +300,56 @@ test.describe('RBAC plugin', () => {
await page.getByLabel('key').fill('status');
await page.getByTestId('save-conditions').click();

expect(
await expect(
page.getByText('Configure access (2 rules)', { exact: true }),
).toBeVisible();

await common.clickButton('Next');
await common.clickButton('Save');
await verifyText('Role role:default/guests updated successfully', page);
await finishAndVerifyUpdate(
'Save',
'Role role:default/guests updated successfully',
);
});

await page.locator(`a`).filter({ hasText: 'RBAC' }).click();
test('Edit existing nested conditional policy', async () => {
await navigateToRole('rbac_admin');

await page.getByText('Configure access (9 rules)', { exact: true }).click();
await expect(page.getByText('AllOf')).toHaveCount(3, { timeout: 20000 });
await page.getByText('Add nested condition').click();
await page.getByText('Not', { exact: true }).last().click();
await page.getByPlaceholder('Select a rule').last().click();
await page.getByText('HAS_LABEL').last().click();
await page.getByLabel('label').last().fill('test');
await page.getByTestId('save-conditions').click();

await expect(
page.getByText('Configure access (10 rules)', { exact: true }),
).toBeVisible();

await finishAndVerifyUpdate(
'Save',
'Role role:default/rbac_admin updated successfully',
);
});

test('Remove existing nested conditional policy', async () => {
await navigateToRole('rbac_admin');

await expect(
page.getByText('Configure access (9 rules)', { exact: true }),
).toHaveCount(1);
await page.getByText('Configure access (9 rules)', { exact: true }).click();
await expect(page.getByText('AllOf')).toHaveCount(3, { timeout: 20000 });
await page.getByTestId('remove-nested-condition').last().click();
await page.getByTestId('save-conditions').click();

await expect(
page.getByText('Configure access (2 rules)', { exact: true }),
).toHaveCount(2);

await finishAndVerifyUpdate(
'Save',
'Role role:default/rbac_admin updated successfully',
);
});
});