[MM-21464] api4/channel: add ability to force move channel by removing non-members (#14887)

* api4/channel: add ability to force move channel by removing non-members

* app/channel: add log to move channel for non taget team members

* app/channel: add tets for remove non-team members for channel

* Update api4/channel_test.go

Co-authored-by: Eli Yukelzon <reflog@gmail.com>

Co-authored-by: Eli Yukelzon <reflog@gmail.com>
Этот коммит содержится в:
Ibrahim Serdar Acikgoz
2020-07-16 11:26:19 +03:00
коммит произвёл GitHub
родитель b916bae0f7
Коммит bf664997a7
7 изменённых файлов: 111 добавлений и 11 удалений

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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) {

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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()

Просмотреть файл

@@ -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")

Просмотреть файл

@@ -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)
}