Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/spicy-zebras-deliver.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': minor
---

Adds a warning modal in order to prevent the `access-permissions` permission from being removed from the last granted role in the permissions table. This hardening measure ensures that administrators cannot accidentally lock themselves out of the system's permission management screen.
2 changes: 2 additions & 0 deletions apps/meteor/app/authorization/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ export const CONSTANTS = {
SETTINGS_LEVEL: 'settings',
} as const;

export const confirmationRequiredPermissions = ['access-permissions'];

export { AuthorizationUtils } from './AuthorizationUtils';
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const PermissionsPage = ({ isEnterprise }: { isEnterprise: boolean }): ReactElem
<Tabs.Item
data-qa='PermissionTable-Permissions'
selected={type === 'permissions'}
onClick={handlePermissionsTab}
onClick={canViewPermission ? handlePermissionsTab : undefined}
disabled={!canViewPermission}
>
{t('Permissions')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,26 @@ type PermissionRowProps = {

const PermissionRow = ({ permission, roleList, onGrant, onRemove }: PermissionRowProps): ReactElement => {
const { t } = useTranslation();
const { _id, roles } = permission;
const changeRole = useChangeRole({ onGrant, onRemove, permissionId: _id });
const { _id: permissionId, roles } = permission;
const changeRole = useChangeRole({ onGrant, onRemove, permissionId });
const permissionName = getName(t, permission);

return (
<GenericTableRow key={_id} role='link' action tabIndex={0}>
<GenericTableCell maxWidth='x300' withTruncatedText title={t(`${_id}_description` as TranslationKey)}>
{getName(t, permission)}
<GenericTableRow key={permissionId} role='link' action tabIndex={0}>
<GenericTableCell maxWidth='x300' withTruncatedText title={t(`${permissionId}_description` as TranslationKey)}>
{permissionName}
</GenericTableCell>
{roleList.map(({ _id, name, description }) => (
<RoleCell key={_id} _id={_id} name={name} description={description} grantedRoles={roles} onChange={changeRole} permissionId={_id} />
{roleList.map(({ _id: roleId, name, description }) => (
<RoleCell
key={roleId}
_id={roleId}
name={name}
description={description}
grantedRoles={roles}
onChange={changeRole}
permissionId={permissionId}
permissionName={permissionName}
/>
))}
</GenericTableRow>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import type { IPermission, IRole } from '@rocket.chat/core-typings';
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { composeStories } from '@storybook/react';
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { axe } from 'jest-axe';

import PermissionsTable from './PermissionsTable';
import * as stories from './PermissionsTable.stories';

const testCases = Object.values(composeStories(stories)).map((Story) => [Story.storyName || 'Story', Story]);
Expand All @@ -20,3 +23,57 @@ test.each(testCases)('%s should have no a11y violations', async (_storyname, Sto
const results = await axe(container, { rules: { 'label': { enabled: false }, 'button-name': { enabled: false } } });
expect(results).toHaveNoViolations();
});

const defaultPermissions: IPermission[] = [
{
_id: 'access-permissions',
_updatedAt: new Date('2023-01-01'),
roles: ['admin'],
},
];

const roles: IRole[] = [
{
description: 'Owner of the workspace',
name: 'owner',
protected: true,
scope: 'Users',
_id: 'owner',
},
{
description: 'Administrator',
name: 'admin',
protected: true,
scope: 'Users',
_id: 'admin',
},
];

test('should display modal if the permission is access-permissions and has only one granted role', async () => {
render(
<PermissionsTable permissions={defaultPermissions} total={defaultPermissions.length} setFilter={() => undefined} roleList={roles} />,
{
wrapper: mockAppRoot().build(),
},
);

await userEvent.click(screen.getByRole('checkbox', { name: 'access-permissions - Administrator' }));
expect(screen.getByRole('dialog')).toBeInTheDocument();
});

test('should NOT display modal if the permission is access-permissions and has more than one granted role', async () => {
const morePermissions: IPermission[] = [
{
_id: 'access-permissions',
_updatedAt: new Date('2023-01-01'),
roles: ['admin', 'owner'],
},
];

render(<PermissionsTable permissions={morePermissions} total={morePermissions.length} setFilter={() => undefined} roleList={roles} />, {
wrapper: mockAppRoot().build(),
});

await userEvent.click(screen.getByRole('checkbox', { name: 'access-permissions - Administrator' }));
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});
Original file line number Diff line number Diff line change
Expand Up @@ -80,41 +80,21 @@ const permissions: IPermission[] = [
_id: '0',
_updatedAt: new Date('2023-01-01'),
roles: ['admin'],
group: 'admin',
level: 'settings',
section: 'General',
settingId: 'general_settings',
sorter: 1,
},
{
_id: '1',
_updatedAt: new Date('2023-01-01'),
roles: ['user'],
group: 'admin',
level: 'settings',
section: 'General',
settingId: 'general_settings',
sorter: 2,
},
{
_id: '2',
_updatedAt: new Date('2023-01-01'),
roles: ['user'],
group: 'admin',
level: 'settings',
section: 'General',
settingId: 'general_settings',
sorter: 3,
},
{
_id: '3',
_updatedAt: new Date('2023-01-01'),
roles: ['user'],
group: 'admin',
level: 'settings',
section: 'General',
settingId: 'general_settings',
sorter: 4,
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type PermissionsTableProps = {
permissions: IPermission[];
setFilter: (filter: string) => void;
total: number;
paginationProps: ReturnType<typeof usePagination>;
paginationProps?: ReturnType<typeof usePagination>;
};

const PermissionsTable = ({ roleList, permissions, setFilter, total, paginationProps }: PermissionsTableProps) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { IRole } from '@rocket.chat/core-typings';
import { Margins, Box, CheckBox, Throbber } from '@rocket.chat/fuselage';
import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
import { GenericModal } from '@rocket.chat/ui-client';
import { useSetModal } from '@rocket.chat/ui-contexts';
import type { ReactElement } from 'react';
import { useState, memo } from 'react';
import { useTranslation } from 'react-i18next';

import { AuthorizationUtils } from '../../../../../app/authorization/lib';
import { AuthorizationUtils, confirmationRequiredPermissions } from '../../../../../app/authorization/lib';
import { GenericTableCell } from '../../../../components/GenericTable';

type RoleCellProps = {
Expand All @@ -13,28 +16,50 @@ type RoleCellProps = {
description: IRole['description'];
onChange: (roleId: IRole['_id'], granted: boolean) => Promise<boolean>;
permissionId: string;
permissionName: string;
grantedRoles: IRole['_id'][];
};

const RoleCell = ({ _id, name, description, onChange, permissionId, grantedRoles = [] }: RoleCellProps): ReactElement => {
const RoleCell = ({ _id, name, description, onChange, permissionId, permissionName, grantedRoles = [] }: RoleCellProps): ReactElement => {
const { t } = useTranslation();
const setModal = useSetModal();
const [granted, setGranted] = useState(() => !!grantedRoles.includes(_id));
const [loading, setLoading] = useState(false);

const isRestrictedForRole = AuthorizationUtils.isPermissionRestrictedForRole(permissionId, _id);
const shouldDisplayConfirmation = confirmationRequiredPermissions.includes(permissionId) && grantedRoles.length === 1 && granted;

const handleChange = useEffectEvent(async () => {
const handleChange = useEffectEvent(() => {
if (shouldDisplayConfirmation) {
const handleSubmit = () => {
handleConfirmChange();
setModal(null);
};

return setModal(
<GenericModal onConfirm={handleSubmit} onCancel={() => setModal(null)} confirmText={t('Remove')} variant='danger'>
{t('Last_role_in_permission_warning')}
</GenericModal>,
);
}

return handleConfirmChange();
});

const handleConfirmChange = useEffectEvent(async () => {
setLoading(true);
const result = await onChange(_id, granted);
setGranted(result);
setLoading(false);
});

const isDisabled = !!loading || !!isRestrictedForRole;
const checkboxLabel = `${permissionName} - ${description || name}`;

return (
<GenericTableCell withTruncatedText>
<Margins inline={2}>
<CheckBox checked={granted} onChange={handleChange} disabled={isDisabled} />
<CheckBox aria-label={checkboxLabel} checked={granted} onChange={handleChange} disabled={isDisabled} />
{!loading && (
<Box display='inline' color='hint' invisible>
{description || name}
Expand Down
Loading
Loading