feat(rbac): cleanup policies when a role is deleted (#1018)

janus-idp · Dec 20, 2023 · fb0ee8c · fb0ee8c
1 parent 9cd3936
commit fb0ee8c
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 26 deletions.
diff --git a/packages/backend/package.json b/packages/backend/package.json
@@ -36,7 +36,7 @@
     "@backstage/plugin-search-backend-module-pg": "^0.5.15",
     "@backstage/plugin-search-backend-node": "^1.2.10",
     "@backstage/plugin-techdocs-backend": "^1.8.0",
-    "@janus-idp/backstage-plugin-rbac-backend": "^1.6.4",
+    "@janus-idp/backstage-plugin-rbac-backend": "^2.0.0",
     "better-sqlite3": "^9.0.0",
     "dockerode": "^4.0.0",
     "express": "^4.18.2",

diff --git a/plugins/rbac/dev/index.tsx b/plugins/rbac/dev/index.tsx
@@ -44,16 +44,16 @@ class MockRBACApi implements RBACAPI {
     return this.resources;
   }
 
-  async getPolicies(): Promise<RoleBasedPolicy[]> {
-    return mockPolicies;
-  }
-
   async getAssociatedPolicies(
     entityReference: string,
   ): Promise<RoleBasedPolicy[]> {
     return mockPolicies.filter(pol => pol.entityReference === entityReference);
   }
 
+  async getPolicies(): Promise<RoleBasedPolicy[]> {
+    return mockPolicies;
+  }
+
   async getUserAuthorization(): Promise<{ status: string }> {
     return {
       status: 'Authorized',
@@ -90,12 +90,15 @@ class MockRBACApi implements RBACAPI {
     return mockPermissionPolicies;
   }
 
-  async deletePolicy(
+  async deletePolicies(
     _entityRef: string,
-    _permission: string,
     _policies: Policy[],
-  ): Promise<number> {
-    return 204;
+  ): Promise<Response> {
+    return {
+      ok: true,
+      status: 204,
+      statusText: 'Deleted Successfully',
+    } as Response;
   }
 
   async createRole(_role: Role): Promise<Response> {

diff --git a/plugins/rbac/src/api/RBACBackendClient.ts b/plugins/rbac/src/api/RBACBackendClient.ts
@@ -6,6 +6,7 @@ import {
 
 import {
   PermissionPolicy,
+  Policy,
   Role,
   RoleBasedPolicy,
 } from '@janus-idp/backstage-plugin-rbac-common';
@@ -22,6 +23,7 @@ export type RBACAPI = {
     entityReference: string,
   ) => Promise<RoleBasedPolicy[] | Response>;
   deleteRole: (role: string) => Promise<Response>;
+  deletePolicies: (role: string, policy: Policy[]) => Promise<Response>;
   getRole: (role: string) => Promise<Role[] | Response>;
   getMembers: () => Promise<MemberEntity[] | Response>;
   listPermissions: () => Promise<PermissionPolicy[]>;
@@ -247,4 +249,23 @@ export class RBACBackendClient implements RBACAPI {
     }
     return jsonResponse;
   }
+
+  async deletePolicies(entityReference: string, policies: Policy[]) {
+    const { token: idToken } = await this.identityApi.getCredentials();
+    const backendUrl = this.configApi.getString('backend.baseUrl');
+    const { kind, namespace, name } = getKindNamespaceName(entityReference);
+    const jsonResponse = await fetch(
+      `${backendUrl}/api/permission/policies/${kind}/${namespace}/${name}`,
+      {
+        headers: {
+          ...(idToken && { Authorization: `Bearer ${idToken}` }),
+          'Content-Type': 'application/json',
+          Accept: 'application/json',
+        },
+        body: JSON.stringify(policies),
+        method: 'DELETE',
+      },
+    );
+    return jsonResponse;
+  }
 }
diff --git a/plugins/rbac/src/components/DeleteRole.tsx b/plugins/rbac/src/components/DeleteRole.tsx
@@ -29,7 +29,7 @@ const DeleteRole = ({
     <Tooltip title={tooltip || ''}>
       <span data-testid={dataTestId}>
         <IconButton
-          color="primary"
+          color="inherit"
           onClick={() => openDialog(roleName)}
           aria-label="Delete"
           disabled={disable}

diff --git a/plugins/rbac/src/components/DeleteRoleDialog.tsx b/plugins/rbac/src/components/DeleteRoleDialog.tsx
@@ -19,6 +19,8 @@ import CloseIcon from '@material-ui/icons/Close';
 import ErrorIcon from '@material-ui/icons/Error';
 import { Alert } from '@material-ui/lab';
 
+import { RoleBasedPolicy } from '@janus-idp/backstage-plugin-rbac-common';
+
 import { rbacApiRef } from '../api/RBACBackendClient';
 import { getMembers } from '../utils/rbac-utils';
 import { useToast } from './ToastContext';
@@ -65,12 +67,32 @@ const DeleteRoleDialog = ({
 
   const deleteRole = async () => {
     try {
+      const policies = await rbacApi.getAssociatedPolicies(roleName);
+      let cleanupPoliciesResponse;
+      if (Array.isArray(policies)) {
+        const allowedPolicies = policies.filter(
+          (policy: RoleBasedPolicy) => policy.effect !== 'deny',
+        );
+        if (allowedPolicies.length > 0) {
+          cleanupPoliciesResponse = await rbacApi.deletePolicies(
+            roleName,
+            allowedPolicies,
+          );
+        }
+        if (cleanupPoliciesResponse && !cleanupPoliciesResponse.ok) {
+          setError(
+            `Unable to delete Policies. ${cleanupPoliciesResponse.statusText}`,
+          );
+          return;
+        }
+      }
+
       const response = await rbacApi.deleteRole(roleName);
       if (response.status === 200 || response.status === 204) {
         setToastMessage(`Role ${roleName} deleted successfully`);
         closeDialog();
       } else {
-        setError(response.statusText);
+        setError(`Unable to delete the role. ${response.statusText}`);
       }
     } catch (err) {
       setError(err instanceof Error ? err.message : `${err}`);
@@ -147,7 +169,7 @@ const DeleteRoleDialog = ({
       </DialogContent>
       {error && (
         <Box maxWidth="650px" marginLeft="20px">
-          <Alert severity="error">{`Unable to delete. ${error}`}</Alert>
+          <Alert severity="error">{error}</Alert>
         </Box>
       )}
       <DialogActions

diff --git a/plugins/rbac/src/components/EditRole.tsx b/plugins/rbac/src/components/EditRole.tsx
@@ -27,6 +27,7 @@ const EditRole = ({
     <Tooltip title={tooltip || ''}>
       <span data-testid={dataTestId}>
         <IconButton
+          color="inherit"
           component={Link}
           aria-label="Update"
           disabled={disable}

diff --git a/plugins/rbac/src/components/RbacPage.test.tsx b/plugins/rbac/src/components/RbacPage.test.tsx
@@ -37,7 +37,7 @@ describe('RbacPage', () => {
     mockUseRoles.mockReturnValue({
       loading: true,
       data: [],
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     await renderInTestApp(<RbacPage />);

diff --git a/plugins/rbac/src/components/RolesList.test.tsx b/plugins/rbac/src/components/RolesList.test.tsx
@@ -63,7 +63,7 @@ describe('RolesList', () => {
     mockUseRoles.mockReturnValue({
       loading: false,
       data: useRolesMockData,
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     const { queryByText } = await renderInTestApp(<RolesList />);
@@ -79,7 +79,7 @@ describe('RolesList', () => {
     mockUseRoles.mockReturnValue({
       loading: false,
       data: [],
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     const { getByTestId } = await renderInTestApp(<RolesList />);
@@ -94,7 +94,7 @@ describe('RolesList', () => {
     mockUseRoles.mockReturnValue({
       loading: false,
       data: useRolesMockData,
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     const { getAllByTestId, getByText } = await renderInTestApp(<RolesList />);
@@ -125,7 +125,7 @@ describe('RolesList', () => {
           },
         },
       ],
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     const { getAllByTestId } = await renderInTestApp(<RolesList />);
@@ -156,7 +156,7 @@ describe('RolesList', () => {
           },
         },
       ],
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: true,
     });
     const { getAllByTestId } = await renderInTestApp(<RolesList />);
@@ -170,7 +170,7 @@ describe('RolesList', () => {
     mockUseRoles.mockReturnValue({
       loading: false,
       data: useRolesMockData,
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: false,
     });
     const { getByTestId } = await renderInTestApp(<RolesList />);
@@ -186,7 +186,7 @@ describe('RolesList', () => {
     mockUseRoles.mockReturnValue({
       loading: false,
       data: useRolesMockData,
-      retry: () => {},
+      retry: jest.fn(),
       createRoleAllowed: true,
     });
     const { getByTestId } = await renderInTestApp(<RolesList />);

diff --git a/plugins/rbac/src/hooks/useRoles.ts b/plugins/rbac/src/hooks/useRoles.ts
@@ -1,5 +1,5 @@
 import React from 'react';
-import { useAsync, useAsyncRetry, useInterval } from 'react-use';
+import { useAsyncRetry, useInterval } from 'react-use';
 
 import { useApi } from '@backstage/core-plugin-api';
 import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common/alpha';
@@ -17,15 +17,22 @@ import { rbacApiRef } from '../api/RBACBackendClient';
 import { RolesData } from '../types';
 import { getPermissions } from '../utils/rbac-utils';
 
-export const useRoles = (pollInterval?: number) => {
+export const useRoles = (
+  pollInterval?: number,
+): {
+  loading: boolean;
+  data: RolesData[];
+  createRoleAllowed: boolean;
+  retry: () => void;
+} => {
   const rbacApi = useApi(rbacApiRef);
   const {
     loading: rolesLoading,
     value: roles,
-    retry,
+    retry: roleRetry,
   } = useAsyncRetry(async () => await rbacApi.getRoles());
 
-  const { loading: policiesLoading, value: policies } = useAsync(
+  const { loading: policiesLoading, value: policies } = useAsyncRetry(
     async () => await rbacApi.getPolicies(),
     [],
   );
@@ -83,7 +90,17 @@ export const useRoles = (pollInterval?: number) => {
     [roles, policies, deletePermissionResult, editPermissionResult],
   );
   const loading = rolesLoading && policiesLoading;
-  useInterval(() => retry(), loading ? null : pollInterval || 5000);
+  useInterval(
+    () => {
+      roleRetry();
+    },
+    loading ? null : pollInterval || 10000,
+  );
 
-  return { loading, data, retry, createRoleAllowed };
+  return {
+    loading,
+    data,
+    createRoleAllowed,
+    retry: roleRetry,
+  };
 };