fix(rbac): show configure-access cta for existing simple permission p…

…olicies in edit form (janus-idp#1702)
kim-tsao · May 31, 2024 · 16b7e00 · 16b7e00
1 parent f482086
commit 16b7e00
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 19 deletions.
diff --git a/plugins/rbac/src/types.ts b/plugins/rbac/src/types.ts
@@ -39,6 +39,7 @@ export type PermissionsDataSet = {
   permission: string;
   policies: Set<RowPolicy>;
   policyString?: Set<string>;
+  isResourced?: boolean;
 };
 
 export type PermissionsData = {

diff --git a/plugins/rbac/src/utils/rbac-utils.test.ts b/plugins/rbac/src/utils/rbac-utils.test.ts
@@ -21,7 +21,7 @@ import {
   getMembersFromGroup,
   getPermissions,
   getPermissionsData,
-  getPluginId,
+  getPluginInfo,
   getPoliciesData,
 } from './rbac-utils';
 
@@ -168,12 +168,21 @@ describe('rbac utils', () => {
   });
 
   it('should return plugin-id of the policy', () => {
-    expect(getPluginId(mockPermissionPolicies, 'catalog-entity')).toBe(
-      'catalog',
-    );
-    expect(getPluginId(mockPermissionPolicies, 'scaffolder-template')).toBe(
-      'scaffolder',
-    );
+    expect(
+      getPluginInfo(mockPermissionPolicies, 'catalog-entity').pluginId,
+    ).toBe('catalog');
+    expect(
+      getPluginInfo(mockPermissionPolicies, 'scaffolder-template').pluginId,
+    ).toBe('scaffolder');
+  });
+
+  it('should return if the permission is resourced', () => {
+    expect(
+      getPluginInfo(mockPermissionPolicies, 'catalog-entity').isResourced,
+    ).toBe(true);
+    expect(
+      getPluginInfo(mockPermissionPolicies, 'scaffolder-template').isResourced,
+    ).toBe(true);
   });
 
   it('should return kind, namespace and name from the reference', () => {
@@ -213,6 +222,7 @@ describe('rbac utils', () => {
         },
       ],
       policyString: ['Read', ', Create', ', Delete'],
+      isResourced: false,
     });
     data = getPermissionsData(mockPolicies, []);
     expect(data[0]).toEqual({
@@ -233,6 +243,7 @@ describe('rbac utils', () => {
         },
       ],
       policyString: ['Read', ', Create', ', Delete'],
+      isResourced: false,
     });
   });
 });

diff --git a/plugins/rbac/src/utils/rbac-utils.ts b/plugins/rbac/src/utils/rbac-utils.ts
@@ -108,12 +108,23 @@ export const getMembersFromGroup = (group: GroupEntity): number => {
   return membersList || 0;
 };
 
-export const getPluginId = (
+export const getPluginInfo = (
   permissions: PermissionPolicy[],
-  permission: string | undefined,
-): string =>
-  permissions.find(p => p.policies?.find(pol => pol.permission === permission))
-    ?.pluginId || '-';
+  permissionName?: string,
+): { pluginId: string; isResourced: boolean } =>
+  permissions.reduce(
+    (acc: { pluginId: string; isResourced: boolean }, p: PermissionPolicy) => {
+      const policy = p.policies?.find(pol => pol.permission === permissionName);
+      if (policy) {
+        return {
+          pluginId: p.pluginId || '-',
+          isResourced: policy?.isResourced || false,
+        };
+      }
+      return acc;
+    },
+    { pluginId: '-', isResourced: false },
+  );
 
 const getPolicy = (str: string) => {
   const arr = str.split('.');
@@ -174,12 +185,15 @@ export const getPermissionsData = (
           const policiesSet = new Set<{ policy: string; effect: string }>();
           acc.push({
             permission: policy.permission || '-',
-            plugin: getPluginId(permissionPolicies, policy?.permission) || '-',
+            plugin: getPluginInfo(permissionPolicies, policy?.permission)
+              .pluginId,
             policyString: policyString.add(policyTitleCase || 'Use'),
             policies: policiesSet.add({
               policy: policyTitleCase || 'Use',
               effect: policy.effect,
             }),
+            isResourced: getPluginInfo(permissionPolicies, policy?.permission)
+              .isResourced,
           });
         }
       }

diff --git a/plugins/rbac/tests/rbac.spec.ts b/plugins/rbac/tests/rbac.spec.ts
@@ -10,6 +10,10 @@ import {
 test.describe('RBAC plugin', () => {
   let page: Page;
   let common: Common;
+  const RoleOverviewPO = {
+    updatePolicies: 'span[data-testid="update-policies"]',
+    updateMembers: 'span[data-testid="update-members"]',
+  };
 
   test.beforeAll(async ({ browser }) => {
     const context = await browser.newContext();
@@ -91,19 +95,14 @@ test.describe('RBAC plugin', () => {
     await page.locator(`a`).filter({ hasText: 'RBAC' }).click();
   });
 
-  test('Edit user from overview page', async () => {
+  test('Edit an existing role', async () => {
     const roleName = 'role:default/rbac_admin';
     await page.locator(`a`).filter({ hasText: roleName }).click();
     await expect(page.getByRole('heading', { name: roleName })).toBeVisible({
       timeout: 20000,
     });
     await page.getByRole('tab', { name: 'Overview' }).click();
 
-    const RoleOverviewPO = {
-      updatePolicies: 'span[data-testid="update-policies"]',
-      updateMembers: 'span[data-testid="update-members"]',
-    };
-
     await page.locator(RoleOverviewPO.updateMembers).click();
     await expect(page.getByRole('heading', { name: 'Edit Role' })).toBeVisible({
       timeout: 20000,
@@ -243,4 +242,41 @@ test.describe('RBAC plugin', () => {
       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,
+    });
+
+    // update simple policy to add conditions
+    await page.getByText('Configure access', { exact: true }).click();
+    await page.getByPlaceholder('Select a rule').first().click();
+    await page.getByText('HAS_METADATA').click();
+    await page.getByLabel('key').fill('status');
+    await page.getByTestId('save-conditions').click();
+
+    expect(
+      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();
+  });
 });