From 6da1d90c4a52d949a6357fa94fed2b4e19fee5ed Mon Sep 17 00:00:00 2001 From: Mylon Suren <23694620+mylonsuren@users.noreply.github.com> Date: Thu, 7 Apr 2022 17:32:18 -0400 Subject: [PATCH] [MM-38615] Permalink previews for DMs/GMs (#19765) --- app/post.go | 24 ++++--- app/post_metadata.go | 24 +++++-- app/post_metadata_test.go | 115 +++++++++++++++++++++++++++++++++ app/post_test.go | 132 ++++++++++++++++++++++++++++++++++++++ model/permalink.go | 12 ++-- 5 files changed, 291 insertions(+), 16 deletions(-) diff --git a/app/post.go b/app/post.go index 7b32efb4b9..508ca43548 100644 --- a/app/post.go +++ b/app/post.go @@ -732,15 +732,23 @@ func (a *App) publishWebsocketEventForPermalinkPost(post *model.Post, message *m return false, err } - for _, cm := range channelMembers { - postForUser, err := a.SanitizePostMetadataForUser(post, cm.UserId) - if err != nil { - if err.StatusCode == http.StatusNotFound { - mlog.Warn("channel containing permalinked post not found", mlog.String("referenced_channel_id", previewedPost.ChannelId)) - return false, nil - } - return false, err + permalinkPreviewedChannel, err := a.GetChannel(previewedPost.ChannelId) + if err != nil { + if err.StatusCode == http.StatusNotFound { + mlog.Warn("channel containing permalinked post not found", mlog.String("referenced_channel_id", previewedPost.ChannelId)) + return false, nil } + return false, err + } + + permalinkPreviewedPost := post.GetPreviewPost() + for _, cm := range channelMembers { + if permalinkPreviewedPost != nil { + post.Metadata.Embeds[0].Data = permalinkPreviewedPost + } + + postForUser := a.sanitizePostMetadataForUserAndChannel(post, permalinkPreviewedPost, permalinkPreviewedChannel, cm.UserId) + // Using DeepCopy here to avoid a race condition // between publishing the event and setting the "post" data value below. messageCopy := message.DeepCopy() diff --git a/app/post_metadata.go b/app/post_metadata.go index 0a0b5449e1..9f38e18884 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -163,6 +163,18 @@ func (a *App) getEmbedsAndImages(post *model.Post, isNewPost bool) *model.Post { return post } +func (a *App) sanitizePostMetadataForUserAndChannel(post *model.Post, previewedPost *model.PreviewPost, previewedChannel *model.Channel, userID string) *model.Post { + if post.Metadata == nil || len(post.Metadata.Embeds) == 0 || previewedPost == nil { + return post + } + + if previewedChannel != nil && !a.HasPermissionToReadChannel(userID, previewedChannel) { + post.Metadata.Embeds[0].Data = nil + } + + return post +} + func (a *App) SanitizePostMetadataForUser(post *model.Post, userID string) (*model.Post, *model.AppError) { if post.Metadata == nil || len(post.Metadata.Embeds) == 0 { return post, nil @@ -179,7 +191,6 @@ func (a *App) SanitizePostMetadataForUser(post *model.Post, userID string) (*mod } if previewedChannel != nil && !a.HasPermissionToReadChannel(userID, previewedChannel) { - post = post.Clone() post.Metadata.Embeds[0].Data = nil } @@ -540,9 +551,14 @@ func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool return nil, nil, nil, appErr } - referencedTeam, appErr := a.GetTeam(referencedChannel.TeamId) - if appErr != nil { - return nil, nil, nil, appErr + var referencedTeam *model.Team + if referencedChannel.Type == model.ChannelTypeDirect || referencedChannel.Type == model.ChannelTypeGroup { + referencedTeam = &model.Team{} + } else { + referencedTeam, appErr = a.GetTeam(referencedChannel.TeamId) + if appErr != nil { + return nil, nil, nil, appErr + } } // Get metadata for embedded post diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 25af86deb2..d75d561a4f 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -585,6 +585,69 @@ func TestPreparePostForClient(t *testing.T) { require.Equal(t, referencedPost.Id, preview.PostID) }) + t.Run("permalink previews for direct and group messages", func(t *testing.T) { + th := setup(t) + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + + th.Context.Session().UserId = th.BasicUser.Id + + directChannel, err := th.App.createDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + require.Nil(t, err) + + groupChannel, err := th.App.createGroupChannel([]string{th.BasicUser.Id, th.BasicUser2.Id, th.CreateUser().Id}) + require.Nil(t, err) + + testCases := []struct { + Description string + Channel *model.Channel + Expected model.ChannelType + }{ + { + Description: "direct message permalink preview", + Channel: directChannel, + Expected: model.ChannelType("D"), + }, + { + Description: "group message permalink preview", + Channel: groupChannel, + Expected: model.ChannelType("G"), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + referencedPost, err := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: testCase.Channel.Id, + Message: "hello world", + }, th.BasicChannel, false, true) + require.Nil(t, err) + referencedPost.Metadata.Embeds = nil + + link := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) + + previewPost, err := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: link, + }, th.BasicChannel, false, true) + require.Nil(t, err) + previewPost.Metadata.Embeds = nil + + clientPost := th.App.PreparePostForClientWithEmbedsAndImages(previewPost, false, false) + firstEmbed := clientPost.Metadata.Embeds[0] + preview := firstEmbed.Data.(*model.PreviewPost) + + assert.Empty(t, preview.TeamName) + assert.Equal(t, testCase.Expected, preview.ChannelType) + }) + } + }) + t.Run("permalink with nested preview should have referenced post metadata", func(t *testing.T) { th := setup(t) defer th.TearDown() @@ -2625,3 +2688,55 @@ func TestContainsPermalink(t *testing.T) { }) } } + +func TestSanitizePostMetadataForUserAndChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableLinkPreviews = true + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + + directChannel, err := th.App.createDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + assert.Nil(t, err) + + userID := model.NewId() + post := &model.Post{ + Id: userID, + Metadata: &model.PostMetadata{ + Embeds: []*model.PostEmbed{ + { + Type: model.PostEmbedOpengraph, + URL: "ogURL", + Data: &opengraph.OpenGraph{ + Images: []*opengraph.Image{ + { + URL: "imageURL", + }, + }, + }, + }, + }, + }, + } + + previewedPost := model.NewPreviewPost(post, th.BasicTeam, directChannel) + + actual := th.App.sanitizePostMetadataForUserAndChannel(post, previewedPost, directChannel, th.BasicUser2.Id) + assert.NotNil(t, actual.Metadata.Embeds[0].Data) + + guestID := model.NewId() + guest := &model.User{ + Email: "success+" + guestID + "@simulator.amazonses.com", + Username: "un_" + guestID, + Nickname: "nn_" + guestID, + Password: "Password1", + EmailVerified: true, + } + guest, appErr := th.App.CreateGuest(th.Context, guest) + require.Nil(t, appErr) + + actual = th.App.sanitizePostMetadataForUserAndChannel(post, previewedPost, directChannel, guest.Id) + assert.Nil(t, actual.Metadata.Embeds[0].Data) +} diff --git a/app/post_test.go b/app/post_test.go index b49e6e32f5..a96696e19c 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -829,6 +829,70 @@ func TestCreatePost(t *testing.T) { require.EqualValues(t, int64(1), val) }) + t.Run("sanitizes post metadata appropriately", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + + th.AddUserToChannel(th.BasicUser, th.BasicChannel) + + user1 := th.CreateUser() + user2 := th.CreateUser() + directChannel, err := th.App.createDirectChannel(user1.Id, user2.Id) + require.Nil(t, err) + + referencedPost := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "hello world", + UserId: th.BasicUser.Id, + } + + th.Context.Session().UserId = th.BasicUser.Id + + referencedPost, err = th.App.CreatePost(th.Context, referencedPost, th.BasicChannel, false, false) + require.Nil(t, err) + + permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) + + testCases := []struct { + Description string + Channel *model.Channel + Author string + Assert func(t assert.TestingT, object interface{}, msgAndArgs ...interface{}) bool + }{ + { + Description: "removes metadata from post for members who cannot read channel", + Channel: directChannel, + Author: user1.Id, + Assert: assert.Nil, + }, + { + Description: "does not remove metadata from post for members who can read channel", + Channel: th.BasicChannel, + Author: th.BasicUser.Id, + Assert: assert.NotNil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + previewPost := &model.Post{ + ChannelId: testCase.Channel.Id, + Message: permalink, + UserId: testCase.Author, + } + + previewPost, err = th.App.CreatePost(th.Context, previewPost, testCase.Channel, false, false) + require.Nil(t, err) + + testCase.Assert(t, previewPost.Metadata.Embeds[0].Data) + }) + } + }) + t.Run("MM-40016 should not panic with `concurrent map read and map write`", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -1246,6 +1310,74 @@ func TestUpdatePost(t *testing.T) { require.Nil(t, err) assert.Equal(t, testPost.GetProps(), model.StringInterface{"previewed_post": referencedPost.Id}) }) + + t.Run("sanitizes post metadata appropriately", func(t *testing.T) { + + th := Setup(t).InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + + th.AddUserToChannel(th.BasicUser, th.BasicChannel) + + user1 := th.CreateUser() + user2 := th.CreateUser() + directChannel, err := th.App.createDirectChannel(user1.Id, user2.Id) + require.Nil(t, err) + + referencedPost := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "hello world", + UserId: th.BasicUser.Id, + } + + th.Context.Session().UserId = th.BasicUser.Id + + referencedPost, err = th.App.CreatePost(th.Context, referencedPost, th.BasicChannel, false, false) + require.Nil(t, err) + + permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) + + testCases := []struct { + Description string + Channel *model.Channel + Author string + Assert func(t assert.TestingT, object interface{}, msgAndArgs ...interface{}) bool + }{ + { + Description: "removes metadata from post for members who cannot read channel", + Channel: directChannel, + Author: user1.Id, + Assert: assert.Nil, + }, + { + Description: "does not remove metadata from post for members who can read channel", + Channel: th.BasicChannel, + Author: th.BasicUser.Id, + Assert: assert.NotNil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + previewPost := &model.Post{ + ChannelId: testCase.Channel.Id, + UserId: testCase.Author, + } + + previewPost, err = th.App.CreatePost(th.Context, previewPost, testCase.Channel, false, false) + require.Nil(t, err) + + previewPost.Message = permalink + previewPost, err = th.App.UpdatePost(th.Context, previewPost, false) + require.Nil(t, err) + + testCase.Assert(t, previewPost.Metadata.Embeds[0].Data) + }) + } + }) } func TestSearchPostsForUser(t *testing.T) { diff --git a/model/permalink.go b/model/permalink.go index 6a19fb7576..1264564622 100644 --- a/model/permalink.go +++ b/model/permalink.go @@ -8,10 +8,12 @@ type Permalink struct { } type PreviewPost struct { - PostID string `json:"post_id"` - Post *Post `json:"post"` - TeamName string `json:"team_name"` - ChannelDisplayName string `json:"channel_display_name"` + PostID string `json:"post_id"` + Post *Post `json:"post"` + TeamName string `json:"team_name"` + ChannelDisplayName string `json:"channel_display_name"` + ChannelType ChannelType `json:"channel_type"` + ChannelID string `json:"channel_id"` } func NewPreviewPost(post *Post, team *Team, channel *Channel) *PreviewPost { @@ -23,5 +25,7 @@ func NewPreviewPost(post *Post, team *Team, channel *Channel) *PreviewPost { Post: post, TeamName: team.Name, ChannelDisplayName: channel.DisplayName, + ChannelType: channel.Type, + ChannelID: channel.Id, } }