[MM-22622] Add Ephemeral response when using mentions without permissions (#13902)

* MM-22282 Add Ephemeral response when using mentions without permission and add new prop to disable mention highlights on client

* MM-22622 Make test name test actually what it does and fix comment style

* MM-22622 Check ephemeral post created when post create with mentions on API

* MM-22622 More tests for App>CreatePost

* MM-22622 Make DisableMentionHighlights more concise and rename ephemeral post

* MM-22622 Dont send ephemeral message for system message created by user

* Trigger build
This commit is contained in:
Farhan Munshi 2020-03-03 05:22:49 -05:00 committed by GitHub
parent 4c7fee7ac8
commit aec606500f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 366 additions and 1 deletions

View file

@ -112,11 +112,33 @@ func TestCreatePost(t *testing.T) {
assert.Equal(t, model.StringArray{fileId}, actualPostWithFiles.FileIds)
})
t.Run("creates a post that has channel mentions without the USE_CHANNEL_MENTIONS Permission", func(t *testing.T) {
t.Run("Create posts without the USE_CHANNEL_MENTIONS Permission - returns ephemeral message with mentions and no ephemeral message without mentions", func(t *testing.T) {
WebSocketClient, err := th.CreateWebSocketClient()
WebSocketClient.Listen()
require.Nil(t, err)
defer th.RestoreDefaultRolePermissions(th.SaveDefaultRolePermissions())
th.RemovePermissionFromRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID)
post.RootId = rpost.Id
post.ParentId = rpost.Id
post.Message = "a post with no channel mentions"
_, resp = Client.CreatePost(post)
CheckNoError(t, resp)
// Message with no channel mentions should result in no ephemeral message
timeout := time.After(300 * time.Millisecond)
waiting := true
for waiting {
select {
case event := <-WebSocketClient.EventChannel:
require.NotEqual(t, model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE, event.EventType(), "should not have ephemeral message event")
case <-timeout:
waiting = false
}
}
post.RootId = rpost.Id
post.ParentId = rpost.Id
post.Message = "a post with @channel"
@ -134,6 +156,21 @@ func TestCreatePost(t *testing.T) {
post.Message = "a post with @here"
_, resp = Client.CreatePost(post)
CheckNoError(t, resp)
timeout = time.After(600 * time.Millisecond)
eventsToGo := 3 // 3 Posts created with @ mentions should result in 3 websocket events
for eventsToGo > 0 {
select {
case event := <-WebSocketClient.EventChannel:
if event.Event == model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE {
require.Equal(t, model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE, event.Event)
eventsToGo = eventsToGo - 1
}
case <-timeout:
require.Fail(t, "Should have received ephemeral message event and not timedout")
eventsToGo = 0
}
}
})
post.RootId = ""

View file

@ -189,6 +189,22 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
return nil, model.NewAppError("createPost", "api.post.create_post.town_square_read_only", nil, "", http.StatusForbidden)
}
var ephemeralPost *model.Post
if post.Type == "" && !a.HasPermissionToChannel(user.Id, channel.Id, model.PERMISSION_USE_CHANNEL_MENTIONS) {
mention := post.DisableMentionHighlights()
if mention != "" {
T := utils.GetUserTranslations(user.Locale)
ephemeralPost = &model.Post{
UserId: user.Id,
RootId: post.RootId,
ParentId: post.ParentId,
ChannelId: channel.Id,
Message: T("model.post.channel_notifications_disabled_in_channel.message", model.StringInterface{"ChannelName": channel.Name, "Mention": mention}),
Props: model.StringInterface{model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
}
}
}
// Verify the parent/child relationships are correct
var parentPostList *model.PostList
if pchan != nil {
@ -311,6 +327,11 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
mlog.Error("Failed to handle post events", mlog.Err(err))
}
// Send any ephemeral posts after the post is created to ensure it shows up after the latest post created
if ephemeralPost != nil {
a.SendEphemeralPost(post.UserId, ephemeralPost)
}
return rpost, nil
}
@ -604,6 +625,10 @@ func (a *App) PatchPost(postId string, patch *model.PostPatch) (*model.Post, *mo
return nil, err
}
if !a.HasPermissionToChannel(post.UserId, post.ChannelId, model.PERMISSION_USE_CHANNEL_MENTIONS) {
patch.DisableMentionHighlights()
}
post.Patch(patch)
updatedPost, err := a.UpdatePost(post, false)

View file

@ -680,6 +680,56 @@ func TestCreatePost(t *testing.T) {
require.Nil(t, err)
assert.Equal(t, "![image]("+proxiedImageURL+")", rpost.Message)
})
t.Run("Sets prop MENTION_HIGHLIGHT_DISABLED when it should", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.AddUserToChannel(th.BasicUser, th.BasicChannel)
t.Run("Does not set prop when user has USE_CHANNEL_MENTIONS", func(t *testing.T) {
postWithNoMention := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "This post does not have mentions",
UserId: th.BasicUser.Id,
}
rpost, err := th.App.CreatePost(postWithNoMention, th.BasicChannel, false)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
postWithMention := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "This post has @here mention @all",
UserId: th.BasicUser.Id,
}
rpost, err = th.App.CreatePost(postWithMention, th.BasicChannel, false)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
})
t.Run("Sets prop when post has mentions and user does not have USE_CHANNEL_MENTIONS", func(t *testing.T) {
th.RemovePermissionFromRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID)
postWithNoMention := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "This post does not have mentions",
UserId: th.BasicUser.Id,
}
rpost, err := th.App.CreatePost(postWithNoMention, th.BasicChannel, false)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
postWithMention := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "This post has @here mention @all",
UserId: th.BasicUser.Id,
}
rpost, err = th.App.CreatePost(postWithMention, th.BasicChannel, false)
require.Nil(t, err)
assert.Equal(t, rpost.Props[model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED], true)
th.AddPermissionToRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID)
})
})
}
func TestPatchPost(t *testing.T) {
@ -716,6 +766,53 @@ func TestPatchPost(t *testing.T) {
require.Nil(t, err)
assert.Equal(t, "![image]("+proxiedImageURL+")", rpost.Message)
})
t.Run("Sets Prop MENTION_HIGHLIGHT_DISABLED when it should", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.AddUserToChannel(th.BasicUser, th.BasicChannel)
post := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "This post does not have mentions",
UserId: th.BasicUser.Id,
}
rpost, err := th.App.CreatePost(post, th.BasicChannel, false)
require.Nil(t, err)
t.Run("Does not set prop when user has USE_CHANNEL_MENTIONS", func(t *testing.T) {
patchWithNoMention := &model.PostPatch{Message: model.NewString("This patch has no channel mention")}
rpost, err = th.App.PatchPost(rpost.Id, patchWithNoMention)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
patchWithMention := &model.PostPatch{Message: model.NewString("This patch has a mention now @here")}
rpost, err = th.App.PatchPost(rpost.Id, patchWithMention)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
})
t.Run("Sets prop when user does not have USE_CHANNEL_MENTIONS", func(t *testing.T) {
th.RemovePermissionFromRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID)
patchWithNoMention := &model.PostPatch{Message: model.NewString("This patch still does not have a mention")}
rpost, err = th.App.PatchPost(rpost.Id, patchWithNoMention)
require.Nil(t, err)
assert.Equal(t, rpost.Props, model.StringInterface{})
patchWithMention := &model.PostPatch{Message: model.NewString("This patch has a mention now @here")}
rpost, err = th.App.PatchPost(rpost.Id, patchWithMention)
require.Nil(t, err)
assert.Equal(t, rpost.Props[model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED], true)
th.AddPermissionToRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID)
})
})
}
func TestPatchPostInArchivedChannel(t *testing.T) {

1
go.sum
View file

@ -264,6 +264,7 @@ github.com/mattermost/gosaml2 v0.3.2 h1:kq2dY5qUe6fPPHra171GVlgo+ycBsEog0gZMetxL
github.com/mattermost/gosaml2 v0.3.2/go.mod h1:Z429EIOiEi9kbq6yHoApfzlcXpa6dzRDc6pO+Vy2Ksk=
github.com/mattermost/ldap v0.0.0-20191128190019-9f62ba4b8d4d h1:2DV7VIlEv6J5R5o6tUcb3ZMKJYeeZuWZL7Rv1m23TgQ=
github.com/mattermost/ldap v0.0.0-20191128190019-9f62ba4b8d4d/go.mod h1:HLbgMEI5K131jpxGazJ97AxfPDt31osq36YS1oxFQPQ=
github.com/mattermost/mattermost-server v5.11.1+incompatible h1:LPzKY0+2Tic/ik67qIg6VrydRCgxNXZQXOeaiJ2rMBY=
github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 h1:G9tL6JXRBMzjuD1kkBtcnd42kUiT6QDwxfFYu7adM6o=
github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0/go.mod h1:nV5bfVpT//+B1RPD2JvRnxbkLmJEYXmRaaVl15fsXjs=
github.com/mattermost/viper v1.0.4 h1:cMYOz4PhguscGSPxrSokUtib5HrG4gCpiUh27wyA3d0=

View file

@ -5322,6 +5322,10 @@
"id": "model.plugin_kvset_options.is_valid.old_value.app_error",
"translation": "Invalid old value, it shouldn't be set when the operation is not atomic."
},
{
"id": "model.post.channel_notifications_disabled_in_channel.message",
"translation": "Channel notifications are disabled in {{.ChannelName}}. The {{.Mention}} did not trigger any notifications."
},
{
"id": "model.post.is_valid.channel_id.app_error",
"translation": "Invalid channel id"

View file

@ -7,6 +7,7 @@ import (
"encoding/json"
"io"
"net/http"
"regexp"
"sort"
"strings"
"unicode/utf8"
@ -58,6 +59,8 @@ const (
POST_PROPS_DELETE_BY = "deleteBy"
POST_PROPS_OVERRIDE_ICON_URL = "override_icon_url"
POST_PROPS_OVERRIDE_ICON_EMOJI = "override_icon_emoji"
POST_PROPS_MENTION_HIGHLIGHT_DISABLED = "mentionHighlightDisabled"
)
type Post struct {
@ -432,6 +435,34 @@ func (o *Post) ChannelMentions() []string {
return ChannelMentions(o.Message)
}
// DisableMentionHighlights disables a posts mention highlighting and returns the first channel mention that was present in the message.
func (o *Post) DisableMentionHighlights() string {
mention, hasMentions := findAtChannelMention(o.Message)
if hasMentions {
o.AddProp(POST_PROPS_MENTION_HIGHLIGHT_DISABLED, true)
}
return mention
}
// DisableMentionHighlights disables mention highlighting for a post patch if required.
func (o *PostPatch) DisableMentionHighlights() {
if _, hasMentions := findAtChannelMention(*o.Message); hasMentions {
if o.Props == nil {
o.Props = &StringInterface{}
}
(*o.Props)[POST_PROPS_MENTION_HIGHLIGHT_DISABLED] = true
}
}
func findAtChannelMention(message string) (mention string, found bool) {
re := regexp.MustCompile(`(?i)\B@(channel|all|here)\b`)
matched := re.FindStringSubmatch(message)
if found = (len(matched) > 0); found {
mention = strings.ToLower(matched[0])
}
return
}
func (o *Post) Attachments() []*SlackAttachment {
if attachments, ok := o.Props["attachments"].([]*SlackAttachment); ok {
return attachments

View file

@ -502,3 +502,173 @@ func BenchmarkRewriteImageURLs(b *testing.B) {
})
}
}
func Test_findAtChannelMention(t *testing.T) {
testCases := []struct {
Name string
Message string
Mention string
Found bool
}{
{
"Returns mention for @here wrapped by spaces",
"hi guys @here wrapped by spaces",
"@here",
true,
},
{
"Returns mention for @all wrapped by spaces",
"hi guys @all wrapped by spaces",
"@all",
true,
},
{
"Returns mention for @channel wrapped by spaces",
"hi guys @channel wrapped by spaces",
"@channel",
true,
},
{
"Returns mention for @here wrapped by dash",
"-@here-",
"@here",
true,
},
{
"Returns mention for @all wrapped by back tick",
"`@all`",
"@all",
true,
},
{
"Returns mention for @channel wrapped by tags",
"<@channel>",
"@channel",
true,
},
{
"Returns mention for @channel wrapped by asterisks",
"*@channel*",
"@channel",
true,
},
{
"Does not return mention when prefixed by letters",
"hi@channel",
"",
false,
},
{
"Does not return mention when suffixed by letters",
"hi @channelanotherword",
"",
false,
},
{
"Returns mention when prefixed by word ending in special character",
"hi-@channel",
"@channel",
true,
},
{
"Returns mention when suffixed by word starting in special character",
"hi @channel-guys",
"@channel",
true,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
mention, found := findAtChannelMention(tc.Message)
assert.Equal(t, tc.Mention, mention)
assert.Equal(t, tc.Found, found)
})
}
}
func TestPostDisableMentionHighlights(t *testing.T) {
post := &Post{}
testCases := []struct {
Name string
Message string
ExpectedProps StringInterface
ExpectedMention string
}{
{
"Does nothing for post with no mentions",
"Sample message with no mentions",
StringInterface(nil),
"",
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED and returns mention",
"Sample message with @here",
StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
"@here",
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED and returns mention",
"Sample message with @channel",
StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
"@channel",
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED and returns mention",
"Sample message with @all",
StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
"@all",
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
post.Message = tc.Message
mention := post.DisableMentionHighlights()
assert.Equal(t, tc.ExpectedMention, mention)
assert.Equal(t, tc.ExpectedProps, post.Props)
post.Props = StringInterface{}
})
}
}
func TestPostPatchDisableMentionHighlights(t *testing.T) {
patch := &PostPatch{}
testCases := []struct {
Name string
Message string
ExpectedProps *StringInterface
}{
{
"Does nothing for post with no mentions",
"Sample message with no mentions",
nil,
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED",
"Sample message with @here",
&StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED",
"Sample message with @channel",
&StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
},
{
"Sets POST_PROPS_MENTION_HIGHLIGHT_DISABLED",
"Sample message with @all",
&StringInterface{POST_PROPS_MENTION_HIGHLIGHT_DISABLED: true},
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
patch.Message = &tc.Message
patch.DisableMentionHighlights()
if tc.ExpectedProps == nil {
assert.Nil(t, patch.Props)
} else {
assert.Equal(t, *tc.ExpectedProps, *patch.Props)
}
patch.Props = nil
})
}
}