MM 57516 - deactivate actions to ldap users (#28199)

* MM-57516 - restrict activation/deactivation over ldap users

* Add unit tests

* refactor test to unify repeated actions

* add disable actions in user details too

* migrate test to use react-testing-library

* add new ldap user test and fix other existing tests

* restrict ldap users status management via api

* use correct server status and update tests

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Pablo Vélez 2024-10-09 18:59:53 +02:00 committed by GitHub
parent ac38f5f751
commit 6cafd45fc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 1413 additions and 851 deletions

View File

@ -1540,6 +1540,11 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if user.AuthService == model.UserAuthServiceLdap {
c.Err = model.NewAppError("updateUserActive", "api.user.update_active.cannot_modify_status_when_user_is_managed_by_ldap.app_error", nil, "userId="+c.Params.UserId, http.StatusForbidden)
return
}
if _, err = c.App.UpdateActive(c.AppContext, user, active); err != nil {
c.Err = err
return

View File

@ -2684,6 +2684,31 @@ func TestUpdateUserActive(t *testing.T) {
require.NoError(t, err)
})
})
t.Run("update active status of LDAP user should fail", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
ldapUser := &model.User{
Email: "ldapuser@mattermost-customer.com",
Username: "ldapuser",
Password: "Password123",
AuthService: model.UserAuthServiceLdap,
EmailVerified: true,
}
user, appErr := th.App.CreateUser(th.Context, ldapUser)
require.Nil(t, appErr)
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
resp, err := client.UpdateUserActive(context.Background(), user.Id, false)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
resp, err = client.UpdateUserActive(context.Background(), user.Id, true)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
})
}
func TestGetUsers(t *testing.T) {

View File

@ -4178,6 +4178,10 @@
"id": "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error",
"translation": "You cannot activate a guest account because Guest Access feature is not enabled."
},
{
"id": "api.user.update_active.cannot_modify_status_when_user_is_managed_by_ldap.app_error",
"translation": "You cannot modify user status. User is managed by LDAP"
},
{
"id": "api.user.update_active.not_enable.app_error",
"translation": "You cannot deactivate yourself because this feature is not enabled. Please contact your System Administrator."

View File

@ -1,18 +1,29 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import '@testing-library/jest-dom';
import React from 'react';
import type {IntlShape} from 'react-intl';
import type {RouteComponentProps} from 'react-router-dom';
import type {UserProfile} from '@mattermost/types/users';
import SystemUserDetail, {getUserAuthenticationTextField} from 'components/admin_console/system_user_detail/system_user_detail';
import type {
Props,
Params,
} from 'components/admin_console/system_user_detail/system_user_detail';
import type {Params, Props} from 'components/admin_console/system_user_detail/system_user_detail';
import {shallowWithIntl, type MockIntl} from 'tests/helpers/intl-test-helper';
import type {MockIntl} from 'tests/helpers/intl-test-helper';
import {renderWithContext, waitFor, within} from 'tests/react_testing_utils';
import Constants from 'utils/constants';
import {TestHelper} from 'utils/test_helper';
// Mock user profile data
const user = Object.assign(TestHelper.getUserMock(), {auth_service: Constants.EMAIL_SERVICE}) as UserProfile;
const ldapUser = {...user, auth_service: Constants.LDAP_SERVICE} as UserProfile;
// Mock getUser action result
const getUserMock = jest.fn().mockResolvedValue({data: user, error: null});
const getLdapUserMock = jest.fn().mockResolvedValue({data: ldapUser, error: null});
describe('SystemUserDetail', () => {
const defaultProps: Props = {
@ -21,7 +32,7 @@ describe('SystemUserDetail', () => {
mfaEnabled: false,
patchUser: jest.fn(),
updateUserMfa: jest.fn(),
getUser: jest.fn(),
getUser: getUserMock,
updateUserActive: jest.fn(),
setNavigationBlocked: jest.fn(),
addUserToTeam: jest.fn(),
@ -39,51 +50,93 @@ describe('SystemUserDetail', () => {
} as RouteComponentProps<Params>),
};
test('should match default snapshot', () => {
const waitForLoadingToFinish = async (container: HTMLElement) => {
const noUserBody = container.querySelector('.noUserBody');
const spinner = within(noUserBody as HTMLElement).getByTestId('loadingSpinner');
expect(spinner).toBeInTheDocument();
await waitFor(() => {
expect(container.querySelector('[data-testid="loadingSpinner"]')).not.toBeInTheDocument();
});
};
test('should match default snapshot', async () => {
const props = defaultProps;
const wrapper = shallowWithIntl(<SystemUserDetail {...props}/>);
expect(wrapper).toMatchSnapshot();
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
expect(container).toMatchSnapshot();
});
test('should match snapshot if MFA is enabled', () => {
test('should match snapshot if MFA is enabled', async () => {
const props = {
...defaultProps,
mfaEnabled: true,
};
const wrapper = shallowWithIntl(<SystemUserDetail {...props}/>);
expect(wrapper).toMatchSnapshot();
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
expect(container).toMatchSnapshot();
});
test('should show manage user settings button as activated', () => {
test('should show manage user settings button as activated', async () => {
const props = {
...defaultProps,
showManageUserSettings: true,
};
const wrapper = shallowWithIntl(<SystemUserDetail {...props}/>);
expect(wrapper).toMatchSnapshot();
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
expect(container).toMatchSnapshot();
});
test('should show manage user settings button as disabled when no license', () => {
test('should show manage user settings button as disabled when no license', async () => {
const props = {
...defaultProps,
showLockedManageUserSettings: false,
};
const wrapper = shallowWithIntl(<SystemUserDetail {...props}/>);
expect(wrapper).toMatchSnapshot();
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
expect(container).toMatchSnapshot();
});
test('should not show manage user settings button when user doesnt have permission', () => {
test('should show the activate user button as disabled when user is LDAP', async () => {
const props = {
...defaultProps,
getUser: getLdapUserMock,
isLoading: false,
};
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
const activateButton = container.querySelector('button[disabled]');
expect(activateButton).toHaveTextContent('Deactivate (Managed By LDAP)');
expect(container).toMatchSnapshot();
});
test('should not show manage user settings button when user doesn\'t have permission', async () => {
const props = {
...defaultProps,
showManageUserSettings: false,
};
const wrapper = shallowWithIntl(<SystemUserDetail {...props}/>);
expect(wrapper).toMatchSnapshot();
const {container} = renderWithContext(<SystemUserDetail {...props}/>);
await waitForLoadingToFinish(container);
expect(container).toMatchSnapshot();
});
});
describe('getUserAuthenticationTextField', () => {
const intl = {formatMessage: ({defaultMessage}) => defaultMessage} as MockIntl;
const intl = {formatMessage: ({defaultMessage}: {defaultMessage: string}) => defaultMessage} as IntlShape;
it('should return empty string if user is not provided', () => {
const result = getUserAuthenticationTextField(intl, false, undefined);

View File

@ -133,7 +133,7 @@ export class SystemUserDetail extends PureComponent<Props, State> {
};
handleActivateUser = async () => {
if (!this.state.user) {
if (!this.state.user || this.state.user?.auth_service === Constants.LDAP_SERVICE) {
return;
}
@ -261,6 +261,9 @@ export class SystemUserDetail extends PureComponent<Props, State> {
*/
toggleOpenModalDeactivateMember = () => {
if (this.state.user?.auth_service === Constants.LDAP_SERVICE) {
return;
}
this.setState({showDeactivateMemberModal: true});
};
@ -319,6 +322,21 @@ export class SystemUserDetail extends PureComponent<Props, State> {
});
};
getManagedByLdapText = () => {
if (this.state.user?.auth_service !== Constants.LDAP_SERVICE) {
return null;
}
return (
<>
{' '}
<FormattedMessage
id='admin.user_item.managedByLdap'
defaultMessage='(Managed By LDAP)'
/>
</>
);
};
render() {
return (
<div className='SystemUserDetail wrapper--fixed'>
@ -402,22 +420,26 @@ export class SystemUserDetail extends PureComponent<Props, State> {
<button
className='btn btn-secondary'
onClick={this.handleActivateUser}
disabled={this.state.user?.auth_service === Constants.LDAP_SERVICE}
>
<FormattedMessage
id='admin.user_item.makeActive'
defaultMessage='Activate'
/>
{this.getManagedByLdapText()}
</button>
)}
{this.state.user?.delete_at === 0 && (
<button
className='btn btn-secondary btn-danger'
onClick={this.toggleOpenModalDeactivateMember}
disabled={this.state.user?.auth_service === Constants.LDAP_SERVICE}
>
<FormattedMessage
id='admin.user_item.deactivate'
defaultMessage='Deactivate'
/>
{this.getManagedByLdapText()}
</button>
)}

View File

@ -114,8 +114,11 @@ export default class TeamList extends React.PureComponent<Props, State> {
private mergeTeamsWithMemberships = (data: [ActionResult<Team[]>, ActionResult<TeamMembership[]>]): TeamWithMembership[] => {
const teams = data[0].data;
const memberships = data[1].data;
let teamsWithMemberships = teams!.map((object: Team) => {
const results = memberships!.filter((team: TeamMembership) => team.team_id === object.id);
if (!teams || !memberships) {
return [];
}
let teamsWithMemberships = teams.map((object: Team) => {
const results = memberships.filter((team: TeamMembership) => team.team_id === object.id);
const team = {...object, ...results[0]};
return team;
});

View File

@ -0,0 +1,115 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {waitFor, screen, within} from '@testing-library/react';
import React from 'react';
import '@testing-library/jest-dom';
import type {UserProfile} from '@mattermost/types/users';
import {haveISystemPermission} from 'mattermost-redux/selectors/entities/roles_helpers';
import {renderWithContext, userEvent} from 'tests/react_testing_utils';
import Constants from 'utils/constants';
import {TestHelper} from 'utils/test_helper';
import {SystemUsersListAction} from './index';
jest.mock('mattermost-redux/selectors/entities/roles_helpers', () => ({
...jest.requireActual('mattermost-redux/selectors/entities/roles_helpers'),
haveISystemPermission: jest.fn(),
}));
jest.mock('mattermost-redux/selectors/entities/common', () => {
const {TestHelper} = jest.requireActual('utils/test_helper');
const currentUser = TestHelper.getUserMock({
id: 'other_user_id',
roles: 'system_admin',
username: 'other-user',
});
return {
...jest.requireActual('mattermost-redux/selectors/entities/common') as typeof import('mattermost-redux/selectors/entities/users'),
getCurrentUser: () => currentUser,
};
});
describe('SystemUsersListAction Component', () => {
const onError = jest.fn();
const updateUser = jest.fn();
const currentUser = TestHelper.getUserMock({
id: 'other_user_id',
roles: 'system_admin',
username: 'other-user',
});
const user = Object.assign(TestHelper.getUserMock(), {auth_service: 'email'}) as UserProfile;
const ldapUser = {...user, auth_service: Constants.LDAP_SERVICE} as UserProfile;
const deactivatedLDAPUser = {...user, auth_service: Constants.LDAP_SERVICE, delete_at: 12345} as UserProfile;
beforeEach(() => {
(haveISystemPermission as jest.Mock).mockImplementation(() => true);
});
afterEach(() => {
jest.clearAllMocks();
});
const renderComponent = (authServiceUser: UserProfile) => {
renderWithContext(
<SystemUsersListAction
user={authServiceUser}
currentUser={currentUser}
tableId='testing'
rowIndex={0}
onError={onError}
updateUser={updateUser}
/>,
);
};
const openMenuAndFindItem = async (buttonText: string, itemText: RegExp) => {
const menuButton = screen.getByText(buttonText);
await userEvent.click(menuButton);
await waitFor(() => {
expect(screen.getByRole('menuitem', {name: itemText})).toBeInTheDocument();
});
return screen.findByRole('menuitem', {name: itemText});
};
const verifyDisabledMenuItem = (menuItem: HTMLElement, disabledText: RegExp) => {
expect(menuItem).toHaveAttribute('aria-disabled', 'true');
expect(menuItem).toHaveClass('Mui-disabled');
expect(within(menuItem).getByText(disabledText)).toBeInTheDocument();
};
test('Deactivate button is disabled and contains the Managed by LDAP text when user authmethod is LDAP', async () => {
renderComponent(ldapUser);
const deactivateMenuItem = await openMenuAndFindItem('Member', /deactivate/i);
// Verify that the item is disabled and contains "Managed by LDAP"
verifyDisabledMenuItem(deactivateMenuItem, /Managed by LDAP/i);
});
test('Activate button is disabled and contains the Managed by LDAP text when user authmethod is LDAP', async () => {
renderComponent(deactivatedLDAPUser);
const activateMenuItem = await openMenuAndFindItem('Deactivated', /activate/i);
// Verify that the item is disabled and contains "Managed by LDAP"
verifyDisabledMenuItem(activateMenuItem, /Managed by LDAP/i);
});
test('element is enabled and does NOT contain the Managed by LDAP text when user authmethod is NOT LDAP', async () => {
renderComponent(user);
const deactivateMenuItem = await openMenuAndFindItem('Member', /deactivate/i);
// Check if the item is enabled and does NOT contain "Managed by LDAP"
expect(deactivateMenuItem).not.toHaveAttribute('aria-disabled', 'true');
expect(deactivateMenuItem).not.toHaveClass('Mui-disabled');
expect(within(deactivateMenuItem).queryByText(/Managed by LDAP/i)).not.toBeInTheDocument();
});
});

View File

@ -281,6 +281,9 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE
}, [user.id, user.auth_service, updateUser, onError]);
const handleDeactivateMemberClick = useCallback(() => {
if (user.auth_service === Constants.LDAP_SERVICE) {
return;
}
function onDeactivateMemberSuccess() {
updateUser({delete_at: new Date().getMilliseconds()});
}
@ -298,6 +301,17 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE
);
}, [user, updateUser, onError]);
const disableActivationToggle = user.auth_service === Constants.LDAP_SERVICE;
const getManagedByLDAPText = (managedByLDAP: boolean) => {
return managedByLDAP ? {
trailingElements: formatMessage({
id: 'admin.system_users.list.actions.menu.managedByLdap',
defaultMessage: 'Managed by LDAP',
}),
} : {};
};
return (
<Menu.Container
menuButton={{
@ -335,7 +349,8 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE
defaultMessage='Activate'
/>
}
disabled={user.auth_service === Constants.LDAP_SERVICE}
disabled={disableActivationToggle}
{...getManagedByLDAPText(disableActivationToggle)}
onClick={handleActivateUserClick}
/>
)}
@ -496,6 +511,8 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE
/>
}
onClick={handleDeactivateMemberClick}
disabled={disableActivationToggle}
{...getManagedByLDAPText(disableActivationToggle)}
/>
)}
</Menu.Container>

View File

@ -2563,6 +2563,7 @@
"admin.system_users.list.actions.menu.deactivate": "Deactivate",
"admin.system_users.list.actions.menu.demoteToGuest": "Demote to guest",
"admin.system_users.list.actions.menu.dropdownAriaLabel": "User actions menu",
"admin.system_users.list.actions.menu.managedByLdap": "(Managed by LDAP)",
"admin.system_users.list.actions.menu.manageRoles": "Manage roles",
"admin.system_users.list.actions.menu.manageSettings": "Manage user settings",
"admin.system_users.list.actions.menu.manageTeams": "Manage teams",
@ -2773,6 +2774,7 @@
"admin.user_item.makeActive": "Activate",
"admin.user_item.makeMember": "Make Team Member",
"admin.user_item.makeTeamAdmin": "Make Team Admin",
"admin.user_item.managedByLdap": "(Managed By LDAP)",
"admin.user_item.manageSettings": "Manage User Settings",
"admin.user_item.manageSettings.confirm_dialog.body": "You are about to access {userDisplayName}'s account settings. Any modifications you make will take effect immediately in their account. {userDisplayName} retains the ability to view and modify these settings at any time.<br></br><br></br> Are you sure you want to proceed with managing {userDisplayName}'s settings?",
"admin.user_item.manageSettings.disabled_tooltip": "Please upgrade to Enterprise to manage user settings",