From 15b5b1c1913b4c945525ca1d68531dd11ca24b23 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Fri, 30 Sep 2022 04:12:15 -0400 Subject: [PATCH] Avoid counting top channel posts for posts made by plugins and OAuth apps (#20943) * add from_integration prop to oauth posts to: - oauth app posts - plugin posts - slash command responses - incoming webhook posts * tests * include check for bot posts * use from_plugin and from_oauth_app props * fix test * avoid counting top channel posts for posts made by plugins and oauth apps --- api4/post_test.go | 47 +++++++++++++++++++++++++++++++++ app/channel_test.go | 34 ++++++++++++++++++++++++ app/plugin_api.go | 2 ++ app/plugin_api_test.go | 19 +++++++++++++ app/post.go | 4 +++ store/sqlstore/channel_store.go | 12 ++++----- 6 files changed, 112 insertions(+), 6 deletions(-) diff --git a/api4/post_test.go b/api4/post_test.go index 31f1185ea9..b1b4754e6e 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -212,6 +212,53 @@ func TestCreatePost(t *testing.T) { require.Equal(t, post.CreateAt, rpost.CreateAt, "create at should match") } +func TestCreatePostWithOAuthClient(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + originalOAuthSetting := *th.App.Config().ServiceSettings.EnableOAuthServiceProvider + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableOAuthServiceProvider = true + }) + + defer th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableOAuthServiceProvider = originalOAuthSetting + }) + + oAuthApp, appErr := th.App.CreateOAuthApp(&model.OAuthApp{ + CreatorId: th.SystemAdminUser.Id, + Name: "name", + CallbackUrls: []string{"http://test.com"}, + Homepage: "http://test.com", + }) + require.Nil(t, appErr, "should create an OAuthApp") + + session, appErr := th.App.CreateSession(&model.Session{ + UserId: th.BasicUser.Id, + Token: "token", + IsOAuth: true, + Props: model.StringMap{model.SessionPropOAuthAppID: oAuthApp.Id}, + }) + require.Nil(t, appErr, "should create a session") + + post, _, err := th.Client.CreatePost(&model.Post{ + ChannelId: th.BasicPost.ChannelId, + Message: "test message", + }) + require.NoError(t, err) + assert.NotContains(t, post.GetProps(), "from_oauth_app", "contains from_oauth_app prop when not using OAuth client") + + client := th.CreateClient() + client.SetOAuthToken(session.Token) + post, _, err = client.CreatePost(&model.Post{ + ChannelId: th.BasicPost.ChannelId, + Message: "test message", + }) + + require.NoError(t, err) + assert.Contains(t, post.GetProps(), "from_oauth_app", "missing from_oauth_app prop when using OAuth client") +} + func TestCreatePostEphemeral(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/app/channel_test.go b/app/channel_test.go index 93ce63dfa9..0657cd337d 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -2434,6 +2434,40 @@ func TestGetTopChannelsForTeamSince(t *testing.T) { }) require.NoError(t, err) + // add an oauth app post to ensure it's not counted + _, err = th.Server.Store.Post().Save(&model.Post{ + Message: "hello from an ouath app", + ChannelId: channel3.Id, + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "from_oauth_app": true, + }, + }) + require.NoError(t, err) + + // add a plugin post to ensure it's not counted + _, err = th.Server.Store.Post().Save(&model.Post{ + Message: "hello from a plugin", + ChannelId: channel3.Id, + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "from_plugin": true, + }, + }) + require.NoError(t, err) + + // add a system post to ensure it's not counted + _, err = th.Server.Store.Post().Save(&model.Post{ + Message: "system message", + Type: "system_join_channel", + ChannelId: channel3.Id, + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "from_oauth_app": true, + }, + }) + require.NoError(t, err) + channel4 := th.CreatePrivateChannel(th.Context, th.BasicTeam) channel5 := th.CreateChannel(th.Context, th.BasicTeam) channel6 := th.CreatePrivateChannel(th.Context, th.BasicTeam) diff --git a/app/plugin_api.go b/app/plugin_api.go index f922d37ee9..ad50fc1360 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -630,6 +630,8 @@ func (api *PluginAPI) GetGroupsForUser(userID string) ([]*model.Group, *model.Ap } func (api *PluginAPI) CreatePost(post *model.Post) (*model.Post, *model.AppError) { + post.AddProp("from_plugin", "true") + post, appErr := api.app.CreatePostMissingChannel(api.ctx, post, true) if post != nil { post = post.ForPlugin() diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index 541d0f3a18..21a1d89af4 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -1383,6 +1383,25 @@ func TestPluginCreatePostWithUploadedFile(t *testing.T) { assert.Equal(t, model.StringArray{fileInfo.Id}, actualPost.FileIds) } +func TestPluginCreatePostAddsFromPluginProp(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + api := th.SetupPluginAPI() + + channelID := th.BasicChannel.Id + userID := th.BasicUser.Id + post, err := api.CreatePost(&model.Post{ + Message: "test", + ChannelId: channelID, + UserId: userID, + }) + require.Nil(t, err) + + actualPost, err := api.GetPost(post.Id) + require.Nil(t, err) + assert.Equal(t, "true", actualPost.GetProp("from_plugin")) +} + func TestPluginAPIGetConfig(t *testing.T) { th := Setup(t) defer th.TearDown() diff --git a/app/post.go b/app/post.go index ea5249ef09..0a3cd3ba36 100644 --- a/app/post.go +++ b/app/post.go @@ -203,6 +203,10 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel post.AddProp("from_bot", "true") } + if c.Session().IsOAuth { + post.AddProp("from_oauth_app", "true") + } + var ephemeralPost *model.Post if post.Type == "" && !a.HasPermissionToChannel(c, user.Id, channel.Id, model.PermissionUseChannelMentions) { mention := post.DisableMentionHighlights() diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index ee33d608ac..78db001082 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -4159,8 +4159,8 @@ func (s SqlChannelStore) GetTeamForChannel(channelID string) (*model.Team, error func (s SqlChannelStore) GetTopChannelsForTeamSince(teamID string, userID string, since int64, offset int, limit int) (*model.TopChannelList, error) { channels := make([]*model.TopChannel, 0) var args []any - postgresPropQuery := `AND (Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false')` - mySqlPropsQuery := `AND (JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false')` + postgresPropQuery := `AND (Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false') AND (Posts.Props ->> 'from_oauth_app' IS NULL OR Posts.Props ->> 'from_oauth_app' = 'false') AND (Posts.Props ->> 'from_plugin' IS NULL OR Posts.Props ->> 'from_plugin' = 'false')` + mySqlPropsQuery := `AND (JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_plugin') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_plugin') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_oauth_app') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_oauth_app') = 'false')` query := ` SELECT @@ -4263,9 +4263,9 @@ func (s SqlChannelStore) GetTopChannelsForUserSince(userID string, teamID string var propsQuery string if s.DriverName() == model.DatabaseDriverMysql { - propsQuery = `AND (JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false')` + propsQuery = `AND (JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_plugin') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_plugin') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_oauth_app') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_oauth_app') = 'false')` } else if s.DriverName() == model.DatabaseDriverPostgres { - propsQuery = `AND (Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false')` + propsQuery = `AND (Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false') AND (Posts.Props ->> 'from_oauth_app' IS NULL OR Posts.Props ->> 'from_oauth_app' = 'false') AND (Posts.Props ->> 'from_plugin' IS NULL OR Posts.Props ->> 'from_plugin' = 'false')` } query = ` @@ -4519,14 +4519,14 @@ func (s SqlChannelStore) PostCountsByDuration(channelIDs []string, sinceUnixMill } else { unixSelect = `DATE_FORMAT(CONVERT_TZ(FROM_UNIXTIME(Posts.CreateAt / 1000), 'GMT', '` + loc + `'),'%Y-%m-%dT%H') AS duration` } - propsQuery = `(JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false')` + propsQuery = `(JSON_EXTRACT(Posts.Props, '$.from_bot') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_bot') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_webhook') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_webhook') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_plugin') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_plugin') = 'false') AND (JSON_EXTRACT(Posts.Props, '$.from_oauth_app') IS NULL OR JSON_EXTRACT(Posts.Props, '$.from_oauth_app') = 'false')` } else if s.DriverName() == model.DatabaseDriverPostgres { if duration == model.PostsByDay { unixSelect = fmt.Sprintf(`TO_CHAR(TO_TIMESTAMP(Posts.CreateAt / 1000) AT TIME ZONE '%s', 'YYYY-MM-DD') AS duration`, loc) } else { unixSelect = fmt.Sprintf(`TO_CHAR(TO_TIMESTAMP(Posts.CreateAt / 1000) AT TIME ZONE '%s', 'YYYY-MM-DD"T"HH24') AS duration`, loc) } - propsQuery = `(Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false')` + propsQuery = `(Posts.Props ->> 'from_bot' IS NULL OR Posts.Props ->> 'from_bot' = 'false') AND (Posts.Props ->> 'from_webhook' IS NULL OR Posts.Props ->> 'from_webhook' = 'false') AND (Posts.Props ->> 'from_oauth_app' IS NULL OR Posts.Props ->> 'from_oauth_app' = 'false') AND (Posts.Props ->> 'from_plugin' IS NULL OR Posts.Props ->> 'from_plugin' = 'false')` } query := sq. Select("Posts.ChannelId AS channelid", unixSelect, "count(Posts.Id) AS postcount").