diff --git a/api4/channel.go b/api4/channel.go index 628ede38e8..d4efc4c62b 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -1810,6 +1810,12 @@ func moveChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } + force, ok := props["force"].(bool) + if !ok { + c.SetInvalidParam("force") + return + } + team, err := c.App.GetTeam(teamId) if err != nil { c.Err = err @@ -1845,6 +1851,14 @@ func moveChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } + if force { + err = c.App.RemoveUsersFromChannelNotMemberOfTeam(user, channel, team) + if err != nil { + c.Err = err + return + } + } + err = c.App.MoveChannel(team, channel, user) if err != nil { c.Err = err diff --git a/api4/channel_test.go b/api4/channel_test.go index 131e7953fd..24ccb2bcc4 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -3839,14 +3839,14 @@ func TestMoveChannel(t *testing.T) { t.Run("Should move channel", func(t *testing.T) { publicChannel := th.CreatePublicChannel() - ch, resp := th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id) + ch, resp := th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id, false) require.Nil(t, resp.Error) require.Equal(t, team2.Id, ch.TeamId) }) t.Run("Should fail when trying to move a private channel", func(t *testing.T) { channel := th.CreatePrivateChannel() - _, resp := Client.MoveChannel(channel.Id, team1.Id) + _, resp := Client.MoveChannel(channel.Id, team1.Id, false) require.NotNil(t, resp.Error) CheckErrorMessage(t, resp, "api.channel.move_channel.type.invalid") }) @@ -3854,7 +3854,7 @@ func TestMoveChannel(t *testing.T) { t.Run("Should fail when trying to move a DM channel", func(t *testing.T) { user := th.CreateUser() dmChannel := th.CreateDmChannel(user) - _, resp := Client.MoveChannel(dmChannel.Id, team1.Id) + _, resp := Client.MoveChannel(dmChannel.Id, team1.Id, false) require.NotNil(t, resp.Error) CheckErrorMessage(t, resp, "api.channel.move_channel.type.invalid") }) @@ -3864,14 +3864,14 @@ func TestMoveChannel(t *testing.T) { gmChannel, err := th.App.CreateGroupChannel([]string{th.BasicUser.Id, th.SystemAdminUser.Id, th.TeamAdminUser.Id}, user.Id) require.Nil(t, err) - _, resp := Client.MoveChannel(gmChannel.Id, team1.Id) + _, resp := Client.MoveChannel(gmChannel.Id, team1.Id, false) require.NotNil(t, resp.Error) CheckErrorMessage(t, resp, "api.channel.move_channel.type.invalid") }) t.Run("Should fail due to permissions", func(t *testing.T) { publicChannel := th.CreatePublicChannel() - _, resp := Client.MoveChannel(publicChannel.Id, team1.Id) + _, resp := Client.MoveChannel(publicChannel.Id, team1.Id, false) require.NotNil(t, resp.Error) CheckErrorMessage(t, resp, "api.context.permissions.app_error") }) @@ -3886,10 +3886,25 @@ func TestMoveChannel(t *testing.T) { _, resp = th.SystemAdminClient.AddChannelMember(publicChannel.Id, user.Id) CheckNoError(t, resp) - _, resp = th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id) + _, resp = th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id, false) require.NotNil(t, resp.Error) CheckErrorMessage(t, resp, "app.channel.move_channel.members_do_not_match.error") }) + + t.Run("Should be able to (force) move channel by a member that is not member of target team", func(t *testing.T) { + publicChannel := th.CreatePublicChannel() + user := th.BasicUser + + _, resp := th.SystemAdminClient.RemoveTeamMember(team2.Id, user.Id) + CheckNoError(t, resp) + + _, resp = th.SystemAdminClient.AddChannelMember(publicChannel.Id, user.Id) + CheckNoError(t, resp) + + newChannel, resp := th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id, true) + require.Nil(t, resp.Error) + require.Equal(t, team2.Id, newChannel.TeamId) + }) } func TestUpdateCategoryForTeamForUser(t *testing.T) { diff --git a/app/app_iface.go b/app/app_iface.go index 35b7fbeeb0..d1b92bddd1 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -805,6 +805,7 @@ type AppIface interface { RemoveTeamMemberFromTeam(teamMember *model.TeamMember, requestorId string) *model.AppError RemoveUserFromChannel(userIdToRemove string, removerUserId string, channel *model.Channel) *model.AppError RemoveUserFromTeam(teamId string, userId string, requestorId string) *model.AppError + RemoveUsersFromChannelNotMemberOfTeam(remover *model.User, channel *model.Channel, team *model.Team) *model.AppError RequestId() string ResetPasswordFromToken(userSuppliedTokenString, newPassword string) *model.AppError ResetPermissionsSystem() *model.AppError diff --git a/app/channel.go b/app/channel.go index d8b02c97ce..d438006e4b 100644 --- a/app/channel.go +++ b/app/channel.go @@ -2373,6 +2373,15 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model. } if len(teamMembers) != len(*channelMembers) { + teamMembersMap := make(map[string]*model.TeamMember, len(teamMembers)) + for _, teamMember := range teamMembers { + teamMembersMap[teamMember.UserId] = teamMember + } + for _, channelMember := range *channelMembers { + if _, ok := teamMembersMap[channelMember.UserId]; !ok { + mlog.Warn("Not member of the target team", mlog.String("userId", channelMember.UserId)) + } + } return model.NewAppError("MoveChannel", "app.channel.move_channel.members_do_not_match.error", nil, "", http.StatusInternalServerError) } } @@ -2427,7 +2436,7 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model. } } - if err := a.removeUsersFromChannelNotMemberOfTeam(user, channel, team); err != nil { + if err := a.RemoveUsersFromChannelNotMemberOfTeam(user, channel, team); err != nil { mlog.Warn("error while removing non-team member users", mlog.Err(err)) } @@ -2457,7 +2466,7 @@ func (a *App) postChannelMoveMessage(user *model.User, channel *model.Channel, p return nil } -func (a *App) removeUsersFromChannelNotMemberOfTeam(remover *model.User, channel *model.Channel, team *model.Team) *model.AppError { +func (a *App) RemoveUsersFromChannelNotMemberOfTeam(remover *model.User, channel *model.Channel, team *model.Team) *model.AppError { channelMembers, err := a.GetChannelMembersPage(channel.Id, 0, 10000000) if err != nil { return err diff --git a/app/channel_test.go b/app/channel_test.go index 6cd1ab07dc..0452255ec4 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -177,6 +177,44 @@ func TestMoveChannel(t *testing.T) { assert.Nil(t, err) } +func TestRemoveUsersFromChannelNotMemberOfTeam(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + team := th.CreateTeam() + team2 := th.CreateTeam() + channel1 := th.CreateChannel(team) + defer func() { + th.App.PermanentDeleteChannel(channel1) + th.App.PermanentDeleteTeam(team) + th.App.PermanentDeleteTeam(team2) + }() + + _, err := th.App.AddUserToTeam(team.Id, th.BasicUser.Id, "") + require.Nil(t, err) + _, err = th.App.AddUserToTeam(team2.Id, th.BasicUser.Id, "") + require.Nil(t, err) + _, err = th.App.AddUserToTeam(team.Id, th.BasicUser2.Id, "") + require.Nil(t, err) + + _, err = th.App.AddUserToChannel(th.BasicUser, channel1) + require.Nil(t, err) + _, err = th.App.AddUserToChannel(th.BasicUser2, channel1) + require.Nil(t, err) + + err = th.App.RemoveUsersFromChannelNotMemberOfTeam(th.SystemAdminUser, channel1, team2) + require.Nil(t, err) + + channelMembers, err := th.App.GetChannelMembersPage(channel1.Id, 0, 10000000) + require.Nil(t, err) + require.Len(t, *channelMembers, 1) + members := make([]model.ChannelMember, len(*channelMembers)) + for i, m := range *channelMembers { + members[i] = m + } + require.Equal(t, members[0].UserId, th.BasicUser.Id) +} + func TestJoinDefaultChannelsCreatesChannelMemberHistoryRecordTownSquare(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 81079c69b4..f72a9dfbf5 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -11390,6 +11390,28 @@ func (a *OpenTracingAppLayer) RemoveUserFromTeam(teamId string, userId string, r return resultVar0 } +func (a *OpenTracingAppLayer) RemoveUsersFromChannelNotMemberOfTeam(remover *model.User, channel *model.Channel, team *model.Team) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveUsersFromChannelNotMemberOfTeam") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.RemoveUsersFromChannelNotMemberOfTeam(remover, channel, team) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) RenameChannel(channel *model.Channel, newChannelName string, newDisplayName string) (*model.Channel, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RenameChannel") diff --git a/model/client4.go b/model/client4.go index 1d1648d907..fe15fecbe6 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2503,11 +2503,12 @@ func (c *Client4) DeleteChannel(channelId string) (bool, *Response) { } // MoveChannel moves the channel to the destination team. -func (c *Client4) MoveChannel(channelId, teamId string) (*Channel, *Response) { - requestBody := map[string]string{ +func (c *Client4) MoveChannel(channelId, teamId string, force bool) (*Channel, *Response) { + requestBody := map[string]interface{}{ "team_id": teamId, + "force": force, } - r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/move", MapToJson(requestBody)) + r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/move", StringInterfaceToJson(requestBody)) if err != nil { return nil, BuildErrorResponse(r, err) }