From f92d3fa51827b9eb75f1b88ef24338b3e700533d Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Tue, 7 Apr 2020 08:05:03 -0400 Subject: [PATCH] =?UTF-8?q?MM-23876:=20Fix=20for=20patching=20channel=20mo?= =?UTF-8?q?derations=20with=20a=20null=20team=20schem=E2=80=A6=20(#14239)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MM-23876: Fix for patching channel moderations with a null team scheme channel guest role. * MM-23876: Tests the moderations response. --- api4/channel_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++ app/channel.go | 43 +++++++++++++++++++++-------- model/role.go | 4 +++ 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/api4/channel_test.go b/api4/channel_test.go index 3231fd6d88..2ae21aa341 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -3282,4 +3282,69 @@ func TestPatchChannelModerations(t *testing.T) { require.NotEqual(t, scheme.DeleteAt, int64(0)) }) + t.Run("Does not return an error if the team scheme has a blank DefaultChannelGuestRole field", func(t *testing.T) { + team := th.BasicTeam + scheme := th.SetupTeamScheme() + scheme.DefaultChannelGuestRole = "" + + mockStore := mocks.Store{} + mockSchemeStore := mocks.SchemeStore{} + mockSchemeStore.On("Get", mock.Anything).Return(scheme, nil) + mockSchemeStore.On("Save", mock.Anything).Return(scheme, nil) + mockSchemeStore.On("Delete", mock.Anything).Return(scheme, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(th.App.Srv().Store.Team()) + mockStore.On("Channel").Return(th.App.Srv().Store.Channel()) + mockStore.On("User").Return(th.App.Srv().Store.User()) + mockStore.On("Post").Return(th.App.Srv().Store.Post()) + mockStore.On("FileInfo").Return(th.App.Srv().Store.FileInfo()) + mockStore.On("Webhook").Return(th.App.Srv().Store.Webhook()) + mockStore.On("System").Return(th.App.Srv().Store.System()) + mockStore.On("License").Return(th.App.Srv().Store.License()) + mockStore.On("Role").Return(th.App.Srv().Store.Role()) + mockStore.On("Close").Return(nil) + th.App.Srv().Store = &mockStore + + team.SchemeId = &scheme.Id + _, err := th.App.UpdateTeamScheme(team) + require.Nil(t, err) + + moderations, res := th.SystemAdminClient.PatchChannelModerations(channel.Id, emptyPatch) + require.Nil(t, res.Error) + require.Equal(t, len(moderations), 4) + for _, moderation := range moderations { + if moderation.Name == "manage_members" { + require.Empty(t, moderation.Roles.Guests) + } else { + require.Equal(t, moderation.Roles.Guests.Value, false) + require.Equal(t, moderation.Roles.Guests.Enabled, false) + } + + require.Equal(t, moderation.Roles.Members.Value, true) + require.Equal(t, moderation.Roles.Members.Enabled, true) + } + + patch := []*model.ChannelModerationPatch{ + { + Name: &createPosts, + Roles: &model.ChannelModeratedRolesPatch{Members: model.NewBool(true)}, + }, + } + + moderations, res = th.SystemAdminClient.PatchChannelModerations(channel.Id, patch) + require.Nil(t, res.Error) + require.Equal(t, len(moderations), 4) + for _, moderation := range moderations { + if moderation.Name == "manage_members" { + require.Empty(t, moderation.Roles.Guests) + } else { + require.Equal(t, moderation.Roles.Guests.Value, false) + require.Equal(t, moderation.Roles.Guests.Enabled, false) + } + + require.Equal(t, moderation.Roles.Members.Value, true) + require.Equal(t, moderation.Roles.Members.Enabled, true) + } + }) + } diff --git a/app/channel.go b/app/channel.go index ad2305b019..d4745bc1a9 100644 --- a/app/channel.go +++ b/app/channel.go @@ -726,7 +726,7 @@ func (a *App) GetChannelModerationsForChannel(channel *model.Channel) ([]*model. } var higherScopedGuestRole *model.Role - if len(guestRoleName) > 0 { + if len(higherScopedGuestRoleName) > 0 { higherScopedGuestRole, err = a.GetRoleByName(higherScopedGuestRoleName) if err != nil { return nil, err @@ -738,19 +738,30 @@ func (a *App) GetChannelModerationsForChannel(channel *model.Channel) ([]*model. // PatchChannelModerationsForChannel Updates a channels scheme roles based on a given ChannelModerationPatch, if the permissions match the higher scoped role the scheme is deleted. func (a *App) PatchChannelModerationsForChannel(channel *model.Channel, channelModerationsPatch []*model.ChannelModerationPatch) ([]*model.ChannelModeration, *model.AppError) { - higherScopedGuestRoleName, higherScopedMemberRoleName, _, _ := a.GetTeamSchemeChannelRoles(channel.TeamId) + higherScopedGuestRoleName, higherScopedMemberRoleName, _, err := a.GetTeamSchemeChannelRoles(channel.TeamId) + if err != nil { + return nil, err + } + higherScopedMemberRole, err := a.GetRoleByName(higherScopedMemberRoleName) if err != nil { return nil, err } - higherScopedGuestRole, err := a.GetRoleByName(higherScopedGuestRoleName) - if err != nil { - return nil, err + var higherScopedGuestRole *model.Role + if len(higherScopedGuestRoleName) > 0 { + higherScopedGuestRole, err = a.GetRoleByName(higherScopedGuestRoleName) + if err != nil { + return nil, err + } } higherScopedMemberPermissions := higherScopedMemberRole.GetChannelModeratedPermissions(channel.Type) - higherScopedGuestPermissions := higherScopedGuestRole.GetChannelModeratedPermissions(channel.Type) + + var higherScopedGuestPermissions map[string]bool + if higherScopedGuestRole != nil { + higherScopedGuestPermissions = higherScopedGuestRole.GetChannelModeratedPermissions(channel.Type) + } for _, moderationPatch := range channelModerationsPatch { if moderationPatch.Roles.Members != nil && *moderationPatch.Roles.Members && !higherScopedMemberPermissions[*moderationPatch.Name] { @@ -772,19 +783,29 @@ func (a *App) PatchChannelModerationsForChannel(channel *model.Channel, channelM mlog.Info("Permission scheme created.", mlog.String("channel_id", channel.Id), mlog.String("channel_name", channel.Name)) } - guestRoleName, memberRoleName, _, _ := a.GetSchemeRolesForChannel(channel.Id) + guestRoleName, memberRoleName, _, err := a.GetSchemeRolesForChannel(channel.Id) + if err != nil { + return nil, err + } + memberRole, err := a.GetRoleByName(memberRoleName) if err != nil { return nil, err } - guestRole, err := a.GetRoleByName(guestRoleName) - if err != nil { - return nil, err + var guestRole *model.Role + if len(guestRoleName) > 0 { + guestRole, err = a.GetRoleByName(guestRoleName) + if err != nil { + return nil, err + } } memberRolePatch := memberRole.RolePatchFromChannelModerationsPatch(channelModerationsPatch, "members") - guestRolePatch := guestRole.RolePatchFromChannelModerationsPatch(channelModerationsPatch, "guests") + var guestRolePatch *model.RolePatch + if guestRole != nil { + guestRolePatch = guestRole.RolePatchFromChannelModerationsPatch(channelModerationsPatch, "guests") + } for _, channelModerationPatch := range channelModerationsPatch { permissionModified := *channelModerationPatch.Name diff --git a/model/role.go b/model/role.go index 08dce3c863..337db748f4 100644 --- a/model/role.go +++ b/model/role.go @@ -204,6 +204,10 @@ func PermissionsChangedByPatch(role *Role, patch *RolePatch) []string { func ChannelModeratedPermissionsChangedByPatch(role *Role, patch *RolePatch) []string { var result []string + if role == nil { + return result + } + if patch.Permissions == nil { return result }