MM-64713 - channel invite modal in abac channel should keep filtered list of users (#32803)

Этот коммит содержится в:
Pablo Vélez
2025-07-10 20:03:54 +02:00
коммит произвёл GitHub
родитель d1e5fdea2c
Коммит a17c387ff2
3 изменённых файлов: 192 добавлений и 14 удалений

Просмотреть файл

@@ -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(
<ChannelInviteModal
@@ -605,6 +639,10 @@ describe('components/channel_invite_modal', () => {
) 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(<ChannelInviteModal {...props}/>);
});
// 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 <span> 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(<ChannelInviteModal {...props}/>);
});
// 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(<ChannelInviteModal {...props}/>);
});
// 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(<ChannelInviteModal {...props}/>);
});
// 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();
});
});

Просмотреть файл

@@ -100,6 +100,7 @@ const ChannelInviteModalComponent = (props: Props) => {
const [groupAndUserOptions, setGroupAndUserOptions] = useState<Array<UserProfileValue | GroupValue>>([]);
const [inviteError, setInviteError] = useState<string | undefined>(undefined);
const [pageCursors, setPageCursors] = useState<{[page: number]: string}>({});
const [abacFilteredUsers, setAbacFilteredUsers] = useState<UserProfile[]>([]);
const searchTimeoutId = useRef<number>(0);
const selectedItemRef = useRef<HTMLDivElement>(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

Просмотреть файл

@@ -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);