diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index ee42d06f2d..7335682031 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -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 } diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 8c5dfbe364..e163886b53 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -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) diff --git a/server/channels/api4/group.go b/server/channels/api4/group.go index 58ddc507ca..be558fcee0 100644 --- a/server/channels/api4/group.go +++ b/server/channels/api4/group.go @@ -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: diff --git a/server/channels/api4/group_test.go b/server/channels/api4/group_test.go index 411c91fda6..0abcfa793f 100644 --- a/server/channels/api4/group_test.go +++ b/server/channels/api4/group_test.go @@ -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{ diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 5a7f9f9e30..1566f48a74 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -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 diff --git a/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql b/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql new file mode 100644 index 0000000000..7e9ccde63d --- /dev/null +++ b/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migrations are destructive \ No newline at end of file diff --git a/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql b/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql new file mode 100644 index 0000000000..7a66afe483 --- /dev/null +++ b/server/channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql @@ -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; diff --git a/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql b/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql new file mode 100644 index 0000000000..7e9ccde63d --- /dev/null +++ b/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migrations are destructive \ No newline at end of file diff --git a/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql b/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql new file mode 100644 index 0000000000..bc6fa7f48a --- /dev/null +++ b/server/channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql @@ -0,0 +1,21 @@ +DO $$ +<> +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 $$; diff --git a/server/public/model/role.go b/server/public/model/role.go index 759e87c8d6..2cbd51316c 100644 --- a/server/public/model/role.go +++ b/server/public/model/role.go @@ -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,