From 0dbaa2d032ae42cbf39945df12efb20fc572b559 Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Fri, 11 May 2018 17:20:47 +0200 Subject: [MM-10458] Change system response to "Could not find the channel" - bug fix (#8738) * [MM-10458] Change system response to "Could not find the channel" when trying to invite user to private channel you can't see * add another check to check if user have permission to add another in pvt channel --- app/apptestlib.go | 23 +++++++++++++++++++++++ app/command_invite.go | 10 +++++++++- app/command_invite_test.go | 7 +++++++ 3 files changed, 39 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/apptestlib.go b/app/apptestlib.go index 626e932e8..b245ddabf 100644 --- a/app/apptestlib.go +++ b/app/apptestlib.go @@ -227,6 +227,29 @@ func (me *TestHelper) createChannel(team *model.Team, channelType string) *model return channel } +func (me *TestHelper) createChannelWithAnotherUser(team *model.Team, channelType, userId string) *model.Channel { + id := model.NewId() + + channel := &model.Channel{ + DisplayName: "dn_" + id, + Name: "name_" + id, + Type: channelType, + TeamId: team.Id, + CreatorId: userId, + } + + utils.DisableDebugLogForTest() + var err *model.AppError + if channel, err = me.App.CreateChannel(channel, true); err != nil { + mlog.Error(err.Error()) + + time.Sleep(time.Second) + panic(err) + } + utils.EnableDebugLogForTest() + return channel +} + func (me *TestHelper) CreateDmChannel(user *model.User) *model.Channel { utils.DisableDebugLogForTest() var err *model.AppError diff --git a/app/command_invite.go b/app/command_invite.go index 4b76c0c45..54cf2da02 100644 --- a/app/command_invite.go +++ b/app/command_invite.go @@ -79,10 +79,18 @@ func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message str return &model.CommandResponse{Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{"User": userProfile.Username, "Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } - if channelToJoin.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { + // Check if the user who wants to add another is trying to add in a pvt channel, but does not have permission + // but is in the channel + _, err = a.GetChannelMember(channelToJoin.Id, args.UserId) + if channelToJoin.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) && err == nil { return &model.CommandResponse{Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{"User": userProfile.Username, "Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } + // In this case just check if is a pvt channel and user has permission + if channelToJoin.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { + return &model.CommandResponse{Text: args.T("api.command_invite.private_channel.app_error", map[string]interface{}{"Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } + // Check if user is already in the channel _, err = a.GetChannelMember(channelToJoin.Id, userProfile.Id) if err == nil { diff --git a/app/command_invite_test.go b/app/command_invite_test.go index c46bc4628..0d1db4a07 100644 --- a/app/command_invite_test.go +++ b/app/command_invite_test.go @@ -18,6 +18,7 @@ func TestInviteProvider(t *testing.T) { channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN) privateChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE) dmChannel := th.CreateDmChannel(th.BasicUser2) + privateChannel2 := th.createChannelWithAnotherUser(th.BasicTeam, model.CHANNEL_PRIVATE, th.BasicUser2.Id) basicUser3 := th.CreateUser() th.LinkUserToTeam(basicUser3, th.BasicTeam) @@ -36,6 +37,7 @@ func TestInviteProvider(t *testing.T) { userAndDisplayChannel := "@" + th.BasicUser2.Username + " ~" + channel.DisplayName + " " userAndPrivateChannel := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name userAndDMChannel := "@" + basicUser3.Username + " ~" + dmChannel.Name + userAndInvalidPrivate := "@" + basicUser3.Username + " ~" + privateChannel2.Name tests := []struct { desc string @@ -97,6 +99,11 @@ func TestInviteProvider(t *testing.T) { expected: "api.command_invite.directchannel.app_error", msg: userAndDMChannel, }, + { + desc: "try to add a user to a privante channel with no permission", + expected: "api.command_invite.private_channel.app_error", + msg: userAndInvalidPrivate, + }, } for _, test := range tests { -- cgit v1.2.3-1-g7c22 From 91c998156336e34ab4b8979db77cc65c97a65782 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Fri, 11 May 2018 11:56:02 -0400 Subject: More potential panic fixes (#8776) --- app/web_hub.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'app') diff --git a/app/web_hub.go b/app/web_hub.go index f69645f50..2ce78b5ef 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -177,8 +177,9 @@ func (a *App) Publish(message *model.WebSocketEvent) { func (a *App) PublishSkipClusterSend(message *model.WebSocketEvent) { if message.Broadcast.UserId != "" { - if len(a.Hubs) != 0 { - a.GetHubForUserId(message.Broadcast.UserId).Broadcast(message) + hub := a.GetHubForUserId(message.Broadcast.UserId) + if hub != nil { + hub.Broadcast(message) } } else { for _, hub := range a.Hubs { @@ -299,8 +300,9 @@ func (a *App) InvalidateCacheForUserSkipClusterSend(userId string) { a.Srv.Store.User().InvalidateProfilesInChannelCacheByUser(userId) a.Srv.Store.User().InvalidatProfileCacheForUser(userId) - if len(a.Hubs) != 0 { - a.GetHubForUserId(userId).InvalidateUser(userId) + hub := a.GetHubForUserId(userId) + if hub != nil { + hub.InvalidateUser(userId) } } @@ -322,8 +324,9 @@ func (a *App) InvalidateCacheForWebhookSkipClusterSend(webhookId string) { } func (a *App) InvalidateWebConnSessionCacheForUser(userId string) { - if len(a.Hubs) != 0 { - a.GetHubForUserId(userId).InvalidateUser(userId) + hub := a.GetHubForUserId(userId) + if hub != nil { + hub.InvalidateUser(userId) } } -- cgit v1.2.3-1-g7c22 From 4ce37601a15a7af11d33465d1ae92de26f7fa498 Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Sat, 12 May 2018 00:57:20 +0800 Subject: add notification when @user, @here, @all and @channel has colon ":" at the end (#8760) --- app/notification.go | 13 ++++++------ app/notification_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) (limited to 'app') diff --git a/app/notification.go b/app/notification.go index 7198de764..4bdc6c94f 100644 --- a/app/notification.go +++ b/app/notification.go @@ -919,12 +919,13 @@ func GetExplicitMentions(message string, keywords map[string][]string) *Explicit // remove trailing '.', as that is the end of a sentence foundWithSuffix := false - - for strings.HasSuffix(word, ".") { - word = strings.TrimSuffix(word, ".") - if checkForMention(word) { - foundWithSuffix = true - break + for _, suffixPunctuation := range []string{".", ":"} { + for strings.HasSuffix(word, suffixPunctuation) { + word = strings.TrimSuffix(word, suffixPunctuation) + if checkForMention(word) { + foundWithSuffix = true + break + } } } diff --git a/app/notification_test.go b/app/notification_test.go index 3b8b4adf5..818ad1d9d 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -148,6 +148,16 @@ func TestGetExplicitMentions(t *testing.T) { OtherPotentialMentions: []string{"user"}, }, }, + "OnePersonWithColonAtEnd": { + Message: "this is a message for @user:", + Keywords: map[string][]string{"this": {id1}}, + Expected: &ExplicitMentions{ + MentionedUserIds: map[string]bool{ + id1: true, + }, + OtherPotentialMentions: []string{"user"}, + }, + }, "MultiplePeopleWithOneWord": { Message: "this is a message for @user", Keywords: map[string][]string{"@user": {id1, id2}}, @@ -188,6 +198,18 @@ func TestGetExplicitMentions(t *testing.T) { ChannelMentioned: true, }, }, + + "ChannelWithColonAtEnd": { + Message: "this is a message for @channel:", + Keywords: map[string][]string{"@channel": {id1, id2}}, + Expected: &ExplicitMentions{ + MentionedUserIds: map[string]bool{ + id1: true, + id2: true, + }, + ChannelMentioned: true, + }, + }, "CapitalizedChannel": { Message: "this is an message for @cHaNNeL", Keywords: map[string][]string{"@channel": {id1, id2}}, @@ -210,6 +232,17 @@ func TestGetExplicitMentions(t *testing.T) { AllMentioned: true, }, }, + "AllWithColonAtEnd": { + Message: "this is a message for @all:", + Keywords: map[string][]string{"@all": {id1, id2}}, + Expected: &ExplicitMentions{ + MentionedUserIds: map[string]bool{ + id1: true, + id2: true, + }, + AllMentioned: true, + }, + }, "CapitalizedAll": { Message: "this is an message for @ALL", Keywords: map[string][]string{"@all": {id1, id2}}, @@ -230,6 +263,15 @@ func TestGetExplicitMentions(t *testing.T) { }, }, }, + "AtUserWithColonAtEnd": { + Message: "this is a message for @user:", + Keywords: map[string][]string{"@user": {id1}}, + Expected: &ExplicitMentions{ + MentionedUserIds: map[string]bool{ + id1: true, + }, + }, + }, "AtUserWithPeriodAtEndOfSentence": { Message: "this is a message for @user.period.", Keywords: map[string][]string{"@user.period": {id1}}, @@ -248,6 +290,15 @@ func TestGetExplicitMentions(t *testing.T) { }, }, }, + "UserWithColonAtEnd": { + Message: "this is a message for user:", + Keywords: map[string][]string{"user": {id1}}, + Expected: &ExplicitMentions{ + MentionedUserIds: map[string]bool{ + id1: true, + }, + }, + }, "PotentialOutOfChannelUser": { Message: "this is an message for @potential and @user", Keywords: map[string][]string{"@user": {id1}}, @@ -452,6 +503,7 @@ func TestGetExplicitMentionsAtHere(t *testing.T) { "\\@here\\": true, "|@here|": true, ";@here;": true, + "@here:": true, ":@here:": false, // This case shouldn't trigger a mention since it follows the format of reactions e.g. :word: "'@here'": true, "\"@here\"": true, -- cgit v1.2.3-1-g7c22