MM-58548 Remove manage_team permissions from System Console Ancillary permissions (#27395)

* remove manage team permissions from sysconsole_write_user_management_chanels and sysconsole_write_user_management_groups

* update migrations, add unit tests

* run migrations-extract

* make updating ancillary permissions a post

* update file names

* add new api to doc, update body to just be []string

* revert moving ancillary permissions to post

* fix queries after final testing

* Update channel.go

* Update channel.go

* Update 000124_remove_manage_team_permission.up.sql

* Update 000124_remove_manage_team_permission.up.sql

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Scott Bishel 2024-07-09 08:21:29 -06:00 committed by GitHub
parent 485dc64c5f
commit 99881b819a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 108 additions and 5 deletions

View File

@ -416,7 +416,8 @@ func restoreChannel(c *Context, w http.ResponseWriter, r *http.Request) {
defer c.LogAuditRec(auditRec)
auditRec.AddEventPriorState(channel)
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), teamId, model.PermissionManageTeam) {
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), teamId, model.PermissionManageTeam) &&
!c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementChannels) {
c.SetPermissionError(model.PermissionManageTeam)
return
}

View File

@ -2293,6 +2293,30 @@ func TestRestoreChannel(t *testing.T) {
require.Error(t, err)
CheckForbiddenStatus(t, resp)
// Make BasicUser, a User Manager
oldRoles := th.BasicUser.Roles
// Because the permissions get set on initialization,
// remove the manage_team permission from the User Management Role
th.RemovePermissionFromRole(model.PermissionManageTeam.Id, model.SystemUserManagerRoleId)
th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId, false)
defer func() {
th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, oldRoles, false)
}()
th.App.Srv().InvalidateAllCaches()
th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
_, resp, err = th.Client.RestoreChannel(context.Background(), publicChannel1.Id)
require.NoError(t, err)
CheckOKStatus(t, resp)
_, resp, err = th.Client.RestoreChannel(context.Background(), privateChannel1.Id)
require.NoError(t, err)
CheckOKStatus(t, resp)
th.Client.DeleteChannel(context.Background(), publicChannel1.Id)
th.Client.DeleteChannel(context.Background(), privateChannel1.Id)
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
defer func() {
client.DeleteChannel(context.Background(), publicChannel1.Id)

View File

@ -646,7 +646,8 @@ func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType
switch syncableType {
case model.GroupSyncableTypeTeam:
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) {
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) &&
!c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementGroups) {
return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionInviteUser})
}
case model.GroupSyncableTypeChannel:
@ -662,7 +663,8 @@ func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType
var nfErr *store.ErrNotFound
switch {
case errors.As(appErr, &nfErr):
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) {
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) &&
!c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementGroups) {
return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionInviteUser})
}
default:

View File

@ -439,6 +439,14 @@ func TestLinkGroupTeam(t *testing.T) {
assert.NotNil(t, groupSyncable)
})
t.Run("System manager without invite_user are allowed to link", func(t *testing.T) {
th.SystemManagerClient.Login(context.Background(), th.SystemManagerUser.Email, th.SystemManagerUser.Password)
groupSyncable, response, err = th.SystemManagerClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
require.NoError(t, err)
CheckCreatedStatus(t, response)
assert.NotNil(t, groupSyncable)
})
t.Run("Custom groups can't be linked", func(t *testing.T) {
gid := model.NewId()
gCustom, appErr := th.App.CreateGroup(&model.Group{
@ -555,6 +563,14 @@ func TestLinkGroupChannel(t *testing.T) {
assert.NotNil(t, groupSyncable)
})
t.Run("System manager without invite_user are allowed to link", func(t *testing.T) {
th.SystemManagerClient.Login(context.Background(), th.SystemManagerUser.Email, th.SystemManagerUser.Password)
groupSyncable, response, err = th.SystemManagerClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
require.NoError(t, err)
CheckCreatedStatus(t, response)
assert.NotNil(t, groupSyncable)
})
t.Run("Custom groups can't be linked", func(t *testing.T) {
gid := model.NewId()
g2, appErr := th.App.CreateGroup(&model.Group{
@ -667,6 +683,13 @@ func TestUnlinkGroupTeam(t *testing.T) {
CheckOKStatus(t, response)
})
t.Run("System manager without invite_user are allowed to link", func(t *testing.T) {
th.SystemManagerClient.Login(context.Background(), th.SystemManagerUser.Email, th.SystemManagerUser.Password)
response, err = th.SystemManagerClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
require.NoError(t, err)
CheckOKStatus(t, response)
})
t.Run("Custom groups can't get unlinked", func(t *testing.T) {
gid := model.NewId()
g2, appErr := th.App.CreateGroup(&model.Group{
@ -778,6 +801,13 @@ func TestUnlinkGroupChannel(t *testing.T) {
CheckOKStatus(t, response)
})
t.Run("System manager without invite_user are allowed to link", func(t *testing.T) {
th.SystemManagerClient.Login(context.Background(), th.SystemManagerUser.Email, th.SystemManagerUser.Password)
response, err = th.SystemManagerClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
require.NoError(t, err)
CheckOKStatus(t, response)
})
t.Run("Custom groups can't get unlinked", func(t *testing.T) {
gid := model.NewId()
g2, appErr := th.App.CreateGroup(&model.Group{

View File

@ -244,6 +244,8 @@ channels/db/migrations/mysql/000122_preferences_value_length.down.sql
channels/db/migrations/mysql/000122_preferences_value_length.up.sql
channels/db/migrations/mysql/000123_remove_upload_file_permission.down.sql
channels/db/migrations/mysql/000123_remove_upload_file_permission.up.sql
channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql
channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql
channels/db/migrations/postgres/000001_create_teams.down.sql
channels/db/migrations/postgres/000001_create_teams.up.sql
channels/db/migrations/postgres/000002_create_team_members.down.sql
@ -488,3 +490,5 @@ channels/db/migrations/postgres/000122_preferences_value_length.down.sql
channels/db/migrations/postgres/000122_preferences_value_length.up.sql
channels/db/migrations/postgres/000123_remove_upload_file_permission.down.sql
channels/db/migrations/postgres/000123_remove_upload_file_permission.up.sql
channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql
channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql

View File

@ -0,0 +1 @@
-- Skipping it because the forward migrations are destructive

View File

@ -0,0 +1,21 @@
CREATE PROCEDURE RemoveUploadFilePermission()
BEGIN
updateRoles: LOOP
-- update affected rows
UPDATE Roles
SET Permissions = REGEXP_REPLACE(Permissions, 'manage_team[[:space:]|?]', '')
WHERE Permissions REGEXP 'manage_team[[:space:]|?]'
AND Permissions not like '%sysconsole_write_user_management_teams%'
AND (Permissions like '%sysconsole_write_user_management_channels%'
OR Permissions like '%sysconsole_write_user_management_groups%')
LIMIT 100;
-- check if the loop has completed
IF ROW_COUNT() < 100 THEN
LEAVE updateRoles;
END IF;
END LOOP updateRoles;
END;
CALL RemoveUploadFilePermission();
DROP PROCEDURE IF EXISTS RemoveUploadFilePermission;

View File

@ -0,0 +1 @@
-- Skipping it because the forward migrations are destructive

View File

@ -0,0 +1,21 @@
DO $$
<<remove_manage_team_permission>>
DECLARE
rows_updated integer;
BEGIN
LOOP
WITH table_holder AS (
SELECT id FROM roles
WHERE Permissions ~ 'manage_team($|\s)'
AND Permissions !~~ '%sysconsole_write_user_management_teams%'
AND (Permissions ~~ '%sysconsole_write_user_management_channels%'
OR Permissions ~~ '%sysconsole_write_user_management_groups%')
ORDER BY id ASC limit 100
)
UPDATE Roles r set permissions = REGEXP_REPLACE(permissions, 'manage_team($|\s)', '')
WHERE r.id in (SELECT id FROM table_holder);
GET DIAGNOSTICS rows_updated = ROW_COUNT;
EXIT WHEN rows_updated < 100;
END LOOP;
END remove_manage_team_permission $$;

View File

@ -116,7 +116,6 @@ func init() {
PermissionPromoteGuest,
},
PermissionSysconsoleWriteUserManagementChannels.Id: {
PermissionManageTeam,
PermissionManagePublicChannelProperties,
PermissionManagePrivateChannelProperties,
PermissionManagePrivateChannelMembers,
@ -136,7 +135,6 @@ func init() {
PermissionAddUserToTeam,
},
PermissionSysconsoleWriteUserManagementGroups.Id: {
PermissionManageTeam,
PermissionManagePrivateChannelMembers,
PermissionManagePublicChannelMembers,
PermissionConvertPublicChannelToPrivate,