From 12a803b5b1228475add78657c0fa1a8ba9107c7f Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 25 Sep 2025 19:11:34 -0300 Subject: [PATCH 1/7] feat: create `disabledRolePermissions` --- .../lib/AuthorizationUtils.spec.ts | 33 ++++++++++++++ .../authorization/lib/AuthorizationUtils.ts | 45 +++++++++++++------ .../root/hooks/loggedIn/useRestrictedRoles.ts | 2 + apps/meteor/jest.config.ts | 1 + 4 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts diff --git a/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts b/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts new file mode 100644 index 0000000000000..8126a1b5fa50f --- /dev/null +++ b/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts @@ -0,0 +1,33 @@ +import { AuthorizationUtils } from './AuthorizationUtils'; + +const fakeRole = 'admin'; +const fakePermissionId = 'access-permissions'; + +beforeAll(() => { + AuthorizationUtils.addRolePermissionDisabledList(fakeRole, [fakePermissionId]); + AuthorizationUtils.addRolePermissionWhiteList(fakeRole, [fakePermissionId]); +}); + +it('should return true if the currentPermission and role is the same added to the disabledRolePermissions list', () => { + const currentPermission = fakePermissionId; + const result = AuthorizationUtils.isPermissionDisabledForRole(currentPermission, fakeRole); + expect(result).toBe(true); +}); + +it('should return false if the currentPermission and role is NOT the same added to the disabledRolePermissions list', () => { + const currentPermission = 'any-permission'; + const result = AuthorizationUtils.isPermissionDisabledForRole(currentPermission, fakeRole); + expect(result).toBe(false); +}); + +it('should return true if the currentPermission and role is NOT the same added to the restrictedRolePermissions list', () => { + const currentPermission = 'any-permission'; + const result = AuthorizationUtils.isPermissionRestrictedForRole(currentPermission, fakeRole); + expect(result).toBe(true); +}); + +it('should return false if the currentPermission and role is the same added to the restrictedRolePermissions list', () => { + const currentPermission = fakePermissionId; + const result = AuthorizationUtils.isPermissionRestrictedForRole(currentPermission, fakeRole); + expect(result).toBe(false); +}); diff --git a/apps/meteor/app/authorization/lib/AuthorizationUtils.ts b/apps/meteor/app/authorization/lib/AuthorizationUtils.ts index d51de55cc067d..342ea0a102458 100644 --- a/apps/meteor/app/authorization/lib/AuthorizationUtils.ts +++ b/apps/meteor/app/authorization/lib/AuthorizationUtils.ts @@ -1,41 +1,60 @@ const restrictedRolePermissions = new Map>(); +const disabledRolePermissions = new Map>(); export const AuthorizationUtils = class { - static addRolePermissionWhiteList(roleId: string, list: string[]): void { - if (!roleId) { - throw new Error('invalid-param'); - } - - if (!list) { + private static addRolePermissionRule(roleId: string, list: string[], permissionMap: Map>): void { + if (!roleId || !list) { throw new Error('invalid-param'); } - if (!restrictedRolePermissions.has(roleId)) { - restrictedRolePermissions.set(roleId, new Set()); + if (!permissionMap.has(roleId)) { + permissionMap.set(roleId, new Set()); } - const rules = restrictedRolePermissions.get(roleId); + const rules = permissionMap.get(roleId); for (const permissionId of list) { rules?.add(permissionId); } } - static isPermissionRestrictedForRole(permissionId: string, roleId: string): boolean { + static addRolePermissionWhiteList(roleId: string, list: string[]): void { + this.addRolePermissionRule(roleId, list, restrictedRolePermissions); + } + + static addRolePermissionDisabledList(roleId: string, list: string[]): void { + this.addRolePermissionRule(roleId, list, disabledRolePermissions); + } + + private static checkPermissionForRole( + permissionId: string, + roleId: string, + permissionMap: Map>, + modifier: 'allow' | 'deny' = 'deny', + ): boolean { if (!roleId || !permissionId) { throw new Error('invalid-param'); } - if (!restrictedRolePermissions.has(roleId)) { + if (!permissionMap.has(roleId)) { return false; } - const rules = restrictedRolePermissions.get(roleId); + const rules = permissionMap.get(roleId); if (!rules?.size) { return false; } - return !rules.has(permissionId); + const hasPermission = rules.has(permissionId); + return modifier === 'deny' ? !hasPermission : hasPermission; + } + + static isPermissionRestrictedForRole(permissionId: string, roleId: string): boolean { + return this.checkPermissionForRole(permissionId, roleId, restrictedRolePermissions); + } + + static isPermissionDisabledForRole(permissionId: string, roleId: string): boolean { + return this.checkPermissionForRole(permissionId, roleId, disabledRolePermissions, 'allow'); } static isPermissionRestrictedForRoleList(permissionId: string, roleList: string[]): boolean { diff --git a/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts b/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts index 57bbb0baf1afa..97b2e2f80de93 100644 --- a/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts +++ b/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts @@ -17,6 +17,8 @@ export const useRestrictedRoles = (): void => { return; } + AuthorizationUtils.addRolePermissionDisabledList('admin', ['access-permissions']); + if (isEnterprise) { AuthorizationUtils.addRolePermissionWhiteList('guest', [ 'view-d-room', diff --git a/apps/meteor/jest.config.ts b/apps/meteor/jest.config.ts index 92cc6f4706569..2e3e2ddb2e9f1 100644 --- a/apps/meteor/jest.config.ts +++ b/apps/meteor/jest.config.ts @@ -13,6 +13,7 @@ export default { '/client/**/**.spec.[jt]s?(x)', '/ee/client/**/**.spec.[jt]s?(x)', '/app/ui-message/client/**/**.spec.[jt]s?(x)', + '/app/authorization/**/**.spec.[jt]s?(x)', '/tests/unit/client/views/**/*.spec.{ts,tsx}', '/tests/unit/client/providers/**/*.spec.{ts,tsx}', ], From 2eee7122e8d6fcc39740c2c3f4425dd8a4520b9d Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 25 Sep 2025 19:12:11 -0300 Subject: [PATCH 2/7] feat: check for disabled permissions for roles in `RoleCell` --- .../admin/permissions/PermissionsPage.tsx | 2 +- .../PermissionsTable/PermissionRow.tsx | 20 +++++++++++++------ .../permissions/PermissionsTable/RoleCell.tsx | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/apps/meteor/client/views/admin/permissions/PermissionsPage.tsx b/apps/meteor/client/views/admin/permissions/PermissionsPage.tsx index 43b192054968d..96c0e9950433a 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsPage.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsPage.tsx @@ -62,7 +62,7 @@ const PermissionsPage = ({ isEnterprise }: { isEnterprise: boolean }): ReactElem {t('Permissions')} diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx index 2fb8853610246..e16aa85a60cb1 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx @@ -34,16 +34,24 @@ 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 }); return ( - - + + {getName(t, permission)} - {roleList.map(({ _id, name, description }) => ( - + {roleList.map(({ _id: roleId, name, description }) => ( + ))} ); diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx index cc9fec9dab43e..18803f63c46f0 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx @@ -21,6 +21,7 @@ const RoleCell = ({ _id, name, description, onChange, permissionId, grantedRoles const [loading, setLoading] = useState(false); const isRestrictedForRole = AuthorizationUtils.isPermissionRestrictedForRole(permissionId, _id); + const isDisabledForRole = AuthorizationUtils.isPermissionDisabledForRole(permissionId, _id); const handleChange = useEffectEvent(async () => { setLoading(true); @@ -29,7 +30,7 @@ const RoleCell = ({ _id, name, description, onChange, permissionId, grantedRoles setLoading(false); }); - const isDisabled = !!loading || !!isRestrictedForRole; + const isDisabled = !!loading || !!isRestrictedForRole || !!isDisabledForRole; return ( From 024a65f383f26d62f1a584056666f1e3376e23c9 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 30 Sep 2025 16:21:16 -0300 Subject: [PATCH 3/7] chore: changeset --- .changeset/spicy-zebras-deliver.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spicy-zebras-deliver.md diff --git a/.changeset/spicy-zebras-deliver.md b/.changeset/spicy-zebras-deliver.md new file mode 100644 index 0000000000000..337808fa8e334 --- /dev/null +++ b/.changeset/spicy-zebras-deliver.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': minor +--- + +Prevents the `access-permissions` permission from being removed from the `admin` role in the permissions table. This hardening measure ensures that administrators cannot accidentally lock themselves out of the system's permission management screen. From e501174bf4a68eb00cebf3f7b9820c86593fb779 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 7 Oct 2025 11:40:06 -0300 Subject: [PATCH 4/7] chore: revert changes --- .../lib/AuthorizationUtils.spec.ts | 33 -------------- .../authorization/lib/AuthorizationUtils.ts | 45 ++++++------------- .../root/hooks/loggedIn/useRestrictedRoles.ts | 2 - apps/meteor/jest.config.ts | 1 - 4 files changed, 13 insertions(+), 68 deletions(-) delete mode 100644 apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts diff --git a/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts b/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts deleted file mode 100644 index 8126a1b5fa50f..0000000000000 --- a/apps/meteor/app/authorization/lib/AuthorizationUtils.spec.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { AuthorizationUtils } from './AuthorizationUtils'; - -const fakeRole = 'admin'; -const fakePermissionId = 'access-permissions'; - -beforeAll(() => { - AuthorizationUtils.addRolePermissionDisabledList(fakeRole, [fakePermissionId]); - AuthorizationUtils.addRolePermissionWhiteList(fakeRole, [fakePermissionId]); -}); - -it('should return true if the currentPermission and role is the same added to the disabledRolePermissions list', () => { - const currentPermission = fakePermissionId; - const result = AuthorizationUtils.isPermissionDisabledForRole(currentPermission, fakeRole); - expect(result).toBe(true); -}); - -it('should return false if the currentPermission and role is NOT the same added to the disabledRolePermissions list', () => { - const currentPermission = 'any-permission'; - const result = AuthorizationUtils.isPermissionDisabledForRole(currentPermission, fakeRole); - expect(result).toBe(false); -}); - -it('should return true if the currentPermission and role is NOT the same added to the restrictedRolePermissions list', () => { - const currentPermission = 'any-permission'; - const result = AuthorizationUtils.isPermissionRestrictedForRole(currentPermission, fakeRole); - expect(result).toBe(true); -}); - -it('should return false if the currentPermission and role is the same added to the restrictedRolePermissions list', () => { - const currentPermission = fakePermissionId; - const result = AuthorizationUtils.isPermissionRestrictedForRole(currentPermission, fakeRole); - expect(result).toBe(false); -}); diff --git a/apps/meteor/app/authorization/lib/AuthorizationUtils.ts b/apps/meteor/app/authorization/lib/AuthorizationUtils.ts index 342ea0a102458..d51de55cc067d 100644 --- a/apps/meteor/app/authorization/lib/AuthorizationUtils.ts +++ b/apps/meteor/app/authorization/lib/AuthorizationUtils.ts @@ -1,60 +1,41 @@ const restrictedRolePermissions = new Map>(); -const disabledRolePermissions = new Map>(); export const AuthorizationUtils = class { - private static addRolePermissionRule(roleId: string, list: string[], permissionMap: Map>): void { - if (!roleId || !list) { + static addRolePermissionWhiteList(roleId: string, list: string[]): void { + if (!roleId) { + throw new Error('invalid-param'); + } + + if (!list) { throw new Error('invalid-param'); } - if (!permissionMap.has(roleId)) { - permissionMap.set(roleId, new Set()); + if (!restrictedRolePermissions.has(roleId)) { + restrictedRolePermissions.set(roleId, new Set()); } - const rules = permissionMap.get(roleId); + const rules = restrictedRolePermissions.get(roleId); for (const permissionId of list) { rules?.add(permissionId); } } - static addRolePermissionWhiteList(roleId: string, list: string[]): void { - this.addRolePermissionRule(roleId, list, restrictedRolePermissions); - } - - static addRolePermissionDisabledList(roleId: string, list: string[]): void { - this.addRolePermissionRule(roleId, list, disabledRolePermissions); - } - - private static checkPermissionForRole( - permissionId: string, - roleId: string, - permissionMap: Map>, - modifier: 'allow' | 'deny' = 'deny', - ): boolean { + static isPermissionRestrictedForRole(permissionId: string, roleId: string): boolean { if (!roleId || !permissionId) { throw new Error('invalid-param'); } - if (!permissionMap.has(roleId)) { + if (!restrictedRolePermissions.has(roleId)) { return false; } - const rules = permissionMap.get(roleId); + const rules = restrictedRolePermissions.get(roleId); if (!rules?.size) { return false; } - const hasPermission = rules.has(permissionId); - return modifier === 'deny' ? !hasPermission : hasPermission; - } - - static isPermissionRestrictedForRole(permissionId: string, roleId: string): boolean { - return this.checkPermissionForRole(permissionId, roleId, restrictedRolePermissions); - } - - static isPermissionDisabledForRole(permissionId: string, roleId: string): boolean { - return this.checkPermissionForRole(permissionId, roleId, disabledRolePermissions, 'allow'); + return !rules.has(permissionId); } static isPermissionRestrictedForRoleList(permissionId: string, roleList: string[]): boolean { diff --git a/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts b/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts index 97b2e2f80de93..57bbb0baf1afa 100644 --- a/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts +++ b/apps/meteor/client/views/root/hooks/loggedIn/useRestrictedRoles.ts @@ -17,8 +17,6 @@ export const useRestrictedRoles = (): void => { return; } - AuthorizationUtils.addRolePermissionDisabledList('admin', ['access-permissions']); - if (isEnterprise) { AuthorizationUtils.addRolePermissionWhiteList('guest', [ 'view-d-room', diff --git a/apps/meteor/jest.config.ts b/apps/meteor/jest.config.ts index 2e3e2ddb2e9f1..92cc6f4706569 100644 --- a/apps/meteor/jest.config.ts +++ b/apps/meteor/jest.config.ts @@ -13,7 +13,6 @@ export default { '/client/**/**.spec.[jt]s?(x)', '/ee/client/**/**.spec.[jt]s?(x)', '/app/ui-message/client/**/**.spec.[jt]s?(x)', - '/app/authorization/**/**.spec.[jt]s?(x)', '/tests/unit/client/views/**/*.spec.{ts,tsx}', '/tests/unit/client/providers/**/*.spec.{ts,tsx}', ], From 8fe2acf6711ded87cd7a8712a866c8bb03d5496e Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 7 Oct 2025 11:40:17 -0300 Subject: [PATCH 5/7] feat: add confirmation modal --- apps/meteor/app/authorization/lib/index.ts | 2 ++ .../permissions/PermissionsTable/RoleCell.tsx | 30 ++++++++++++++++--- packages/i18n/src/locales/en.i18n.json | 1 + 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/apps/meteor/app/authorization/lib/index.ts b/apps/meteor/app/authorization/lib/index.ts index 8e2a37a64138a..c8e5e7bbdc017 100644 --- a/apps/meteor/app/authorization/lib/index.ts +++ b/apps/meteor/app/authorization/lib/index.ts @@ -9,4 +9,6 @@ export const CONSTANTS = { SETTINGS_LEVEL: 'settings', } as const; +export const confirmationRequiredPermissions = ['access-permissions']; + export { AuthorizationUtils } from './AuthorizationUtils'; diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx index 18803f63c46f0..22a417970cb14 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx @@ -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 = { @@ -17,20 +20,39 @@ type RoleCellProps = { }; const RoleCell = ({ _id, name, description, onChange, permissionId, 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 isDisabledForRole = AuthorizationUtils.isPermissionDisabledForRole(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( + setModal(null)} confirmText={t('Remove')} variant='danger'> + {t('Last_role_in_permission_warning')} + , + ); + } + + return handleConfirmChange(); + }); + + const handleConfirmChange = useEffectEvent(async () => { setLoading(true); const result = await onChange(_id, granted); setGranted(result); setLoading(false); }); - const isDisabled = !!loading || !!isRestrictedForRole || !!isDisabledForRole; + const isDisabled = !!loading || !!isRestrictedForRole; return ( diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 54815d3ee34d3..14946bb27c5b9 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -2904,6 +2904,7 @@ "Language_Version": "English Version", "Language_setting_warning": "Server language setting does not affect user's client
Each user has their own preference for language, that will be kept if this setting is changed.", "Larger_amounts_of_active_connections": "For larger amounts of active connections you can consider our <1>multiple instance solutions.", + "Last_role_in_permission_warning": "This is the last role with this permission. Removing it will soft-lock this page from the UI, requiring database access to revert it. Do you want to proceed?", "Last_5_minutes": "Last 5 minutes", "Last_15_minutes": "Last 15 minutes", "Last_30_minutes": "Last 30 minutes", From 7d116f4d908640b74c19240717c1dacd678ad1aa Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 7 Oct 2025 11:49:08 -0300 Subject: [PATCH 6/7] chore: update changeset --- .changeset/spicy-zebras-deliver.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/spicy-zebras-deliver.md b/.changeset/spicy-zebras-deliver.md index 337808fa8e334..632edbb298cba 100644 --- a/.changeset/spicy-zebras-deliver.md +++ b/.changeset/spicy-zebras-deliver.md @@ -2,4 +2,4 @@ '@rocket.chat/meteor': minor --- -Prevents the `access-permissions` permission from being removed from the `admin` role in the permissions table. This hardening measure ensures that administrators cannot accidentally lock themselves out of the system's permission management screen. +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. From d1600560067aae5ca4421631a1f3c9321cd5e1fe Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 7 Oct 2025 12:38:35 -0300 Subject: [PATCH 7/7] test: add unit tests --- .../PermissionsTable/PermissionRow.tsx | 4 +- .../PermissionsTable.spec.tsx | 59 ++++++++++++++++++- .../PermissionsTable.stories.tsx | 20 ------- .../PermissionsTable/PermissionsTable.tsx | 2 +- .../permissions/PermissionsTable/RoleCell.tsx | 6 +- .../PermissionsTable.spec.tsx.snap | 40 +++++++++++-- 6 files changed, 102 insertions(+), 29 deletions(-) diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx index e16aa85a60cb1..2c0392e7f761b 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionRow.tsx @@ -36,11 +36,12 @@ const PermissionRow = ({ permission, roleList, onGrant, onRemove }: PermissionRo const { t } = useTranslation(); const { _id: permissionId, roles } = permission; const changeRole = useChangeRole({ onGrant, onRemove, permissionId }); + const permissionName = getName(t, permission); return ( - {getName(t, permission)} + {permissionName} {roleList.map(({ _id: roleId, name, description }) => ( ))} diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.spec.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.spec.tsx index 3922eab005c32..c1116b09798cc 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.spec.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.spec.tsx @@ -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]); @@ -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( + 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( undefined} roleList={roles} />, { + wrapper: mockAppRoot().build(), + }); + + await userEvent.click(screen.getByRole('checkbox', { name: 'access-permissions - Administrator' })); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); +}); diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.stories.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.stories.tsx index 7ef2e4dfc01fe..c553c19f36fb3 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.stories.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.stories.tsx @@ -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, }, ]; diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.tsx index c3752505e7afe..e034adfcf1f46 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTable.tsx @@ -16,7 +16,7 @@ type PermissionsTableProps = { permissions: IPermission[]; setFilter: (filter: string) => void; total: number; - paginationProps: ReturnType; + paginationProps?: ReturnType; }; const PermissionsTable = ({ roleList, permissions, setFilter, total, paginationProps }: PermissionsTableProps) => { diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx index 22a417970cb14..70eddbafcc098 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/RoleCell.tsx @@ -16,10 +16,11 @@ type RoleCellProps = { description: IRole['description']; onChange: (roleId: IRole['_id'], granted: boolean) => Promise; 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)); @@ -53,11 +54,12 @@ const RoleCell = ({ _id, name, description, onChange, permissionId, grantedRoles }); const isDisabled = !!loading || !!isRestrictedForRole; + const checkboxLabel = `${permissionName} - ${description || name}`; return ( - + {!loading && ( {description || name} diff --git a/apps/meteor/client/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snap b/apps/meteor/client/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snap index c3685ddaaf682..397abc9b43e5d 100644 --- a/apps/meteor/client/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snap +++ b/apps/meteor/client/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snap @@ -270,7 +270,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-table__cell rcx-css-117bfyn" title="0_description" > - admin > General > general_settings + 0 @@ -300,6 +301,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -343,6 +346,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -364,6 +368,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -385,6 +390,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -406,6 +412,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -427,6 +434,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -451,7 +459,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-table__cell rcx-css-117bfyn" title="1_description" > - admin > General > general_settings + 1 @@ -481,6 +490,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -502,6 +512,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -523,6 +534,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -544,6 +556,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -587,6 +601,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -608,6 +623,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -632,7 +648,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-table__cell rcx-css-117bfyn" title="2_description" > - admin > General > general_settings + 2 @@ -662,6 +679,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -683,6 +701,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -704,6 +723,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -725,6 +745,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -768,6 +790,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -789,6 +812,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -813,7 +837,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-table__cell rcx-css-117bfyn" title="3_description" > - admin > General > general_settings + 3 @@ -843,6 +868,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -864,6 +890,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -885,6 +912,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -906,6 +934,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -949,6 +979,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" > @@ -970,6 +1001,7 @@ exports[`renders Default without crashing 1`] = ` class="rcx-box rcx-box--full rcx-check-box rcx-css-17ztjgq rcx-css-wcp0mp" >