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 <serdaracikgoz86@gmail.com>

* Apply remaining code suggestions

* Add RestrictLinkPreviews value to telemetries

Co-authored-by: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Harrison Healey <harrisonmhealey@gmail.com>
This commit is contained in:
Berke Kalkan 2021-03-09 20:54:54 +03:00 committed by GitHub
parent 8612f3a4d1
commit f5590d4e5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 110 additions and 13 deletions

View file

@ -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...)
}
}

View file

@ -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)
})

View file

@ -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)
}

View file

@ -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{}{