diff --git a/api4/channel.go b/api4/channel.go index 319ba0d73c..32c4efad9f 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -45,6 +45,7 @@ func (api *API) InitChannel() { api.BaseRoutes.Channel.Handle("/pinned", api.ApiSessionRequired(getPinnedPosts)).Methods("GET") api.BaseRoutes.Channel.Handle("/timezones", api.ApiSessionRequired(getChannelMembersTimezones)).Methods("GET") api.BaseRoutes.Channel.Handle("/members_minus_group_members", api.ApiSessionRequired(channelMembersMinusGroupMembers)).Methods("GET") + api.BaseRoutes.Channel.Handle("/move", api.ApiSessionRequired(moveChannel)).Methods("POST") api.BaseRoutes.Channel.Handle("/member_counts_by_group", api.ApiSessionRequired(channelMemberCountsByGroup)).Methods("GET") api.BaseRoutes.ChannelForUser.Handle("/unread", api.ApiSessionRequired(getChannelUnread)).Methods("GET") @@ -1753,3 +1754,70 @@ func patchChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) auditRec.Success() w.Write(b) } + +func moveChannel(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + + channel, err := c.App.GetChannel(c.Params.ChannelId) + if err != nil { + c.Err = err + return + } + + props := model.StringInterfaceFromJson(r.Body) + teamId, ok := props["team_id"].(string) + if !ok { + c.SetInvalidParam("team_id") + return + } + + team, err := c.App.GetTeam(teamId) + if err != nil { + c.Err = err + return + } + + auditRec := c.MakeAuditRecord("moveChannel", audit.Fail) + defer c.LogAuditRec(auditRec) + auditRec.AddMeta("channel_id", channel.Id) + auditRec.AddMeta("channel_name", channel.Name) + auditRec.AddMeta("team_id", team.Id) + auditRec.AddMeta("team_name", team.Name) + + if channel.Type == model.CHANNEL_DIRECT || channel.Type == model.CHANNEL_GROUP || channel.Type == model.CHANNEL_PRIVATE { + c.Err = model.NewAppError("moveChannel", "api.channel.move_channel.type.invalid", nil, "", http.StatusForbidden) + return + } + + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) { + c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + return + } + + user, err := c.App.GetUser(c.App.Session().UserId) + if err != nil { + c.Err = err + return + } + + err = c.App.RemoveAllDeactivatedMembersFromChannel(channel) + if err != nil { + c.Err = err + return + } + + err = c.App.MoveChannel(team, channel, user) + if err != nil { + c.Err = err + return + } + + auditRec.Success() + c.LogAudit("channel=" + channel.Name) + c.LogAudit("team=" + team.Name) + + w.Write([]byte(channel.ToJson())) +} diff --git a/api4/channel_test.go b/api4/channel_test.go index 98ed91bbce..4bb674c316 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -3790,3 +3790,67 @@ func TestGetChannelMemberCountsByGroup(t *testing.T) { require.ElementsMatch(t, expectedMemberCounts, memberCounts) }) } + +func TestMoveChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + Client := th.Client + team1 := th.BasicTeam + team2 := th.CreateTeam() + + t.Run("Should move channel", func(t *testing.T) { + publicChannel := th.CreatePublicChannel() + ch, resp := th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id) + 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) + require.NotNil(t, resp.Error) + CheckErrorMessage(t, resp, "api.channel.move_channel.type.invalid") + }) + + 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) + require.NotNil(t, resp.Error) + CheckErrorMessage(t, resp, "api.channel.move_channel.type.invalid") + }) + + t.Run("Should fail when trying to move a group channel", func(t *testing.T) { + user := th.CreateUser() + + 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) + 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) + require.NotNil(t, resp.Error) + CheckErrorMessage(t, resp, "api.context.permissions.app_error") + }) + + t.Run("Should fail to move channel due to a member 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) + + _, resp = th.SystemAdminClient.MoveChannel(publicChannel.Id, team2.Id) + require.NotNil(t, resp.Error) + CheckErrorMessage(t, resp, "app.channel.move_channel.members_do_not_match.error") + }) + +} diff --git a/app/app_iface.go b/app/app_iface.go index bd31d38c6e..421ba8c156 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -222,6 +222,9 @@ type AppIface interface { MakeAuditRecord(event string, initialStatus string) *audit.Record // MarkChanelAsUnreadFromPost will take a post and set the channel as unread from that one. MarkChannelAsUnreadFromPost(postID string, userID string) (*model.ChannelUnreadAt, *model.AppError) + // MoveChannel method is prone to data races if someone joins to channel during the move process. However this + // function is only exposed to sysadmins and the possibility of this edge case is realtively small. + MoveChannel(team *model.Team, channel *model.Channel, user *model.User) *model.AppError // NewWebConn returns a new WebConn instance. NewWebConn(ws *websocket.Conn, session model.Session, t goi18n.TranslateFunc, locale string) *WebConn // NewWebHub creates a new Hub. @@ -292,9 +295,6 @@ type AppIface interface { // The result can be used, for example, to determine the set of users who would be removed from a team if the team // were group-constrained with the given groups. TeamMembersMinusGroupMembers(teamID string, groupIDs []string, page, perPage int) ([]*model.UserWithGroups, int64, *model.AppError) - // This function is intended for use from the CLI. It is not robust against people joining the channel while the move - // is in progress, and therefore should not be used from the API without first fixing this potential race condition. - MoveChannel(team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError // This function migrates the default built in roles from code/config to the database. DoAdvancedPermissionsMigration() // This to be used for places we check the users password when they are already logged in @@ -788,6 +788,7 @@ type AppIface interface { RegenerateTeamInviteId(teamId string) (*model.Team, *model.AppError) RegisterPluginCommand(pluginId string, command *model.Command) error ReloadConfig() error + RemoveAllDeactivatedMembersFromChannel(channel *model.Channel) *model.AppError RemoveConfigListener(id string) RemoveFile(path string) *model.AppError RemovePlugin(id string) *model.AppError diff --git a/app/channel.go b/app/channel.go index 90f9209cfe..0c92f57d2b 100644 --- a/app/channel.go +++ b/app/channel.go @@ -2344,15 +2344,13 @@ func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError { return nil } -// This function is intended for use from the CLI. It is not robust against people joining the channel while the move -// is in progress, and therefore should not be used from the API without first fixing this potential race condition. -func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError { - if removeDeactivatedMembers { - if err := a.Srv().Store.Channel().RemoveAllDeactivatedMembers(channel.Id); err != nil { - return err - } - } +func (a *App) RemoveAllDeactivatedMembersFromChannel(channel *model.Channel) *model.AppError { + return a.Srv().Store.Channel().RemoveAllDeactivatedMembers(channel.Id) +} +// MoveChannel method is prone to data races if someone joins to channel during the move process. However this +// function is only exposed to sysadmins and the possibility of this edge case is realtively small. +func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.User) *model.AppError { // Check that all channel members are in the destination team. channelMembers, err := a.GetChannelMembersPage(channel.Id, 0, 10000000) if err != nil { @@ -2394,7 +2392,40 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model. return model.NewAppError("MoveChannel", "app.channel.update_channel.internal_error", nil, err.Error(), http.StatusInternalServerError) } } - a.postChannelMoveMessage(user, channel, previousTeam) + + if incomingWebhooks, err := a.GetIncomingWebhooksForTeamPage(previousTeam.Id, 0, 10000000); err != nil { + mlog.Warn("Failed to get incoming webhooks", mlog.Err(err)) + } else { + for _, webhook := range incomingWebhooks { + if webhook.ChannelId == channel.Id { + webhook.TeamId = team.Id + if _, err := a.Srv().Store.Webhook().UpdateIncoming(webhook); err != nil { + mlog.Warn("Failed to move incoming webhook to new team", mlog.String("webhook id", webhook.Id)) + } + } + } + } + + if outgoingWebhooks, err := a.GetOutgoingWebhooksForTeamPage(previousTeam.Id, 0, 10000000); err != nil { + mlog.Warn("Failed to get outgoing webhooks", mlog.Err(err)) + } else { + for _, webhook := range outgoingWebhooks { + if webhook.ChannelId == channel.Id { + webhook.TeamId = team.Id + if _, err := a.Srv().Store.Webhook().UpdateOutgoing(webhook); err != nil { + mlog.Warn("Failed to move outgoing webhook to new team.", mlog.String("webhook id", webhook.Id)) + } + } + } + } + + if err := a.removeUsersFromChannelNotMemberOfTeam(user, channel, team); err != nil { + mlog.Warn("error while removing non-team member users", mlog.Err(err)) + } + + if err := a.postChannelMoveMessage(user, channel, previousTeam); err != nil { + mlog.Warn("error while posting move channel message", mlog.Err(err)) + } return nil } @@ -2418,6 +2449,40 @@ 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 { + channelMembers, err := a.GetChannelMembersPage(channel.Id, 0, 10000000) + if err != nil { + return err + } + + channelMemberIds := []string{} + channelMemberMap := make(map[string]struct{}) + for _, channelMember := range *channelMembers { + channelMemberMap[channelMember.UserId] = struct{}{} + channelMemberIds = append(channelMemberIds, channelMember.UserId) + } + + if len(channelMemberIds) > 0 { + teamMembers, err := a.GetTeamMembersByIds(team.Id, channelMemberIds, nil) + if err != nil { + return err + } + + if len(teamMembers) != len(*channelMembers) { + for _, teamMember := range teamMembers { + delete(channelMemberMap, teamMember.UserId) + } + for userId := range channelMemberMap { + if err := a.removeUserFromChannel(userId, remover.Id, channel); err != nil { + return err + } + } + } + } + + return nil +} + func (a *App) GetPinnedPosts(channelId string) (*model.PostList, *model.AppError) { return a.Srv().Store.Channel().GetPinnedPosts(channelId) } diff --git a/app/channel_test.go b/app/channel_test.go index 31f095d6b0..f09be78251 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -68,6 +68,40 @@ func TestPermanentDeleteChannel(t *testing.T) { require.NotNil(t, err, "Outgoing webhook wasn't deleted") } +func TestRemoveAllDeactivatedMembersFromChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + var err *model.AppError + + team := th.CreateTeam() + channel := th.CreateChannel(team) + defer func() { + th.App.PermanentDeleteChannel(channel) + th.App.PermanentDeleteTeam(team) + }() + + _, err = th.App.AddUserToTeam(team.Id, th.BasicUser.Id, "") + require.Nil(t, err) + + deacivatedUser := th.CreateUser() + _, err = th.App.AddUserToTeam(team.Id, deacivatedUser.Id, "") + require.Nil(t, err) + _, err = th.App.AddUserToChannel(deacivatedUser, channel) + require.Nil(t, err) + channelMembers, err := th.App.GetChannelMembersPage(channel.Id, 0, 10000000) + require.Nil(t, err) + require.Len(t, *channelMembers, 2) + _, err = th.App.UpdateActive(deacivatedUser, false) + require.Nil(t, err) + + err = th.App.RemoveAllDeactivatedMembersFromChannel(channel) + require.Nil(t, err) + + channelMembers, err = th.App.GetChannelMembersPage(channel.Id, 0, 10000000) + require.Nil(t, err) + require.Len(t, *channelMembers, 1) +} + func TestMoveChannel(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -97,13 +131,13 @@ func TestMoveChannel(t *testing.T) { _, err = th.App.AddUserToChannel(th.BasicUser2, channel1) require.Nil(t, err) - err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false) + err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser) require.NotNil(t, err, "Should have failed due to mismatched members.") _, err = th.App.AddUserToTeam(targetTeam.Id, th.BasicUser2.Id, "") require.Nil(t, err) - err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false) + err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser) require.Nil(t, err) // Test moving a channel with a deactivated user who isn't in the destination team. @@ -123,12 +157,9 @@ func TestMoveChannel(t *testing.T) { _, err = th.App.UpdateActive(deacivatedUser, false) require.Nil(t, err) - err = th.App.MoveChannel(targetTeam, channel2, th.BasicUser, false) + err = th.App.MoveChannel(targetTeam, channel2, th.BasicUser) require.NotNil(t, err, "Should have failed due to mismatched deacivated member.") - err = th.App.MoveChannel(targetTeam, channel2, th.BasicUser, true) - require.Nil(t, err) - // Test moving a channel with no members. channel3 := &model.Channel{ DisplayName: "dn_" + model.NewId(), @@ -142,7 +173,7 @@ func TestMoveChannel(t *testing.T) { require.Nil(t, err) defer th.App.PermanentDeleteChannel(channel3) - err = th.App.MoveChannel(targetTeam, channel3, th.BasicUser, false) + err = th.App.MoveChannel(targetTeam, channel3, th.BasicUser) assert.Nil(t, err) } diff --git a/app/opentracing_layer.go b/app/opentracing_layer.go index 40433bd119..56c31bd17f 100644 --- a/app/opentracing_layer.go +++ b/app/opentracing_layer.go @@ -9966,7 +9966,7 @@ func (a *OpenTracingAppLayer) MigrateFilenamesToFileInfos(post *model.Post) []*m return resultVar0 } -func (a *OpenTracingAppLayer) MoveChannel(team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError { +func (a *OpenTracingAppLayer) MoveChannel(team *model.Team, channel *model.Channel, user *model.User) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.MoveChannel") @@ -9978,7 +9978,7 @@ func (a *OpenTracingAppLayer) MoveChannel(team *model.Team, channel *model.Chann }() defer span.Finish() - resultVar0 := a.app.MoveChannel(team, channel, user, removeDeactivatedMembers) + resultVar0 := a.app.MoveChannel(team, channel, user) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) @@ -11029,6 +11029,28 @@ func (a *OpenTracingAppLayer) ReloadConfig() error { return resultVar0 } +func (a *OpenTracingAppLayer) RemoveAllDeactivatedMembersFromChannel(channel *model.Channel) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveAllDeactivatedMembersFromChannel") + + 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.RemoveAllDeactivatedMembersFromChannel(channel) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) RemoveConfigListener(id string) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveConfigListener") diff --git a/cmd/mattermost/commands/channel.go b/cmd/mattermost/commands/channel.go index b36dcde73d..1049a4ee3b 100644 --- a/cmd/mattermost/commands/channel.go +++ b/cmd/mattermost/commands/channel.go @@ -138,7 +138,6 @@ func init() { ChannelCreateCmd.Flags().Bool("private", false, "Create a private channel.") MoveChannelsCmd.Flags().String("username", "", "Required. Username who is moving the channel.") - MoveChannelsCmd.Flags().Bool("remove-deactivated-users", false, "Automatically remove any deactivated users from the channel before moving it.") DeleteChannelsCmd.Flags().Bool("confirm", false, "Confirm you really want to delete the channels.") @@ -404,8 +403,6 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error { } user := getUserFromUserArg(a, username) - removeDeactivatedMembers, _ := command.Flags().GetBool("remove-deactivated-users") - channels := getChannelsFromChannelArgs(a, args[1:]) for i, channel := range channels { if channel == nil { @@ -413,7 +410,7 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error { continue } originTeamID := channel.TeamId - if err := moveChannel(a, team, channel, user, removeDeactivatedMembers); err != nil { + if err := moveChannel(a, team, channel, user); err != nil { CommandPrintErrorln("Unable to move channel '" + channel.Name + "' error: " + err.Error()) } else { CommandPrettyPrintln("Moved channel '" + channel.Name + "' to " + team.Name + "(" + team.Id + ") from " + originTeamID + ".") @@ -422,10 +419,14 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error { return nil } -func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError { +func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *model.User) *model.AppError { oldTeamId := channel.TeamId - if err := a.MoveChannel(team, channel, user, removeDeactivatedMembers); err != nil { + if err := a.RemoveAllDeactivatedMembersFromChannel(channel); err != nil { + return err + } + + if err := a.MoveChannel(team, channel, user); err != nil { return err } diff --git a/i18n/en.json b/i18n/en.json index 5ffa61932d..f03837f0f1 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -331,6 +331,10 @@ "id": "api.channel.leave.left", "translation": "%v left the channel." }, + { + "id": "api.channel.move_channel.type.invalid", + "translation": "Unable to move direct or group message channels" + }, { "id": "api.channel.patch_channel_moderations.license.error", "translation": "Your license does not support channel moderation" diff --git a/model/client4.go b/model/client4.go index 127a227d27..b937323d8d 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2456,6 +2456,19 @@ func (c *Client4) DeleteChannel(channelId string) (bool, *Response) { return CheckStatusOK(r), BuildResponse(r) } +// MoveChannel moves the channel to the destination team. +func (c *Client4) MoveChannel(channelId, teamId string) (*Channel, *Response) { + requestBody := map[string]string{ + "team_id": teamId, + } + r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/move", MapToJson(requestBody)) + if err != nil { + return nil, BuildErrorResponse(r, err) + } + defer closeBody(r) + return ChannelFromJson(r.Body), BuildResponse(r) +} + // GetChannelByName returns a channel based on the provided channel name and team id strings. func (c *Client4) GetChannelByName(channelName, teamId string, etag string) (*Channel, *Response) { r, err := c.DoApiGet(c.GetChannelByNameRoute(channelName, teamId), etag)