diff --git a/plugins/rbac/README.md b/plugins/rbac/README.md index b61279bf17..e013c3346f 100644 --- a/plugins/rbac/README.md +++ b/plugins/rbac/README.md @@ -30,6 +30,8 @@ p, role:default/team_a, policy-entity, create, allow g, user:default/, role:default/team_a ``` +> Note: Even after applying above permissions if the create button is still disabled then please contact your administrator as you might be conditionally restricted to access the create button. + - To fetch the permissions from other plugins like `Kubernetes` and `Jenkins` in the Role Form as mentioned [here](https://github.com/janus-idp/backstage-plugins/blob/main/plugins/rbac-backend/docs/permissions.md), add the following configuration in your `app-config.yaml`: ```yaml title="app-config.yaml" diff --git a/plugins/rbac/src/components/CreateRole/AddMembersForm.tsx b/plugins/rbac/src/components/CreateRole/AddMembersForm.tsx index afb96f1ccc..b7e111af00 100644 --- a/plugins/rbac/src/components/CreateRole/AddMembersForm.tsx +++ b/plugins/rbac/src/components/CreateRole/AddMembersForm.tsx @@ -107,6 +107,7 @@ export const AddMembersForm = ({ renderOption={(option: SelectedMember, state) => ( )} + noOptionsText="No users and groups found." clearOnEscape renderInput={params => ( ({ RoleForm: () =>
RoleForm
, })); -jest.mock('@backstage/plugin-permission-react', () => ({ - RequirePermission: jest - .fn() - .mockImplementation(({ children }) =>
{children}
), -})); - -const mockedPrequirePermission = RequirePermission as jest.MockedFunction< - typeof RequirePermission ->; - jest.mock('@backstage/core-components', () => ({ Page: jest.fn().mockImplementation(({ children }) => (
@@ -67,13 +56,6 @@ describe('CreateRolePage', () => { }); render(); - expect(mockedPrequirePermission).toHaveBeenCalledWith( - expect.objectContaining({ - permission: expect.objectContaining({ name: 'catalog.entity.read' }), - resourceRef: expect.stringContaining('catalog-entity'), - }), - expect.anything(), - ); expect(mockedPage).toHaveBeenCalled(); expect(mockedHeader).toHaveBeenCalled(); expect(mockedContent).toHaveBeenCalled(); diff --git a/plugins/rbac/src/components/CreateRole/CreateRolePage.tsx b/plugins/rbac/src/components/CreateRole/CreateRolePage.tsx index 9c2564870b..29eaaa95fa 100644 --- a/plugins/rbac/src/components/CreateRole/CreateRolePage.tsx +++ b/plugins/rbac/src/components/CreateRole/CreateRolePage.tsx @@ -1,10 +1,14 @@ import React from 'react'; import { useAsync } from 'react-use'; -import { Content, Header, Page } from '@backstage/core-components'; +import { + Content, + ErrorPage, + Header, + Page, + Progress, +} from '@backstage/core-components'; import { useApi } from '@backstage/core-plugin-api'; -import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common/alpha'; -import { RequirePermission } from '@backstage/plugin-permission-react'; import { rbacApiRef } from '../../api/RBACBackendClient'; import { MemberEntity } from '../../types'; @@ -15,13 +19,19 @@ import { RoleFormValues } from './types'; export const CreateRolePage = () => { const rbacApi = useApi(rbacApiRef); const { - loading, + loading: membersLoading, value: members, error: membersError, } = useAsync(async () => { return await rbacApi.getMembers(); }); + const canReadUsersAndGroups = + !membersLoading && + !membersError && + Array.isArray(members) && + members.length > 0; + const initialValues: RoleFormValues = { name: '', namespace: 'default', @@ -31,35 +41,34 @@ export const CreateRolePage = () => { permissionPoliciesRows: [initialPermissionPolicyRowValue], }; - return ( - - -
- - - - - + if (membersLoading) { + return ; + } + + return canReadUsersAndGroups ? ( + +
+ + + + + ) : ( + ); }; diff --git a/plugins/rbac/src/components/CreateRole/EditRolePage.test.tsx b/plugins/rbac/src/components/CreateRole/EditRolePage.test.tsx index 878764ad61..be269c5da0 100644 --- a/plugins/rbac/src/components/CreateRole/EditRolePage.test.tsx +++ b/plugins/rbac/src/components/CreateRole/EditRolePage.test.tsx @@ -47,17 +47,6 @@ jest.mock('./RoleForm', () => ({ RoleForm: () =>
RoleForm
, })); -jest.mock('@backstage/plugin-permission-react', () => ({ - RequirePermission: jest - .fn() - .mockImplementation(({ permission, resourceRef, children }) => ( -
- {`${permission} ${resourceRef}`} - {children} -
- )), -})); - jest.mock('@backstage/core-components', () => ({ useQueryParamState: () => ['roleName', jest.fn()], Progress: () =>
MockedProgressComponent
, @@ -117,6 +106,7 @@ describe('EditRolePage', () => { loading: false, roleError: { name: '', message: '' }, membersError: new Error(''), + canReadUsersAndGroups: true, }); render(); expect(screen.getByText('Edit Role Page')).toBeInTheDocument(); @@ -131,6 +121,7 @@ describe('EditRolePage', () => { role: undefined, membersError: { name: '', message: '' }, roleError: { name: '', message: '' }, + canReadUsersAndGroups: true, }); render(); expect(screen.queryByText('MockedProgressComponent')).toBeInTheDocument(); @@ -144,6 +135,7 @@ describe('EditRolePage', () => { role: undefined, membersError: { name: 'Error', message: 'Error Message' }, loading: false, + canReadUsersAndGroups: true, }); render(); expect(screen.queryByText('MockedErrorPageComponent')).toBeInTheDocument(); diff --git a/plugins/rbac/src/components/CreateRole/EditRolePage.tsx b/plugins/rbac/src/components/CreateRole/EditRolePage.tsx index 6db2da4d0c..e845f4b4ca 100644 --- a/plugins/rbac/src/components/CreateRole/EditRolePage.tsx +++ b/plugins/rbac/src/components/CreateRole/EditRolePage.tsx @@ -9,8 +9,6 @@ import { Progress, useQueryParamState, } from '@backstage/core-components'; -import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common/alpha'; -import { RequirePermission } from '@backstage/plugin-permission-react'; import { usePermissionPolicies } from '../../hooks/usePermissionPolicies'; import { useSelectedMembers } from '../../hooks/useSelectedMembers'; @@ -20,10 +18,17 @@ import { RoleFormValues } from './types'; export const EditRolePage = () => { const { roleName, roleNamespace, roleKind } = useParams(); const [queryParamState] = useQueryParamState('activeStep'); - const { selectedMembers, members, role, loading, roleError, membersError } = - useSelectedMembers( - roleName ? `${roleKind}:${roleNamespace}/${roleName}` : '', - ); + const { + selectedMembers, + members, + role, + loading, + roleError, + membersError, + canReadUsersAndGroups, + } = useSelectedMembers( + roleName ? `${roleKind}:${roleNamespace}/${roleName}` : '', + ); const { data: permissionPolicies, conditionsData } = usePermissionPolicies( `${roleKind}:${roleNamespace}/${roleName}`, @@ -37,44 +42,35 @@ export const EditRolePage = () => { selectedMembers, permissionPoliciesRows: [...conditionsData, ...permissionPolicies], }; - const renderPage = () => { - if (loading) { - return ; - } else if (roleError.name) { - return ( - - ); - } + + if (loading) { + return ; + } else if (roleError.name) { return ( - <> -
- - - - + ); - }; + } - return ( - - {renderPage()} - + return canReadUsersAndGroups ? ( + +
+ + + + + ) : ( + ); }; diff --git a/plugins/rbac/src/components/RoleOverview/MembersCard.test.tsx b/plugins/rbac/src/components/RoleOverview/MembersCard.test.tsx index d15d1c00d5..b214e4d6f9 100644 --- a/plugins/rbac/src/components/RoleOverview/MembersCard.test.tsx +++ b/plugins/rbac/src/components/RoleOverview/MembersCard.test.tsx @@ -3,7 +3,6 @@ import React from 'react'; import { usePermission } from '@backstage/plugin-permission-react'; import { renderInTestApp } from '@backstage/test-utils'; -import { useMembers } from '../../hooks/useMembers'; import { MembersData } from '../../types'; import { MembersCard } from './MembersCard'; @@ -58,7 +57,6 @@ const useMembersMockData: MembersData[] = [ }, ]; -const mockMembers = useMembers as jest.MockedFunction; const mockUsePermission = usePermission as jest.MockedFunction< typeof usePermission >; @@ -66,14 +64,18 @@ const mockUsePermission = usePermission as jest.MockedFunction< describe('MembersCard', () => { it('should show list of Users and groups associated with the role when the data is loaded', async () => { mockUsePermission.mockReturnValue({ loading: false, allowed: true }); - mockMembers.mockReturnValue({ + const membersInfo = { loading: false, data: useMembersMockData, error: undefined, retry: { roleRetry: jest.fn(), membersRetry: jest.fn() }, - }); + canReadUsersAndGroups: true, + }; const { queryByText } = await renderInTestApp( - , + , ); expect(queryByText('Users and groups (2 users, 2 groups)')).not.toBeNull(); expect(queryByText('Calum Leavy')).not.toBeNull(); @@ -83,14 +85,18 @@ describe('MembersCard', () => { it('should show empty table when there are no users and groups', async () => { mockUsePermission.mockReturnValue({ loading: false, allowed: true }); - mockMembers.mockReturnValue({ + const membersInfo = { loading: false, data: [], error: undefined, retry: { roleRetry: jest.fn(), membersRetry: jest.fn() }, - }); + canReadUsersAndGroups: true, + }; const { queryByText } = await renderInTestApp( - , + , ); expect(queryByText('Users and groups')).not.toBeNull(); expect(queryByText('No records found')).not.toBeNull(); @@ -98,14 +104,18 @@ describe('MembersCard', () => { it('should show an error if api call fails', async () => { mockUsePermission.mockReturnValue({ loading: false, allowed: true }); - mockMembers.mockReturnValue({ + const membersInfo = { loading: false, data: [], error: { message: 'xyz' }, retry: { roleRetry: jest.fn(), membersRetry: jest.fn() }, - }); + canReadUsersAndGroups: false, + }; const { queryByText } = await renderInTestApp( - , + , ); expect( queryByText( @@ -118,28 +128,36 @@ describe('MembersCard', () => { it('should show edit icon when the user is authorized to update roles', async () => { mockUsePermission.mockReturnValue({ loading: false, allowed: true }); - mockMembers.mockReturnValue({ + const membersInfo = { loading: false, data: useMembersMockData, error: undefined, retry: { roleRetry: jest.fn(), membersRetry: jest.fn() }, - }); + canReadUsersAndGroups: true, + }; const { getByTestId } = await renderInTestApp( - , + , ); expect(getByTestId('update-members')).not.toBeNull(); }); it('should disable edit icon when the user is not authorized to update roles', async () => { mockUsePermission.mockReturnValue({ loading: false, allowed: false }); - mockMembers.mockReturnValue({ + const membersInfo = { loading: false, data: useMembersMockData, error: undefined, retry: { roleRetry: jest.fn(), membersRetry: jest.fn() }, - }); + canReadUsersAndGroups: false, + }; const { queryByTestId } = await renderInTestApp( - , + , ); expect(queryByTestId('disable-update-members')).not.toBeNull(); }); diff --git a/plugins/rbac/src/components/RoleOverview/MembersCard.tsx b/plugins/rbac/src/components/RoleOverview/MembersCard.tsx index 54e95d30d3..ec6080106a 100644 --- a/plugins/rbac/src/components/RoleOverview/MembersCard.tsx +++ b/plugins/rbac/src/components/RoleOverview/MembersCard.tsx @@ -1,7 +1,6 @@ import React from 'react'; import { Table, WarningPanel } from '@backstage/core-components'; -import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common/alpha'; import { usePermission } from '@backstage/plugin-permission-react'; import { Card, CardContent, makeStyles } from '@material-ui/core'; @@ -9,7 +8,7 @@ import CachedIcon from '@material-ui/icons/Cached'; import { policyEntityUpdatePermission } from '@janus-idp/backstage-plugin-rbac-common'; -import { useMembers } from '../../hooks/useMembers'; +import { MembersInfo } from '../../hooks/useMembers'; import { MembersData } from '../../types'; import { getKindNamespaceName, getMembers } from '../../utils/rbac-utils'; import EditRole from '../EditRole'; @@ -17,6 +16,7 @@ import { columns } from './MembersListColumns'; type MembersCardProps = { roleName: string; + membersInfo: MembersInfo; }; const useStyles = makeStyles(theme => ({ @@ -41,17 +41,13 @@ const getEditIcon = (isAllowed: boolean, roleName: string) => { ); }; -export const MembersCard = ({ roleName }: MembersCardProps) => { - const { data, loading, retry, error } = useMembers(roleName); +export const MembersCard = ({ roleName, membersInfo }: MembersCardProps) => { + const { data, loading, retry, error, canReadUsersAndGroups } = membersInfo; const [members, setMembers] = React.useState(); const policyEntityPermissionResult = usePermission({ permission: policyEntityUpdatePermission, resourceRef: policyEntityUpdatePermission.resourceType, }); - const catalogEntityPermissionResult = usePermission({ - permission: catalogEntityReadPermission, - resourceRef: catalogEntityReadPermission.resourceType, - }); const classes = useStyles(); const actions = [ @@ -67,13 +63,11 @@ export const MembersCard = ({ roleName }: MembersCardProps) => { { icon: () => getEditIcon( - policyEntityPermissionResult.allowed && - catalogEntityPermissionResult.allowed, + policyEntityPermissionResult.allowed && canReadUsersAndGroups, roleName, ), tooltip: - catalogEntityPermissionResult.allowed && - policyEntityPermissionResult.allowed + policyEntityPermissionResult.allowed && canReadUsersAndGroups ? 'Edit' : 'Unauthorized to edit', isFreeAction: true, diff --git a/plugins/rbac/src/components/RoleOverview/PermissionsCard.test.tsx b/plugins/rbac/src/components/RoleOverview/PermissionsCard.test.tsx index 236b25e810..bed3fc0210 100644 --- a/plugins/rbac/src/components/RoleOverview/PermissionsCard.test.tsx +++ b/plugins/rbac/src/components/RoleOverview/PermissionsCard.test.tsx @@ -55,7 +55,10 @@ describe('PermissionsCard', () => { error: new Error(''), }); const { queryByText } = await renderInTestApp( - , + , ); expect(queryByText('Permission Policies (3)')).not.toBeNull(); expect(queryByText('Read, Create, Delete')).not.toBeNull(); @@ -71,7 +74,10 @@ describe('PermissionsCard', () => { error: new Error(''), }); const { queryByText } = await renderInTestApp( - , + , ); expect(queryByText('Permission Policies')).not.toBeNull(); expect(queryByText('No records found')).not.toBeNull(); @@ -86,7 +92,10 @@ describe('PermissionsCard', () => { error: { message: '404', name: 'Not Found' }, }); const { queryByText } = await renderInTestApp( - , + , ); expect( queryByText( @@ -106,7 +115,10 @@ describe('PermissionsCard', () => { retry: { policiesRetry: jest.fn(), permissionPoliciesRetry: jest.fn() }, }); const { getByTestId } = await renderInTestApp( - , + , ); expect(getByTestId('update-policies')).not.toBeNull(); }); @@ -121,7 +133,10 @@ describe('PermissionsCard', () => { retry: { policiesRetry: jest.fn(), permissionPoliciesRetry: jest.fn() }, }); const { queryByTestId } = await renderInTestApp( - , + , ); expect(queryByTestId('disable-update-policies')).not.toBeNull(); }); diff --git a/plugins/rbac/src/components/RoleOverview/PermissionsCard.tsx b/plugins/rbac/src/components/RoleOverview/PermissionsCard.tsx index 5aa51e7b11..1c8d9dad90 100644 --- a/plugins/rbac/src/components/RoleOverview/PermissionsCard.tsx +++ b/plugins/rbac/src/components/RoleOverview/PermissionsCard.tsx @@ -24,6 +24,7 @@ const useStyles = makeStyles(theme => ({ type PermissionsCardProps = { entityReference: string; + canReadUsersAndGroups: boolean; }; const getRefreshIcon = () => ; @@ -40,7 +41,10 @@ const getEditIcon = (isAllowed: boolean, roleName: string) => { ); }; -export const PermissionsCard = ({ entityReference }: PermissionsCardProps) => { +export const PermissionsCard = ({ + entityReference, + canReadUsersAndGroups, +}: PermissionsCardProps) => { const { data, loading, retry, error } = usePermissionPolicies(entityReference); const [permissions, setPermissions] = React.useState(); @@ -71,8 +75,15 @@ export const PermissionsCard = ({ entityReference }: PermissionsCardProps) => { }, }, { - icon: () => getEditIcon(permissionResult.allowed, entityReference), - tooltip: !permissionResult.allowed ? 'Unauthorized to edit' : 'Edit', + icon: () => + getEditIcon( + permissionResult.allowed && canReadUsersAndGroups, + entityReference, + ), + tooltip: + permissionResult.allowed && canReadUsersAndGroups + ? 'Edit' + : 'Unauthorized to edit', isFreeAction: true, onClick: () => {}, }, diff --git a/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx b/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx index e6cf501c88..cb8727fa2c 100644 --- a/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx +++ b/plugins/rbac/src/components/RoleOverview/RoleOverviewPage.tsx @@ -6,6 +6,7 @@ import { Header, Page, TabbedLayout } from '@backstage/core-components'; import { Grid } from '@material-ui/core'; import { useLocationToast } from '../../hooks/useLocationToast'; +import { useMembers } from '../../hooks/useMembers'; import { SnackbarAlert } from '../SnackbarAlert'; import { useToast } from '../ToastContext'; import { AboutCard } from './AboutCard'; @@ -15,6 +16,7 @@ import { PermissionsCard } from './PermissionsCard'; export const RoleOverviewPage = () => { const { roleName, roleNamespace, roleKind } = useParams(); const { toastMessage, setToastMessage } = useToast(); + const membersInfo = useMembers(`${roleKind}:${roleNamespace}/${roleName}`); useLocationToast(setToastMessage); @@ -42,11 +44,13 @@ export const RoleOverviewPage = () => { diff --git a/plugins/rbac/src/hooks/useMembers.ts b/plugins/rbac/src/hooks/useMembers.ts index 7c5eb57533..9def554e51 100644 --- a/plugins/rbac/src/hooks/useMembers.ts +++ b/plugins/rbac/src/hooks/useMembers.ts @@ -8,6 +8,14 @@ import { rbacApiRef } from '../api/RBACBackendClient'; import { MemberEntity, MembersData } from '../types'; import { getKindNamespaceName, getMembersFromGroup } from '../utils/rbac-utils'; +export type MembersInfo = { + loading: boolean; + data: MembersData[]; + retry: { roleRetry: () => void; membersRetry: () => void }; + error?: { message: string }; + canReadUsersAndGroups: boolean; +}; + const getErrorText = ( role: any, members: any, @@ -58,7 +66,10 @@ const getMemberData = ( }; }; -export const useMembers = (roleName: string, pollInterval?: number) => { +export const useMembers = ( + roleName: string, + pollInterval?: number, +): MembersInfo => { const rbacApi = useApi(rbacApiRef); let data: MembersData[] = []; const { @@ -77,6 +88,9 @@ export const useMembers = (roleName: string, pollInterval?: number) => { return await rbacApi.getMembers(); }); + const canReadUsersAndGroups = + !membersError && Array.isArray(members) && members.length > 0; + const loading = !roleError && !membersError && !role && !members; data = React.useMemo( @@ -109,5 +123,6 @@ export const useMembers = (roleName: string, pollInterval?: number) => { data, retry: { roleRetry, membersRetry }, error: getErrorText(role, members) || roleError || membersError, + canReadUsersAndGroups, }; }; diff --git a/plugins/rbac/src/hooks/useRoles.ts b/plugins/rbac/src/hooks/useRoles.ts index eb413015b6..c860990445 100644 --- a/plugins/rbac/src/hooks/useRoles.ts +++ b/plugins/rbac/src/hooks/useRoles.ts @@ -1,8 +1,7 @@ import React from 'react'; -import { useAsyncRetry, useInterval } from 'react-use'; +import { useAsync, useAsyncRetry, useInterval } from 'react-use'; import { useApi } from '@backstage/core-plugin-api'; -import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common/alpha'; import { usePermission } from '@backstage/plugin-permission-react'; import { @@ -43,6 +42,20 @@ export const useRoles = ( error: policiesError, } = useAsyncRetry(async () => await rbacApi.getPolicies(), []); + const { + loading: membersLoading, + value: members, + error: membersError, + } = useAsync(async () => { + return await rbacApi.getMembers(); + }); + + const canReadUsersAndGroups = + !membersLoading && + !membersError && + Array.isArray(members) && + members.length > 0; + const deletePermissionResult = usePermission({ permission: policyEntityDeletePermission, resourceRef: policyEntityDeletePermission.resourceType, @@ -53,18 +66,11 @@ export const useRoles = ( resourceRef: policyEntityCreatePermission.resourceType, }); - const catalogEntityReadPermissionResult = usePermission({ - permission: catalogEntityReadPermission, - resourceRef: catalogEntityReadPermission.resourceType, - }); - const createRoleLoading = - policyEntityCreatePermissionResult.loading || - catalogEntityReadPermissionResult.loading; + policyEntityCreatePermissionResult.loading || membersLoading; const createRoleAllowed = - policyEntityCreatePermissionResult.allowed && - catalogEntityReadPermissionResult.allowed; + policyEntityCreatePermissionResult.allowed && canReadUsersAndGroups; const editPermissionResult = usePermission({ permission: policyEntityUpdatePermission, @@ -93,11 +99,8 @@ export const useRoles = ( delete: deletePermissionResult, edit: { allowed: - editPermissionResult.allowed && - catalogEntityReadPermissionResult.allowed, - loading: - editPermissionResult.loading && - catalogEntityReadPermissionResult.loading, + editPermissionResult.allowed && canReadUsersAndGroups, + loading: editPermissionResult.loading, }, }, }, @@ -108,8 +111,9 @@ export const useRoles = ( roles, policies, deletePermissionResult, - editPermissionResult, - catalogEntityReadPermissionResult, + editPermissionResult.allowed, + editPermissionResult.loading, + canReadUsersAndGroups, ], ); const loading = !rolesError && !policiesError && !roles && !policies; diff --git a/plugins/rbac/src/hooks/useSelectedMembers.ts b/plugins/rbac/src/hooks/useSelectedMembers.ts index 2f7c8cf5ba..c704c11d50 100644 --- a/plugins/rbac/src/hooks/useSelectedMembers.ts +++ b/plugins/rbac/src/hooks/useSelectedMembers.ts @@ -20,6 +20,7 @@ export const useSelectedMembers = ( membersError: Error; roleError: Error; loading: boolean; + canReadUsersAndGroups: boolean; } => { const rbacApi = useApi(rbacApiRef); const { role, loading: roleLoading, roleError } = useRole(roleName); @@ -32,6 +33,12 @@ export const useSelectedMembers = ( return await rbacApi.getMembers(); }); + const canReadUsersAndGroups = + !membersLoading && + !membersError && + Array.isArray(members) && + members.length > 0; + const data: SelectedMember[] = role ? (role as Role).memberReferences.reduce((acc: SelectedMember[], ref) => { const memberResource = @@ -54,5 +61,6 @@ export const useSelectedMembers = ( }, roleError: roleError, loading: roleLoading && membersLoading, + canReadUsersAndGroups, }; };