From a0f75cadb2b0fa91f6e321b1b8b859519b2ec865 Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Mon, 1 Oct 2018 13:51:12 +0100 Subject: MM-12110: Don't /invite or /kick deactivated users. (#9494) --- app/command_invite.go | 14 ++++++++---- app/command_invite_test.go | 8 +++++++ app/command_remove.go | 57 ++++++++++++++++++++++++++++++++++++---------- app/command_remove_test.go | 15 ++++++++++++ i18n/en.json | 8 +++++-- 5 files changed, 84 insertions(+), 18 deletions(-) diff --git a/app/command_invite.go b/app/command_invite.go index 1f6006018..c4f197374 100644 --- a/app/command_invite.go +++ b/app/command_invite.go @@ -49,15 +49,21 @@ func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message str targetUsername := splitMessage[0] targetUsername = strings.TrimPrefix(targetUsername, "@") - var userProfile *model.User - if result := <-a.Srv.Store.User().GetByUsername(targetUsername); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(targetUsername) + if result.Err != nil { mlog.Error(result.Err.Error()) return &model.CommandResponse{ Text: args.T("api.command_invite.missing_user.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, } - } else { - userProfile = result.Data.(*model.User) + } + + userProfile := result.Data.(*model.User) + if userProfile.DeleteAt != 0 { + return &model.CommandResponse{ + Text: args.T("api.command_invite.missing_user.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } var channelToJoin *model.Channel diff --git a/app/command_invite_test.go b/app/command_invite_test.go index f141cb239..b48d3e13e 100644 --- a/app/command_invite_test.go +++ b/app/command_invite_test.go @@ -23,6 +23,8 @@ func TestInviteProvider(t *testing.T) { basicUser3 := th.CreateUser() th.LinkUserToTeam(basicUser3, th.BasicTeam) basicUser4 := th.CreateUser() + deactivatedUser := th.CreateUser() + th.App.UpdateActive(deactivatedUser, false) InviteP := InviteProvider{} args := &model.CommandArgs{ @@ -38,6 +40,7 @@ func TestInviteProvider(t *testing.T) { userAndPrivateChannel := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name userAndDMChannel := "@" + basicUser3.Username + " ~" + dmChannel.Name userAndInvalidPrivate := "@" + basicUser3.Username + " ~" + privateChannel2.Name + deactivatedUserPublicChannel := "@" + deactivatedUser.Username + " ~" + channel.Name tests := []struct { desc string @@ -99,6 +102,11 @@ func TestInviteProvider(t *testing.T) { expected: "api.command_invite.private_channel.app_error", msg: userAndInvalidPrivate, }, + { + desc: "try to add a deleted user to a public channel", + expected: "api.command_invite.missing_user.app_error", + msg: deactivatedUserPublicChannel, + }, } for _, test := range tests { diff --git a/app/command_remove.go b/app/command_remove.go index 6a67996e9..f79ba5b51 100644 --- a/app/command_remove.go +++ b/app/command_remove.go @@ -67,24 +67,39 @@ func (me *KickProvider) DoCommand(a *App, args *model.CommandArgs, message strin func doCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse { channel, err := a.GetChannel(args.ChannelId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_remove.channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } switch channel.Type { case model.CHANNEL_OPEN: if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_remove.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } case model.CHANNEL_PRIVATE: if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_remove.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } default: - return &model.CommandResponse{Text: args.T("api.command_remove.direct_group.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_remove.direct_group.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if len(message) == 0 { - return &model.CommandResponse{Text: args.T("api.command_remove.message.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_remove.message.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } targetUsername := "" @@ -92,22 +107,40 @@ func doCommand(a *App, args *model.CommandArgs, message string) *model.CommandRe targetUsername = strings.SplitN(message, " ", 2)[0] targetUsername = strings.TrimPrefix(targetUsername, "@") - var userProfile *model.User - if result := <-a.Srv.Store.User().GetByUsername(targetUsername); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(targetUsername) + if result.Err != nil { mlog.Error(result.Err.Error()) - return &model.CommandResponse{Text: args.T("api.command_remove.missing.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } else { - userProfile = result.Data.(*model.User) + return &model.CommandResponse{ + Text: args.T("api.command_remove.missing.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + userProfile := result.Data.(*model.User) + if userProfile.DeleteAt != 0 { + return &model.CommandResponse{ + Text: args.T("api.command_remove.missing.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } _, err = a.GetChannelMember(args.ChannelId, userProfile.Id) if err != nil { nameFormat := *a.Config().TeamSettings.TeammateNameDisplay - return &model.CommandResponse{Text: args.T("api.command_remove.user_not_in_channel", map[string]interface{}{"Username": userProfile.GetDisplayName(nameFormat)}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_remove.user_not_in_channel", map[string]interface{}{ + "Username": userProfile.GetDisplayName(nameFormat), + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if err = a.RemoveUserFromChannel(userProfile.Id, args.UserId, channel); err != nil { - return &model.CommandResponse{Text: args.T(err.Id, map[string]interface{}{"Channel": model.DEFAULT_CHANNEL}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T(err.Id, map[string]interface{}{ + "Channel": model.DEFAULT_CHANNEL, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } return &model.CommandResponse{} diff --git a/app/command_remove_test.go b/app/command_remove_test.go index f17a70bad..66257f563 100644 --- a/app/command_remove_test.go +++ b/app/command_remove_test.go @@ -106,4 +106,19 @@ func TestRemoveProviderDoCommand(t *testing.T) { actual = rp.DoCommand(th.App, args, user1.Username).Text assert.Equal(t, "api.command_remove.direct_group.app_error", actual) + + // Try a public channel with a deactivated user. + deactivatedUser := th.CreateUser() + th.App.AddUserToTeam(th.BasicTeam.Id, deactivatedUser.Id, deactivatedUser.Id) + th.App.AddUserToChannel(deactivatedUser, publicChannel) + th.App.UpdateActive(deactivatedUser, false) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: publicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, deactivatedUser.Username).Text + assert.Equal(t, "api.command_remove.missing.app_error", actual) } diff --git a/i18n/en.json b/i18n/en.json index eaa6d1014..469e6976d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -411,6 +411,10 @@ "id": "api.command_channel_purpose.update_channel.app_error", "translation": "Error to update the current channel." }, + { + "id": "api.command_channel_remove.channel.app_error", + "translation": "Error retrieving the current channel." + }, { "id": "api.command_channel_rename.channel.app_error", "translation": "Error to retrieve the current channel." @@ -616,7 +620,7 @@ }, { "id": "api.command_invite.missing_user.app_error", - "translation": "Unable to find the user." + "translation": "We couldn't find the user. They may have been deactivated by the System Administrator." }, { "id": "api.command_invite.name", @@ -824,7 +828,7 @@ }, { "id": "api.command_remove.missing.app_error", - "translation": "Unable to find the user" + "translation": "We couldn't find the user. They may have been deactivated by the System Administrator." }, { "id": "api.command_remove.name", -- cgit v1.2.3-1-g7c22