From 0c585bdac63707e832b5635159cbc96dbf63386b Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Tue, 10 Sep 2024 11:26:55 -0600 Subject: [PATCH] MM-59529 Set Channel/Team Admin permissions if All members get set (#28104) * initial fix * cleanup code * move struct back * fix unit test * add unit tests * add comments * add manage_bookmark permissions * revert package-lock.json --------- Co-authored-by: Mattermost Build --- server/channels/app/permissions_migrations.go | 12 ++++ server/channels/testlib/store.go | 1 + server/public/model/migration.go | 1 + ...permission_system_scheme_settings.test.tsx | 23 ++++++++ .../permission_system_scheme_settings.tsx | 24 +++++++- .../permission_team_scheme_settings.test.tsx | 23 ++++++++ .../permission_team_scheme_settings.tsx | 57 +++++++++---------- webapp/channels/src/utils/constants.tsx | 20 +++++++ 8 files changed, 129 insertions(+), 32 deletions(-) diff --git a/server/channels/app/permissions_migrations.go b/server/channels/app/permissions_migrations.go index b51763b91b..8ab3ce8694 100644 --- a/server/channels/app/permissions_migrations.go +++ b/server/channels/app/permissions_migrations.go @@ -1215,6 +1215,17 @@ func (a *App) getAddManageJobAncillaryPermissionsMigration() (permissionsMap, er return transformations, nil } +func (a *App) getAddUploadFilePermissionMigration() (permissionsMap, error) { + transformations := []permissionTransformation{} + + transformations = append(transformations, permissionTransformation{ + On: permissionExists(model.PermissionCreatePost.Id), + Add: []string{model.PermissionUploadFile.Id}, + }) + + return transformations, nil +} + // DoPermissionsMigrations execute all the permissions migrations need by the current version. func (a *App) DoPermissionsMigrations() error { return a.Srv().doPermissionsMigrations() @@ -1263,6 +1274,7 @@ func (s *Server) doPermissionsMigrations() error { {Key: model.MigrationKeyAddOutgoingOAuthConnectionsPermissions, Migration: a.getAddOutgoingOAuthConnectionsPermissions}, {Key: model.MigrationKeyAddChannelBookmarksPermissions, Migration: a.getAddChannelBookmarksPermissionsMigration}, {Key: model.MigrationKeyAddManageJobAncillaryPermissions, Migration: a.getAddManageJobAncillaryPermissionsMigration}, + {Key: model.MigrationKeyAddUploadFilePermission, Migration: a.getAddUploadFilePermissionMigration}, } roles, err := s.Store().Role().GetAll() diff --git a/server/channels/testlib/store.go b/server/channels/testlib/store.go index 9fca69b6cb..0ca7da7b03 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -78,6 +78,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MigrationKeyAddChannelBookmarksPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelBookmarksPermissions, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyDeleteDmsPreferences).Return(&model.System{Name: model.MigrationKeyDeleteDmsPreferences, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddManageJobAncillaryPermissions).Return(&model.System{Name: model.MigrationKeyAddManageJobAncillaryPermissions, Value: "true"}, nil) + systemStore.On("GetByName", model.MigrationKeyAddUploadFilePermission).Return(&model.System{Name: model.MigrationKeyAddUploadFilePermission, Value: "true"}, nil) systemStore.On("GetByName", "CustomGroupAdminRoleCreationMigrationComplete").Return(&model.System{Name: model.MigrationKeyAddPlayboosksManageRolesPermissions, Value: "true"}, nil) systemStore.On("GetByName", "products_boards").Return(&model.System{Name: "products_boards", Value: "true"}, nil) systemStore.On("GetByName", "elasticsearch_fix_channel_index_migration").Return(&model.System{Name: "elasticsearch_fix_channel_index_migration", Value: "true"}, nil) diff --git a/server/public/model/migration.go b/server/public/model/migration.go index 7ac3c7e4ed..3653b10221 100644 --- a/server/public/model/migration.go +++ b/server/public/model/migration.go @@ -49,4 +49,5 @@ const ( MigrationKeyAddChannelBookmarksPermissions = "add_channel_bookmarks_permissions" MigrationKeyDeleteDmsPreferences = "delete_dms_preferences_migration" MigrationKeyAddManageJobAncillaryPermissions = "add_manage_jobs_ancillary_permissions" + MigrationKeyAddUploadFilePermission = "add_upload_file_permission" ) diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.test.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.test.tsx index 07608ad924..02c19b8830 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.test.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.test.tsx @@ -5,6 +5,8 @@ import React from 'react'; import type {Role} from '@mattermost/types/roles'; +import Permissions from 'mattermost-redux/constants/permissions'; + import PermissionSystemSchemeSettings from 'components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings'; import {shallowWithIntl} from 'tests/helpers/intl-test-helper'; @@ -258,4 +260,25 @@ describe('components/admin_console/permission_schemes_settings/permission_system expect(getAnyState(wrapper).roles.team_admin.permissions).toBe(DefaultRolePermissions.team_admin); expect(getAnyState(wrapper).roles.system_admin.permissions?.length).toBe(defaultProps.roles.system_admin.permissions.length); }); + + test('should set moderated permissions on team/channel admins', () => { + const wrapper = shallowWithIntl( + , + ); + const instance = getAnyInstance(wrapper); + + // A moderated permission should set team/channel admins + instance.togglePermission('all_users', [Permissions.CREATE_POST]); + expect(getAnyState(wrapper).roles.all_users.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.channel_admin.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.team_admin.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.playbook_admin.permissions.indexOf(Permissions.CREATE_POST)).toEqual(-1); + + // Changing a non-moderated permission should NOT set team/channel admins + instance.togglePermission('all_users', [Permissions.EDIT_OTHERS_POSTS]); + expect(getAnyState(wrapper).roles.all_users.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.channel_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + expect(getAnyState(wrapper).roles.team_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + expect(getAnyState(wrapper).roles.playbook_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + }); }); diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx index a1a73b215e..7e8d44e300 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_system_scheme_settings/permission_system_scheme_settings.tsx @@ -20,7 +20,7 @@ import SaveButton from 'components/save_button'; import AdminHeader from 'components/widgets/admin_console/admin_header'; import AdminPanelTogglable from 'components/widgets/admin_console/admin_panel_togglable'; -import {PermissionsScope, DefaultRolePermissions, DocLinks} from 'utils/constants'; +import {PermissionsScope, DefaultRolePermissions, DocLinks, ModeratedPermissions} from 'utils/constants'; import GuestPermissionsTree, {GUEST_INCLUDED_PERMISSIONS} from '../guest_permissions_tree'; import PermissionsTree, {EXCLUDED_PERMISSIONS} from '../permissions_tree'; @@ -62,7 +62,6 @@ type RolesState = { all_users: {name: string; display_name: string; permissions: Role['permissions']}; guests: {name: string; display_name: string; permissions: Role['permissions']}; } - class PermissionSystemSchemeSettings extends React.PureComponent { private rolesNeeded: string[]; @@ -338,6 +337,27 @@ class PermissionSystemSchemeSettings extends React.PureComponent { role.permissions = newPermissions; roles[roleId as keyof RolesState] = role; + if (roleId === 'all_users') { + const channelAdminRole = {...roles.channel_admin} as Role; + const channelAdminPermissions = [...channelAdminRole.permissions!]; + const teamAdminRole = {...roles.team_admin} as Role; + const teamAdminPermissions = [...teamAdminRole.permissions!]; + for (const permission of permissions) { + if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { + if (channelAdminPermissions.indexOf(permission) === -1) { + channelAdminPermissions.push(permission); + } + if (teamAdminPermissions.indexOf(permission) === -1) { + teamAdminPermissions.push(permission); + } + } + } + channelAdminRole.permissions = channelAdminPermissions; + roles.channel_admin = channelAdminRole; + teamAdminRole.permissions = teamAdminPermissions; + roles.team_admin = teamAdminRole; + } + this.setState({roles, saveNeeded: true}); this.props.actions.setNavigationBlocked(true); }; diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.test.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.test.tsx index 8fe12e7754..302413315c 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.test.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.test.tsx @@ -3,6 +3,8 @@ import React from 'react'; +import Permissions from 'mattermost-redux/constants/permissions'; + import PermissionTeamSchemeSettings from 'components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings'; import {shallowWithIntl} from 'tests/helpers/intl-test-helper'; @@ -431,4 +433,25 @@ describe('components/admin_console/permission_schemes_settings/permission_team_s done(); }); }); + + test('should set moderated permissions on team/channel admins', () => { + const wrapper = shallowWithIntl( + , + ); + const instance = getAnyInstance(wrapper); + + // A moderated permission should set team/channel admins + instance.togglePermission('all_users', [Permissions.CREATE_POST]); + expect(getAnyState(wrapper).roles.all_users.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.channel_admin.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.team_admin.permissions.indexOf(Permissions.CREATE_POST)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.playbook_admin.permissions.indexOf(Permissions.CREATE_POST)).toEqual(-1); + + // Changing a non-moderated permission should NOT set team/channel admins + instance.togglePermission('all_users', [Permissions.EDIT_OTHERS_POSTS]); + expect(getAnyState(wrapper).roles.all_users.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toBeGreaterThan(-1); + expect(getAnyState(wrapper).roles.channel_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + expect(getAnyState(wrapper).roles.team_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + expect(getAnyState(wrapper).roles.playbook_admin.permissions.indexOf(Permissions.EDIT_OTHERS_POSTS)).toEqual(-1); + }); }); diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx index 954dfe969f..495a129929 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permission_team_scheme_settings/permission_team_scheme_settings.tsx @@ -25,7 +25,7 @@ import AdminPanel from 'components/widgets/admin_console/admin_panel'; import AdminPanelTogglable from 'components/widgets/admin_console/admin_panel_togglable'; import AdminPanelWithButton from 'components/widgets/admin_console/admin_panel_with_button'; -import {PermissionsScope, ModalIdentifiers, DocLinks} from 'utils/constants'; +import {PermissionsScope, ModalIdentifiers, DocLinks, ModeratedPermissions} from 'utils/constants'; import TeamInList from './team_in_list'; @@ -500,40 +500,37 @@ class PermissionTeamSchemeSettings extends React.PureComponent { const roles = {...this.getStateRoles()} as RolesMap; - let role = null; - if (roles.team_admin.name === roleId) { - role = {...roles.team_admin}; - } else if (roles.channel_admin.name === roleId) { - role = {...roles.channel_admin}; - } else if (roles.all_users.name === roleId) { - role = {...roles.all_users}; - } else if (roles.guests.name === roleId) { - role = {...roles.guests}; - } else if (roles.playbook_admin.name === roleId) { - role = {...roles.playbook_admin}; + const role = {...roles[roleId]} as Role; + const newPermissions = [...role.permissions]; + for (const permission of permissions) { + if (newPermissions.indexOf(permission) === -1) { + newPermissions.push(permission); + } else { + newPermissions.splice(newPermissions.indexOf(permission), 1); + } } + role.permissions = newPermissions; + roles[roleId] = role; - if (role) { - const newPermissions = [...role.permissions]; + if (roleId === 'all_users') { + const channelAdminRole = {...roles.channel_admin} as Role; + const channelAdminPermissions = [...channelAdminRole.permissions!]; + const teamAdminRole = {...roles.team_admin} as Role; + const teamAdminPermissions = [...teamAdminRole.permissions!]; for (const permission of permissions) { - if (newPermissions.indexOf(permission) === -1) { - newPermissions.push(permission); - } else { - newPermissions.splice(newPermissions.indexOf(permission), 1); + if (ModeratedPermissions.indexOf(permission) !== -1 && role.permissions.indexOf(permission) !== -1) { + if (channelAdminPermissions.indexOf(permission) === -1) { + channelAdminPermissions.push(permission); + } + if (teamAdminPermissions.indexOf(permission) === -1) { + teamAdminPermissions.push(permission); + } } } - role.permissions = newPermissions; - if (roles.team_admin.name === roleId) { - roles.team_admin = role; - } else if (roles.channel_admin.name === roleId) { - roles.channel_admin = role; - } else if (roles.all_users.name === roleId) { - roles.all_users = role; - } else if (roles.guests.name === roleId) { - roles.guests = role; - } else if (roles.playbook_admin.name === roleId) { - roles.playbook_admin = role; - } + channelAdminRole.permissions = channelAdminPermissions; + roles.channel_admin = channelAdminRole; + teamAdminRole.permissions = teamAdminPermissions; + roles.team_admin = teamAdminRole; } this.setState({roles, saveNeeded: true}); diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index 614d4900d0..c751ac0783 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1391,6 +1391,26 @@ export const DefaultRolePermissions = { ], }; +// ModeratedPermissions are permissions that can be turned off for members and guests +// on a per channel basis. These permissions are on by default for team/channel admins. +export const ModeratedPermissions = [ + Permissions.CREATE_POST, + Permissions.UPLOAD_FILE, + Permissions.ADD_REACTION, + Permissions.REMOVE_REACTION, + Permissions.MANAGE_PUBLIC_CHANNEL_MEMBERS, + Permissions.MANAGE_PRIVATE_CHANNEL_MEMBERS, + Permissions.USE_CHANNEL_MENTIONS, + Permissions.ADD_BOOKMARK_PUBLIC_CHANNEL, + Permissions.EDIT_BOOKMARK_PUBLIC_CHANNEL, + Permissions.DELETE_BOOKMARK_PUBLIC_CHANNEL, + Permissions.ORDER_BOOKMARK_PUBLIC_CHANNEL, + Permissions.ADD_BOOKMARK_PRIVATE_CHANNEL, + Permissions.EDIT_BOOKMARK_PRIVATE_CHANNEL, + Permissions.DELETE_BOOKMARK_PRIVATE_CHANNEL, + Permissions.ORDER_BOOKMARK_PRIVATE_CHANNEL, +]; + export const Locations = { CENTER: 'CENTER' as const, RHS_ROOT: 'RHS_ROOT' as const,