Do not rewrite messgaes from bots

mattermost-community · Mar 23, 2020 · 64909f0 · 64909f0
1 parent a10f0ee
commit 64909f0
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 0 deletions.
diff --git a/server/autolink/autolink_test.go b/server/autolink/autolink_test.go
@@ -25,6 +25,12 @@ func setupTestPlugin(t *testing.T, l autolink.Autolink) *autolinkplugin.Plugin {
 		TeamId: "theteam_id0123456789012345",
 		Name:   "thechannel_name",
 	}, (*model.AppError)(nil))
+
+	testUser := model.User{
+		IsBot: false,
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(&testUser, nil)
+
 	p.SetAPI(api)
 
 	err := l.Compile()

diff --git a/server/autolinkplugin/plugin.go b/server/autolinkplugin/plugin.go
@@ -78,7 +78,29 @@ func contains(team string, channel string, list []string) bool {
 	return false
 }
 
+func (p *Plugin) isBotUser(userId string) (bool, *model.AppError) {
+	user, appErr := p.API.GetUser(userId)
+	if appErr != nil {
+		p.API.LogError("failed to check if message for rewriting was send by a bot", "error", appErr)
+		return false, appErr
+	}
+
+	return user.IsBot, nil
+}
+
 func (p *Plugin) ProcessPost(c *plugin.Context, post *model.Post) (*model.Post, string) {
+	isBot, appErr := p.isBotUser(post.UserId)
+	if appErr != nil {
+		// NOTE: Not sure how we want to handle errors here, we can either:
+		// * assume that occasional rewrites of Bot messges are ok
+		// * assume that occasional not rewriting of all messages is ok
+		// Let's assume for now that former is a lesser evil and carry on.
+	}
+	if isBot {
+		p.API.LogDebug("not rewriting message from bot", "userId", post.UserId)
+		return nil, ""
+	}
+
 	conf := p.getConfig()
 	postText := post.Message
 	offset := 0

diff --git a/server/autolinkplugin/plugin_test.go b/server/autolinkplugin/plugin_test.go
@@ -47,6 +47,11 @@ func TestPlugin(t *testing.T) {
 	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil)
 	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil)
 
+	testUser := model.User{
+		IsBot: false,
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(&testUser, nil)
+
 	p := New()
 	p.SetAPI(api)
 	p.OnConfigurationChange()
@@ -314,6 +319,11 @@ func TestSpecialCases(t *testing.T) {
 	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil)
 	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil)
 
+	testUser := model.User{
+		IsBot: false,
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(&testUser, nil)
+
 	p := New()
 	p.SetAPI(api)
 	p.OnConfigurationChange()
@@ -487,6 +497,95 @@ func TestSpecialCases(t *testing.T) {
 	}
 }
 
+func TestBotMessagesAreRewritenWhenGetUserFails(t *testing.T) {
+	conf := Config{
+		Links: []autolink.Autolink{
+			autolink.Autolink{
+				Pattern:  "(Mattermost)",
+				Template: "[Mattermost](https://mattermost.com)",
+			},
+		},
+	}
+
+	testChannel := model.Channel{
+		Name: "TestChanel",
+	}
+
+	testTeam := model.Team{
+		Name: "TestTeam",
+	}
+
+	api := &plugintest.API{}
+
+	api.On("LoadPluginConfiguration", mock.AnythingOfType("*autolinkplugin.Config")).Return(func(dest interface{}) error {
+		*dest.(*Config) = conf
+		return nil
+	}).Once()
+	api.On("UnregisterCommand", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return((*model.AppError)(nil)).Once()
+
+	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil).Once()
+	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil).Once()
+	api.On("LogError", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("*model.AppError"))
+
+	err := &model.AppError{
+		Message: "foo error!",
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(nil, err).Once()
+
+	p := Plugin{}
+	p.SetAPI(api)
+	p.OnConfigurationChange()
+
+	post := &model.Post{Message: "Welcome to Mattermost!"}
+	rpost, _ := p.MessageWillBePosted(&plugin.Context{}, post)
+
+	assert.Equal(t, "Welcome to [Mattermost](https://mattermost.com)!", rpost.Message)
+}
+
+func TestBotMessagesAreNotRewriten(t *testing.T) {
+	conf := Config{
+		Links: []autolink.Autolink{
+			autolink.Autolink{
+				Pattern:  "(Mattermost)",
+				Template: "[Mattermost](https://mattermost.com)",
+			},
+		},
+	}
+
+	testChannel := model.Channel{
+		Name: "TestChanel",
+	}
+
+	testTeam := model.Team{
+		Name: "TestTeam",
+	}
+
+	api := &plugintest.API{}
+
+	api.On("LoadPluginConfiguration", mock.AnythingOfType("*autolinkplugin.Config")).Return(func(dest interface{}) error {
+		*dest.(*Config) = conf
+		return nil
+	}).Once()
+	api.On("UnregisterCommand", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return((*model.AppError)(nil)).Once()
+
+	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil).Once()
+	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil).Once()
+	testUser := model.User{
+		IsBot: true,
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(&testUser, nil).Once()
+	api.On("LogDebug", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"))
+
+	p := Plugin{}
+	p.SetAPI(api)
+	p.OnConfigurationChange()
+
+	post := &model.Post{Message: "Welcome to Mattermost!"}
+	rpost, _ := p.MessageWillBePosted(&plugin.Context{}, post)
+
+	assert.Nil(t, rpost)
+}
+
 func TestHashtags(t *testing.T) {
 	conf := Config{
 		Links: []autolink.Autolink{
@@ -520,6 +619,11 @@ func TestHashtags(t *testing.T) {
 	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil)
 	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil)
 
+	testUser := model.User{
+		IsBot: false,
+	}
+	api.On("GetUser", mock.AnythingOfType("string")).Return(&testUser, nil)
+
 	p := New()
 	p.SetAPI(api)
 	p.OnConfigurationChange()