From a17c387ff24a93c215a727e87856717a04af0ecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20V=C3=A9lez?= Date: Thu, 10 Jul 2025 20:03:54 +0200 Subject: [PATCH] MM-64713 - channel invite modal in abac channel should keep filtered list of users (#32803) --- .../channel_invite_modal.test.tsx | 129 +++++++++++++++++- .../channel_invite_modal.tsx | 59 ++++++-- .../components/channel_invite_modal/index.ts | 18 ++- 3 files changed, 192 insertions(+), 14 deletions(-) diff --git a/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.test.tsx b/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.test.tsx index b242497f31..6c6a98868c 100644 --- a/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.test.tsx +++ b/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.test.tsx @@ -57,6 +57,19 @@ jest.mock('utils/utils', () => { }; }); +// Mock Client4 for ABAC tests +jest.mock('mattermost-redux/client', () => ({ + Client4: { + getProfilesNotInChannel: jest.fn(), + getProfilePictureUrl: jest.fn(() => 'mock-url'), + getUsersRoute: jest.fn(() => '/api/v4/users'), + getTeamsRoute: jest.fn(() => '/api/v4/teams'), + getChannelsRoute: jest.fn(() => '/api/v4/channels'), + getUrl: jest.fn(() => 'http://localhost:8065'), + getBaseRoute: jest.fn(() => '/api/v4'), + }, +})); + describe('components/channel_invite_modal', () => { const users = [{ id: 'user-1', @@ -132,6 +145,27 @@ describe('components/channel_invite_modal', () => { }; }); + beforeEach(() => { + // Reset Client4 mocks before each test + const {Client4} = require('mattermost-redux/client'); + Client4.getProfilesNotInChannel.mockClear(); + Client4.getProfilePictureUrl.mockClear(); + Client4.getUsersRoute.mockClear(); + Client4.getTeamsRoute.mockClear(); + Client4.getChannelsRoute.mockClear(); + Client4.getUrl.mockClear(); + Client4.getBaseRoute.mockClear(); + + // Set default return values + Client4.getProfilesNotInChannel.mockResolvedValue([]); + Client4.getProfilePictureUrl.mockReturnValue('mock-url'); + Client4.getUsersRoute.mockReturnValue('/api/v4/users'); + Client4.getTeamsRoute.mockReturnValue('/api/v4/teams'); + Client4.getChannelsRoute.mockReturnValue('/api/v4/channels'); + Client4.getUrl.mockReturnValue('http://localhost:8065'); + Client4.getBaseRoute.mockReturnValue('/api/v4'); + }); + test('should match snapshot for channel_invite_modal with profiles', () => { const wrapper = shallowWithIntl( { ) as HTMLElement; test('should not include DM users when ABAC is enabled', async () => { + // Mock Client4 to return user-1 for ABAC channels + const {Client4} = require('mattermost-redux/client'); + Client4.getProfilesNotInChannel.mockResolvedValue([users[0]]); + const channelWithPolicy = {...channel, policy_enforced: true}; const props = { ...baseProps, @@ -617,9 +655,19 @@ describe('components/channel_invite_modal', () => { renderWithContext(); }); + // Wait for the API call to complete and state to update + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + const input = screen.getByRole('combobox', {name: /search for people/i}); await userEvent.type(input, 'user'); + // Wait for the search to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + // now only one visible should match "user-1" expect(getUserSpan('user-1')).toBeInTheDocument(); @@ -764,6 +812,10 @@ describe('components/channel_invite_modal', () => { }); test('should filter out groups when ABAC is enforced', async () => { + // Mock Client4 to return user-1 for ABAC channels + const {Client4} = require('mattermost-redux/client'); + Client4.getProfilesNotInChannel.mockResolvedValue([users[0]]); + const mockGroups = [ { id: 'group1', @@ -798,8 +850,18 @@ describe('components/channel_invite_modal', () => { renderWithContext(); }); + // Wait for the API call to complete and state to update + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + const input = screen.getByRole('combobox', {name: /search for people/i}); - await userEvent.type(input, '@'); + await userEvent.type(input, 'user'); + + // Wait for the search to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); // Should only show users, not groups when ABAC is enforced expect(getUserSpan('user-1')).toBeInTheDocument(); @@ -807,4 +869,69 @@ describe('components/channel_invite_modal', () => { // Groups should not appear in the dropdown expect(screen.queryByText('Developers')).toBeNull(); }); + + test('should force fresh API call when ABAC is enforced', async () => { + // For ABAC channels, we use Client4 directly, not the Redux action + const {Client4} = require('mattermost-redux/client'); + Client4.getProfilesNotInChannel.mockResolvedValue([]); + + const props = { + ...baseProps, + channel: {...channel, policy_enforced: true}, + }; + + await act(async () => { + renderWithContext(); + }); + + // Wait for the API call to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // For ABAC channels, we should call Client4 directly, not the Redux action + expect(Client4.getProfilesNotInChannel).toHaveBeenCalledWith( + props.channel.team_id, + props.channel.id, + props.channel.group_constrained, + 0, + 50, + '', + ); + }); + + test('should ignore contaminated Redux data when ABAC is enforced', async () => { + // Mock Client4 to return only user-1 for ABAC channels (ignoring contaminated Redux data) + const {Client4} = require('mattermost-redux/client'); + Client4.getProfilesNotInChannel.mockResolvedValue([users[0]]); + + const props = { + ...baseProps, + channel: {...channel, policy_enforced: true}, + profilesNotInCurrentChannel: [users[0]], // Clean ABAC data + profilesFromRecentDMs: [users[1]], // Contaminated data + includeUsers: {[users[1].id]: users[1]}, // Contaminated data + }; + + await act(async () => { + renderWithContext(); + }); + + // Wait for the API call to complete and state to update + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + const input = screen.getByRole('combobox', {name: /search for people/i}); + await userEvent.type(input, 'user'); + + // Wait for the search to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // Should only show clean ABAC data + expect(getUserSpan('user-1')).toBeInTheDocument(); + expect(screen.queryByText('user-2')).toBeNull(); + }); }); diff --git a/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.tsx b/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.tsx index a084fea0bf..98112134f6 100644 --- a/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.tsx +++ b/webapp/channels/src/components/channel_invite_modal/channel_invite_modal.tsx @@ -100,6 +100,7 @@ const ChannelInviteModalComponent = (props: Props) => { const [groupAndUserOptions, setGroupAndUserOptions] = useState>([]); const [inviteError, setInviteError] = useState(undefined); const [pageCursors, setPageCursors] = useState<{[page: number]: string}>({}); + const [abacFilteredUsers, setAbacFilteredUsers] = useState([]); const searchTimeoutId = useRef(0); const selectedItemRef = useRef(null); @@ -183,18 +184,18 @@ const ChannelInviteModalComponent = (props: Props) => { let users: UserProfileValue[]; if (props.channel.policy_enforced) { - // When ABAC is enabled, only use the ABAC-filtered profilesNotInCurrentChannel - const filteredUsers = filterProfilesStartingWithTerm(props.profilesNotInCurrentChannel, term); + // ABAC mode: Use local state with fresh API data, completely bypass Redux + const filteredUsers = filterProfilesStartingWithTerm(abacFilteredUsers, term); users = filterOutDeletedAndExcludedAndNotInTeamUsers(filteredUsers, excludedAndNotInTeamUserIds); } else { - // When ABAC is not enabled, use the current logic + // Non-ABAC mode: existing logic const filteredUsers = filterProfilesStartingWithTerm(props.profilesNotInCurrentChannel.concat(props.profilesInCurrentChannel), term); users = filterOutDeletedAndExcludedAndNotInTeamUsers(filteredUsers, excludedAndNotInTeamUserIds); - } - // Only include explicitly added users if ABAC is not enabled - if (props.includeUsers && !props.channel.policy_enforced) { - users = [...users, ...Object.values(props.includeUsers)]; + // Only include explicitly added users if ABAC is not enabled + if (props.includeUsers) { + users = [...users, ...Object.values(props.includeUsers)]; + } } const groupsAndUsers = [ @@ -220,6 +221,7 @@ const ChannelInviteModalComponent = (props: Props) => { props.channel.policy_enforced, excludedUsers, filterOutDeletedAndExcludedAndNotInTeamUsers, + abacFilteredUsers, ]); // Handle modal hide @@ -249,6 +251,24 @@ const ChannelInviteModalComponent = (props: Props) => { setLoadingUsers(loadingState); }, []); + // Custom function to fetch ABAC users without polluting Redux store + const fetchAbacUsers = useCallback(async (page = 0, perPage = USERS_PER_PAGE, cursorId = '') => { + try { + const profiles = await Client4.getProfilesNotInChannel( + props.channel.team_id, + props.channel.id, + props.channel.group_constrained, + page, + perPage, + cursorId, + ); + setAbacFilteredUsers(profiles); + return {data: profiles}; + } catch (error) { + return {error}; + } + }, [props.channel.team_id, props.channel.id, props.channel.group_constrained]); + // Handle page change with cursor-based pagination const handlePageChange = useCallback((page: number, prevPage: number) => { if (page > prevPage) { @@ -443,9 +463,24 @@ const ChannelInviteModalComponent = (props: Props) => { // Initial data loading - only run when channel changes or component mounts useEffect(() => { - props.actions.getProfilesNotInChannel(props.channel.team_id, props.channel.id, props.channel.group_constrained, 0, USERS_PER_PAGE).then(() => { - setUsersLoadingState(false); - }); + if (props.channel.policy_enforced) { + // For ABAC channels, use custom function to avoid Redux store pollution + fetchAbacUsers().then(() => { + setUsersLoadingState(false); + }); + } else { + // For non-ABAC channels, use normal Redux actions + props.actions.getProfilesNotInChannel( + props.channel.team_id, + props.channel.id, + props.channel.group_constrained, + 0, + USERS_PER_PAGE, + ).then(() => { + setUsersLoadingState(false); + }); + } + props.actions.getProfilesInChannel(props.channel.id, 0, USERS_PER_PAGE, '', {active: true}); props.actions.getTeamStats(props.channel.team_id); props.actions.loadStatusesForProfilesList(props.profilesNotInCurrentChannel); @@ -454,7 +489,9 @@ const ChannelInviteModalComponent = (props: Props) => { props.channel.id, props.channel.team_id, props.channel.group_constrained, + props.channel.policy_enforced, props.actions, + fetchAbacUsers, ]); // Compute options with useMemo to ensure they're always fresh @@ -467,6 +504,8 @@ const ChannelInviteModalComponent = (props: Props) => { props.groups, props.profilesNotInCurrentTeam, props.excludeUsers, + props.channel.policy_enforced, // Add this to trigger recomputation when ABAC mode changes + abacFilteredUsers, // Add local ABAC state ]); // Update team members when options change diff --git a/webapp/channels/src/components/channel_invite_modal/index.ts b/webapp/channels/src/components/channel_invite_modal/index.ts index d7560243cf..17270477f1 100644 --- a/webapp/channels/src/components/channel_invite_modal/index.ts +++ b/webapp/channels/src/components/channel_invite_modal/index.ts @@ -10,7 +10,7 @@ import type {UserProfile} from '@mattermost/types/users'; import {getTeamStats, getTeamMembersByIds} from 'mattermost-redux/actions/teams'; import {getProfilesNotInChannel, getProfilesInChannel, searchProfiles} from 'mattermost-redux/actions/users'; import {Permissions} from 'mattermost-redux/constants'; -import {getRecentProfilesFromDMs} from 'mattermost-redux/selectors/entities/channels'; +import {getRecentProfilesFromDMs, getChannel} from 'mattermost-redux/selectors/entities/channels'; import {getConfig, getLicense} from 'mattermost-redux/selectors/entities/general'; import {makeGetAllAssociatedGroupsForReference} from 'mattermost-redux/selectors/entities/groups'; import {getTeammateNameDisplaySetting, isCustomGroupsEnabled} from 'mattermost-redux/selectors/entities/preferences'; @@ -45,12 +45,22 @@ function makeMapStateToProps(initialState: GlobalState, initialProps: OwnProps) } return (state: GlobalState, props: OwnProps) => { + // Check if this is an ABAC channel to bypass contaminated Redux state + const channel = props.channelId ? getChannel(state, props.channelId) : null; + const isAbacChannel = Boolean(channel?.policy_enforced); + let profilesNotInCurrentChannel: UserProfile[]; let profilesInCurrentChannel: UserProfile[]; let profilesNotInCurrentTeam: UserProfile[]; let membersInTeam; - if (props.channelId && props.teamId) { + if (isAbacChannel) { + // For ABAC channels, return empty arrays to force component to use fresh API data + profilesNotInCurrentChannel = []; + profilesInCurrentChannel = []; + profilesNotInCurrentTeam = []; + membersInTeam = props.teamId ? getMembersInTeam(state, props.teamId) : getMembersInCurrentTeam(state); + } else if (props.channelId && props.teamId) { profilesNotInCurrentChannel = doGetProfilesNotInChannel(state, props.channelId); profilesInCurrentChannel = doGetProfilesInChannel(state, props.channelId); profilesNotInCurrentTeam = getProfilesNotInTeam(state, props.teamId); @@ -61,7 +71,9 @@ function makeMapStateToProps(initialState: GlobalState, initialProps: OwnProps) profilesNotInCurrentTeam = getProfilesNotInCurrentTeam(state); membersInTeam = getMembersInCurrentTeam(state); } - const profilesFromRecentDMs = getRecentProfilesFromDMs(state); + + // For ABAC channels, also return empty DM profiles to avoid contamination + const profilesFromRecentDMs = isAbacChannel ? [] : getRecentProfilesFromDMs(state); const config = getConfig(state); const license = getLicense(state);