Skip to content
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { render, waitFor } from '@testing-library/react';
import { useEffect } from 'react';

import ComposerPopupProvider from './ComposerPopupProvider';
import { useComposerPopupOptions } from '../contexts/ComposerPopupContext';
import FakeRoomProvider from '../../../../tests/mocks/client/FakeRoomProvider';
import { createFakeRoom } from '../../../../tests/mocks/data';

const mockGrantedPermissions = new Set<string>();

jest.mock('../../../../app/authorization/client', () => ({
hasAtLeastOnePermission: (permissions: string[] | string) => {
const permissionList = Array.isArray(permissions) ? permissions : [permissions];

return permissionList.some((permission) => mockGrantedPermissions.has(permission));
},
Comment on lines +12 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make the permission mock enforce the room scope.

The mock ignores scope, so these tests would still pass if ComposerPopupProvider accidentally called hasAtLeastOnePermission('mention-all') without rid. Encode the room id into the mocked grant, or assert the second argument, so the test protects the scoped permission behavior.

🧪 Proposed test hardening
 const mockGrantedPermissions = new Set<string>();

 jest.mock('../../../../app/authorization/client', () => ({
-	hasAtLeastOnePermission: (permissions: string[] | string) => {
+	hasAtLeastOnePermission: (permissions: string[] | string, scope?: string) => {
 		const permissionList = Array.isArray(permissions) ? permissions : [permissions];

-		return permissionList.some((permission) => mockGrantedPermissions.has(permission));
+		return permissionList.some((permission) => mockGrantedPermissions.has(`${scope}:${permission}`));
 	},
 }));
 const renderProvider = async (permissions: string[] = []) => {
-	mockGrantedPermissions.clear();
-	permissions.forEach((permission) => mockGrantedPermissions.add(permission));
-
-	const room = createFakeRoom({ t: 'c' });
-	const appRoot = permissions.reduce((wrapper, permission) => wrapper.withPermission(permission), mockAppRoot().withJohnDoe()).build();
+	const room = createFakeRoom({ _id: 'permission-scoped-room', t: 'c' });
+
+	mockGrantedPermissions.clear();
+	permissions.forEach((permission) => mockGrantedPermissions.add(`${room._id}:${permission}`));
+
+	const appRoot = mockAppRoot().withJohnDoe().build();

apps/meteor/app/authorization/client/hasPermission.ts:72-73 exposes scope as the second argument, and lines 41-64 pass it into validation.

Also applies to: 40-45

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/client/views/room/providers/ComposerPopupProvider.spec.tsx`
around lines 12 - 17, The permission mock for hasAtLeastOnePermission ignores
the scope/rid so tests don't verify scoped checks; update the mocked function in
ComposerPopupProvider.spec.tsx (the jest.mock that defines
hasAtLeastOnePermission and uses mockGrantedPermissions) to accept the second
argument (scope) and validate that the granted entry includes the room id—either
by asserting scope === expectedRid when called or by encoding grants with a
composite key (e.g., `${scope}:${permission}`) and checking
mockGrantedPermissions for that composite, so the mock enforces room-scoped
permission checks used by ComposerPopupProvider.

}));

jest.mock('../../../../app/utils/client', () => ({
slashCommands: {
commands: {},
},
}));

type PopupOptionsConsumerProps = {
onReady: (options: ReturnType<typeof useComposerPopupOptions>) => void;
};

const PopupOptionsConsumer = ({ onReady }: PopupOptionsConsumerProps) => {
const options = useComposerPopupOptions();

useEffect(() => {
onReady(options);
}, [onReady, options]);

return null;
};

const renderProvider = async (permissions: string[] = []) => {
mockGrantedPermissions.clear();
permissions.forEach((permission) => mockGrantedPermissions.add(permission));

const room = createFakeRoom({ t: 'c' });
const appRoot = permissions.reduce((wrapper, permission) => wrapper.withPermission(permission), mockAppRoot().withJohnDoe()).build();

let popupOptions: ReturnType<typeof useComposerPopupOptions> | undefined;

render(
<FakeRoomProvider roomOverrides={room}>
<ComposerPopupProvider room={room}>
<PopupOptionsConsumer
onReady={(options) => {
popupOptions = options;
}}
/>
</ComposerPopupProvider>
</FakeRoomProvider>,
{
wrapper: appRoot,
},
);

await waitFor(() => expect(popupOptions).toBeDefined());

const mentionPopup = popupOptions?.find(({ trigger }) => trigger === '@');
expect(mentionPopup).toBeDefined();

const items = await mentionPopup?.getItemsFromLocal?.('');

return items?.map(({ _id }) => _id) ?? [];
};

describe('ComposerPopupProvider', () => {
it('does not show @all or @here in autocomplete when user does not have permissions', async () => {
const itemIds = await renderProvider();

expect(itemIds).not.toContain('all');
expect(itemIds).not.toContain('here');
});

it('shows only @all when user has mention-all permission', async () => {
const itemIds = await renderProvider(['mention-all']);

expect(itemIds).toContain('all');
expect(itemIds).not.toContain('here');
});

it('shows only @here when user has mention-here permission', async () => {
const itemIds = await renderProvider(['mention-here']);

expect(itemIds).toContain('here');
expect(itemIds).not.toContain('all');
});

it('shows both @all and @here when user has both permissions', async () => {
const itemIds = await renderProvider(['mention-all', 'mention-here']);

expect(itemIds).toContain('all');
expect(itemIds).toContain('here');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ const ComposerPopupProvider = ({ children, room }: ComposerPopupProviderProps) =
const filterRegex = filter && new RegExp(escapeRegExp(filter), 'i');
const items: ComposerBoxPopupUserProps[] = [];

const canMentionAll = hasAtLeastOnePermission('mention-all', rid);
const canMentionHere = hasAtLeastOnePermission('mention-here', rid);

const roomMessageUsers = getLastRecentUsers(rid, uid!)
.filter((u) => {
if (!filterRegex) return true;
Expand All @@ -106,7 +109,7 @@ const ComposerPopupProvider = ({ children, room }: ComposerPopupProviderProps) =
suggestion: true,
}));

if (!filterRegex || filterRegex.test('all')) {
if (canMentionAll && (!filterRegex || filterRegex.test('all'))) {
items.push({
_id: 'all',
username: 'all',
Expand All @@ -116,7 +119,7 @@ const ComposerPopupProvider = ({ children, room }: ComposerPopupProviderProps) =
});
}

if (!filterRegex || filterRegex.test('here')) {
if (canMentionHere && (!filterRegex || filterRegex.test('here'))) {
items.push({
_id: 'here',
username: 'here',
Expand Down