From fb0ee8c269892f6c2ccaea69754a9dda653d4fcb Mon Sep 17 00:00:00 2001 From: Debsmita Santra Date: Wed, 20 Dec 2023 20:05:37 +0530 Subject: [PATCH] feat(rbac): cleanup policies when a role is deleted (#1018) --- packages/backend/package.json | 2 +- plugins/rbac/dev/index.tsx | 19 +++++++----- plugins/rbac/src/api/RBACBackendClient.ts | 21 ++++++++++++++ plugins/rbac/src/components/DeleteRole.tsx | 2 +- .../rbac/src/components/DeleteRoleDialog.tsx | 26 +++++++++++++++-- plugins/rbac/src/components/EditRole.tsx | 1 + plugins/rbac/src/components/RbacPage.test.tsx | 2 +- .../rbac/src/components/RolesList.test.tsx | 14 ++++----- plugins/rbac/src/hooks/useRoles.ts | 29 +++++++++++++++---- 9 files changed, 90 insertions(+), 26 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index eb006ac93f..65628d5914 100644 --- 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 index ff80d5b7a9..5633cd1831 100644 --- 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 { - return mockPolicies; - } - async getAssociatedPolicies( entityReference: string, ): Promise { return mockPolicies.filter(pol => pol.entityReference === entityReference); } + async getPolicies(): Promise { + 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 { - return 204; + ): Promise { + return { + ok: true, + status: 204, + statusText: 'Deleted Successfully', + } as Response; } async createRole(_role: Role): Promise { diff --git a/plugins/rbac/src/api/RBACBackendClient.ts b/plugins/rbac/src/api/RBACBackendClient.ts index ed93fbaf8b..91c6ccd396 100644 --- 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; deleteRole: (role: string) => Promise; + deletePolicies: (role: string, policy: Policy[]) => Promise; getRole: (role: string) => Promise; getMembers: () => Promise; listPermissions: () => Promise; @@ -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 index 852c368185..0b415fc2a9 100644 --- a/plugins/rbac/src/components/DeleteRole.tsx +++ b/plugins/rbac/src/components/DeleteRole.tsx @@ -29,7 +29,7 @@ const DeleteRole = ({ openDialog(roleName)} aria-label="Delete" disabled={disable} diff --git a/plugins/rbac/src/components/DeleteRoleDialog.tsx b/plugins/rbac/src/components/DeleteRoleDialog.tsx index f046a54548..a9f3b89e7b 100644 --- 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 = ({ {error && ( - {`Unable to delete. ${error}`} + {error} )} { mockUseRoles.mockReturnValue({ loading: true, data: [], - retry: () => {}, + retry: jest.fn(), createRoleAllowed: false, }); await renderInTestApp(); diff --git a/plugins/rbac/src/components/RolesList.test.tsx b/plugins/rbac/src/components/RolesList.test.tsx index 399a81f7bf..74690f42e2 100644 --- 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(); @@ -79,7 +79,7 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: [], - retry: () => {}, + retry: jest.fn(), createRoleAllowed: false, }); const { getByTestId } = await renderInTestApp(); @@ -94,7 +94,7 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: () => {}, + retry: jest.fn(), createRoleAllowed: false, }); const { getAllByTestId, getByText } = await renderInTestApp(); @@ -125,7 +125,7 @@ describe('RolesList', () => { }, }, ], - retry: () => {}, + retry: jest.fn(), createRoleAllowed: false, }); const { getAllByTestId } = await renderInTestApp(); @@ -156,7 +156,7 @@ describe('RolesList', () => { }, }, ], - retry: () => {}, + retry: jest.fn(), createRoleAllowed: true, }); const { getAllByTestId } = await renderInTestApp(); @@ -170,7 +170,7 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: () => {}, + retry: jest.fn(), createRoleAllowed: false, }); const { getByTestId } = await renderInTestApp(); @@ -186,7 +186,7 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: () => {}, + retry: jest.fn(), createRoleAllowed: true, }); const { getByTestId } = await renderInTestApp(); diff --git a/plugins/rbac/src/hooks/useRoles.ts b/plugins/rbac/src/hooks/useRoles.ts index 38051740eb..54ce7cf6e3 100644 --- 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, + }; };