From f5590d4e5fdd933563bc379aabdd09a428e3a690 Mon Sep 17 00:00:00 2001 From: Berke Kalkan <42390512+berkeka@users.noreply.github.com> Date: Tue, 9 Mar 2021 20:54:54 +0300 Subject: [PATCH] MM-29726 Allow disabling link previews from certain domains (#16869) * Add field to config model Config option for disabling link previews for given domains. * Refactor functions and corresponding tests * Expand logic for link preview Newly added isLinkAllowedForPreview function determines whether a link should display a preview. It gets corresponding config values consisting of comma separated domain values, normalizes them and checks for matches. * Create tests for link preview restriction * Fix formatting issue * Add test cases where images are expected * Apply suggestions from code review Co-authored-by: Ibrahim Serdar Acikgoz * Apply remaining code suggestions * Add RestrictLinkPreviews value to telemetries Co-authored-by: Ibrahim Serdar Acikgoz Co-authored-by: Mattermod Co-authored-by: Harrison Healey --- app/post_metadata.go | 37 ++++++++++----- app/post_metadata_test.go | 80 ++++++++++++++++++++++++++++++++- model/config.go | 5 +++ services/telemetry/telemetry.go | 1 + 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/app/post_metadata.go b/app/post_metadata.go index e32896ded5a..3909a4c620d 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -123,7 +123,7 @@ func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isE } // Embeds and image dimensions - firstLink, images := getFirstLinkAndImages(post.Message) + firstLink, images := a.getFirstLinkAndImages(post.Message) if embed, err := a.getEmbedForPost(post, firstLink, isNewPost); err != nil { mlog.Debug("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Err(err)) @@ -212,7 +212,7 @@ func (a *App) getImagesForPost(post *model.Post, imageURLs []string, isNewPost b imageURLs = append(imageURLs, embed.URL) case model.POST_EMBED_MESSAGE_ATTACHMENT: - imageURLs = append(imageURLs, getImagesInMessageAttachments(post)...) + imageURLs = append(imageURLs, a.getImagesInMessageAttachments(post)...) case model.POST_EMBED_OPENGRAPH: for _, image := range embed.Data.(*opengraph.OpenGraph).Images { @@ -306,23 +306,38 @@ func (a *App) getCustomEmojisForPost(post *model.Post, reactions []*model.Reacti return a.GetMultipleEmojiByName(names) } +func (a *App) isLinkAllowedForPreview(link string) bool { + domains := a.normalizeDomains(*a.Config().ServiceSettings.RestrictLinkPreviews) + for _, d := range domains { + if strings.Contains(link, d) { + return false + } + } + + return true +} + // Given a string, returns the first autolinked URL in the string as well as an array of all Markdown // images of the form ![alt text](image url). Note that this does not return Markdown links of the // form [text](url). -func getFirstLinkAndImages(str string) (string, []string) { +func (a *App) getFirstLinkAndImages(str string) (string, []string) { firstLink := "" images := []string{} markdown.Inspect(str, func(blockOrInline interface{}) bool { switch v := blockOrInline.(type) { case *markdown.Autolink: - if firstLink == "" { - firstLink = v.Destination() + if link := v.Destination(); firstLink == "" && a.isLinkAllowedForPreview(link) { + firstLink = link } case *markdown.InlineImage: - images = append(images, v.Destination()) + if link := v.Destination(); a.isLinkAllowedForPreview(link) { + images = append(images, link) + } case *markdown.ReferenceImage: - images = append(images, v.ReferenceDefinition.Destination()) + if link := v.ReferenceDefinition.Destination(); a.isLinkAllowedForPreview(link) { + images = append(images, link) + } } return true @@ -331,19 +346,19 @@ func getFirstLinkAndImages(str string) (string, []string) { return firstLink, images } -func getImagesInMessageAttachments(post *model.Post) []string { +func (a *App) getImagesInMessageAttachments(post *model.Post) []string { var images []string for _, attachment := range post.Attachments() { - _, imagesInText := getFirstLinkAndImages(attachment.Text) + _, imagesInText := a.getFirstLinkAndImages(attachment.Text) images = append(images, imagesInText...) - _, imagesInPretext := getFirstLinkAndImages(attachment.Pretext) + _, imagesInPretext := a.getFirstLinkAndImages(attachment.Pretext) images = append(images, imagesInPretext...) for _, field := range attachment.Fields { if value, ok := field.Value.(string); ok { - _, imagesInFieldValue := getFirstLinkAndImages(value) + _, imagesInFieldValue := a.getFirstLinkAndImages(value) images = append(images, imagesInFieldValue...) } } diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 2989f1b0a23..7b97abfece1 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -1248,6 +1248,9 @@ func TestGetCustomEmojisForPost(t *testing.T) { } func TestGetFirstLinkAndImages(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + for name, testCase := range map[string]struct { Input string ExpectedFirstLink string @@ -1304,7 +1307,77 @@ func TestGetFirstLinkAndImages(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - firstLink, images := getFirstLinkAndImages(testCase.Input) + firstLink, images := th.App.getFirstLinkAndImages(testCase.Input) + + assert.Equal(t, firstLink, testCase.ExpectedFirstLink) + assert.Equal(t, images, testCase.ExpectedImages) + }) + } + + for name, testCase := range map[string]struct { + Input string + ExpectedFirstLink string + ExpectedImages []string + }{ + "http link domain is restricted": { + Input: "this is a http://example.com", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "http link domain is not restricted": { + Input: "this is a http://example1.com", + ExpectedFirstLink: "http://example1.com", + ExpectedImages: []string{}, + }, + "www link domain is restricted": { + Input: "this is a www.example.com", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "image domain is restricted": { + Input: "this is a ![our logo](http://example.com/logo)", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "image domain is not restricted": { + Input: "this is a ![our logo](http://example1.com/logo)", + ExpectedFirstLink: "", + ExpectedImages: []string{"http://example1.com/logo"}, + }, + "multiple images is domain restricted": { + Input: "this is a ![our logo](http://example.com/logo) and ![their logo](http://example.com/logo2) and ![my logo](http://example.com/logo3)", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "multiple images domain is not restricted": { + Input: "this is a ![our logo](http://example1.com/logo) and ![their logo](http://example1.com/logo2) and ![my logo](http://example1.com/logo3)", + ExpectedFirstLink: "", + ExpectedImages: []string{"http://example1.com/logo", "http://example1.com/logo2", "http://example1.com/logo3"}, + }, + "multiple images with duplicate domain is restricted": { + Input: "this is a ![our logo](http://example.com/logo) and ![their logo](http://example.com/logo2) and ![my logo which is their logo](http://example.com/logo2)", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "reference image domain is restricted": { + Input: `this is a ![our logo][logo] + +[logo]: http://example.com/logo`, + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "image and link domain is restricted": { + Input: "this is a https://example.com and ![our logo](https://example.com/logo)", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + } { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.RestrictLinkPreviews = "example.com, test.com" + }) + + t.Run(name, func(t *testing.T) { + firstLink, images := th.App.getFirstLinkAndImages(testCase.Input) assert.Equal(t, firstLink, testCase.ExpectedFirstLink) assert.Equal(t, images, testCase.ExpectedImages) @@ -1313,6 +1386,9 @@ func TestGetFirstLinkAndImages(t *testing.T) { } func TestGetImagesInMessageAttachments(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + for _, test := range []struct { Name string Post *model.Post @@ -1517,7 +1593,7 @@ func TestGetImagesInMessageAttachments(t *testing.T) { }, } { t.Run(test.Name, func(t *testing.T) { - images := getImagesInMessageAttachments(test.Post) + images := th.App.getImagesInMessageAttachments(test.Post) assert.ElementsMatch(t, images, test.Expected) }) diff --git a/model/config.go b/model/config.go index 1424ed59bd6..7e109a94335 100644 --- a/model/config.go +++ b/model/config.go @@ -300,6 +300,7 @@ type ServiceSettings struct { EnablePostUsernameOverride *bool `access:"integrations"` EnablePostIconOverride *bool `access:"integrations"` EnableLinkPreviews *bool `access:"site"` + RestrictLinkPreviews *string `access:"site"` EnableTesting *bool `access:"environment,write_restrictable,cloud_restrictable"` EnableDeveloper *bool `access:"environment,write_restrictable,cloud_restrictable"` EnableOpenTracing *bool `access:"write_restrictable,cloud_restrictable"` @@ -407,6 +408,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableLinkPreviews = NewBool(true) } + if s.RestrictLinkPreviews == nil { + s.RestrictLinkPreviews = NewString("") + } + if s.EnableTesting == nil { s.EnableTesting = NewBool(false) } diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index ce8dfd20a99..ab563702130 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -440,6 +440,7 @@ func (ts *TelemetryService) trackConfig() { "thread_auto_follow": *cfg.ServiceSettings.ThreadAutoFollow, "enable_link_previews": *cfg.ServiceSettings.EnableLinkPreviews, "enable_file_search": *cfg.ServiceSettings.EnableFileSearch, + "restrict_link_previews": isDefault(*cfg.ServiceSettings.RestrictLinkPreviews, ""), }) ts.sendTelemetry(TrackConfigTeam, map[string]interface{}{