From 72c86448b94f29aa17dd4a3ea56b18d38a45cb42 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Wed, 9 Jun 2021 17:40:22 +0200 Subject: [PATCH] [MM-27622] Publish messages as SystemBot when a user is required (#17598) Automatic Merge --- api4/channel_local.go | 110 +++++++++++++++++++++-- api4/channel_test.go | 130 +++++++++++++++------------ app/app.go | 39 ++++---- app/app_iface.go | 3 + app/bot.go | 68 ++++++++++++-- app/bot_test.go | 38 ++++++++ app/channel.go | 103 ++++++++++++++++++--- app/opentracing/opentracing_layer.go | 44 +++++++++ i18n/en.json | 12 +++ model/bot.go | 1 + 10 files changed, 439 insertions(+), 109 deletions(-) diff --git a/api4/channel_local.go b/api4/channel_local.go index e921d4789c..267d7c07b8 100644 --- a/api4/channel_local.go +++ b/api4/channel_local.go @@ -6,6 +6,7 @@ package api4 import ( "net/http" + "github.com/mattermost/mattermost-server/v5/app" "github.com/mattermost/mattermost-server/v5/audit" "github.com/mattermost/mattermost-server/v5/model" ) @@ -18,6 +19,8 @@ func (api *API) InitChannelLocal() { api.BaseRoutes.Channel.Handle("", api.ApiLocal(localDeleteChannel)).Methods("DELETE") api.BaseRoutes.Channel.Handle("/patch", api.ApiLocal(localPatchChannel)).Methods("PUT") api.BaseRoutes.Channel.Handle("/move", api.ApiLocal(localMoveChannel)).Methods("POST") + api.BaseRoutes.Channel.Handle("/privacy", api.ApiLocal(localUpdateChannelPrivacy)).Methods("PUT") + api.BaseRoutes.Channel.Handle("/restore", api.ApiLocal(localRestoreChannel)).Methods("POST") api.BaseRoutes.ChannelMember.Handle("", api.ApiLocal(localRemoveChannelMember)).Methods("DELETE") api.BaseRoutes.ChannelMember.Handle("", api.ApiLocal(getChannelMember)).Methods("GET") @@ -57,6 +60,76 @@ func localCreateChannel(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(sc.ToJson())) } +func localUpdateChannelPrivacy(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + + props := model.StringInterfaceFromJson(r.Body) + privacy, ok := props["privacy"].(string) + if !ok || (privacy != model.CHANNEL_OPEN && privacy != model.CHANNEL_PRIVATE) { + c.SetInvalidParam("privacy") + return + } + + channel, err := c.App.GetChannel(c.Params.ChannelId) + if err != nil { + c.Err = err + return + } + + auditRec := c.MakeAuditRecord("localUpdateChannelPrivacy", audit.Fail) + defer c.LogAuditRec(auditRec) + auditRec.AddMeta("channel", channel) + auditRec.AddMeta("new_type", privacy) + + if channel.Name == model.DEFAULT_CHANNEL && privacy == model.CHANNEL_PRIVATE { + c.Err = model.NewAppError("updateChannelPrivacy", "api.channel.update_channel_privacy.default_channel_error", nil, "", http.StatusBadRequest) + return + } + channel.Type = privacy + + updatedChannel, err := c.App.UpdateChannelPrivacy(c.AppContext, channel, nil) + if err != nil { + c.Err = err + return + } + + auditRec.Success() + c.LogAudit("name=" + updatedChannel.Name) + + w.Write([]byte(updatedChannel.ToJson())) +} + +func localRestoreChannel(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + + channel, err := c.App.GetChannel(c.Params.ChannelId) + if err != nil { + c.Err = err + return + } + + auditRec := c.MakeAuditRecord("localRestoreChannel", audit.Fail) + defer c.LogAuditRec(auditRec) + auditRec.AddMeta("channel", channel) + + channel, err = c.App.RestoreChannel(c.AppContext, channel, "") + if err != nil { + c.Err = err + return + } + + auditRec.Success() + c.LogAudit("name=" + channel.Name) + + w.Write([]byte(channel.ToJson())) +} + func localAddChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireChannelId() if c.Err != nil { @@ -70,13 +143,30 @@ func localAddChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { return } - user, err := c.App.GetUser(userId) - if err != nil { - c.Err = err + member := &model.ChannelMember{ + ChannelId: c.Params.ChannelId, + UserId: userId, + } + + postRootId, ok := props["post_root_id"].(string) + if ok && postRootId != "" && !model.IsValidId(postRootId) { + c.SetInvalidParam("post_root_id") return } - channel, err := c.App.GetChannel(c.Params.ChannelId) + if ok && len(postRootId) == 26 { + rootPost, err := c.App.GetSinglePost(postRootId) + if err != nil { + c.Err = err + return + } + if rootPost.ChannelId != member.ChannelId { + c.SetInvalidParam("post_root_id") + return + } + } + + channel, err := c.App.GetChannel(member.ChannelId) if err != nil { c.Err = err return @@ -87,27 +177,29 @@ func localAddChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("channel", channel) if channel.Type == model.CHANNEL_DIRECT || channel.Type == model.CHANNEL_GROUP { - c.Err = model.NewAppError("addUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "", http.StatusBadRequest) + c.Err = model.NewAppError("localAddChannelMember", "api.channel.add_user_to_channel.type.app_error", nil, "", http.StatusBadRequest) return } if channel.IsGroupConstrained() { - nonMembers, err := c.App.FilterNonGroupChannelMembers([]string{user.Id}, channel) + nonMembers, err := c.App.FilterNonGroupChannelMembers([]string{member.UserId}, channel) if err != nil { if v, ok := err.(*model.AppError); ok { c.Err = v } else { - c.Err = model.NewAppError("addChannelMember", "api.channel.add_members.error", nil, err.Error(), http.StatusBadRequest) + c.Err = model.NewAppError("localAddChannelMember", "api.channel.add_members.error", nil, err.Error(), http.StatusBadRequest) } return } if len(nonMembers) > 0 { - c.Err = model.NewAppError("addChannelMember", "api.channel.add_members.user_denied", map[string]interface{}{"UserIDs": nonMembers}, "", http.StatusBadRequest) + c.Err = model.NewAppError("localAddChannelMember", "api.channel.add_members.user_denied", map[string]interface{}{"UserIDs": nonMembers}, "", http.StatusBadRequest) return } } - cm, err := c.App.AddUserToChannel(user, channel, false) + cm, err := c.App.AddChannelMember(c.AppContext, member.UserId, channel, app.ChannelMemberOpts{ + PostRootID: postRootId, + }) if err != nil { c.Err = err return diff --git a/api4/channel_test.go b/api4/channel_test.go index 85968de730..5b49cad815 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -1844,7 +1844,8 @@ func TestConvertChannelToPrivate(t *testing.T) { func TestUpdateChannelPrivacy(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - Client := th.Client + + defaultChannel, _ := th.App.GetChannelByName(model.DEFAULT_CHANNEL, th.BasicTeam.Id, false) type testTable []struct { name string @@ -1852,100 +1853,112 @@ func TestUpdateChannelPrivacy(t *testing.T) { expectedPrivacy string } - defaultChannel, _ := th.App.GetChannelByName(model.DEFAULT_CHANNEL, th.BasicTeam.Id, false) - privateChannel := th.CreatePrivateChannel() - publicChannel := th.CreatePublicChannel() + t.Run("Should get a forbidden response if not logged in", func(t *testing.T) { + privateChannel := th.CreatePrivateChannel() + publicChannel := th.CreatePublicChannel() - tt := testTable{ - {"Updating default channel should fail with forbidden status if not logged in", defaultChannel, model.CHANNEL_OPEN}, - {"Updating private channel should fail with forbidden status if not logged in", privateChannel, model.CHANNEL_PRIVATE}, - {"Updating public channel should fail with forbidden status if not logged in", publicChannel, model.CHANNEL_OPEN}, - } + tt := testTable{ + {"Updating default channel should fail with forbidden status if not logged in", defaultChannel, model.CHANNEL_OPEN}, + {"Updating private channel should fail with forbidden status if not logged in", privateChannel, model.CHANNEL_PRIVATE}, + {"Updating public channel should fail with forbidden status if not logged in", publicChannel, model.CHANNEL_OPEN}, + } - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - _, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) - CheckForbiddenStatus(t, resp) - }) - } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + _, resp := th.Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) + CheckForbiddenStatus(t, resp) + }) + } + }) - th.LoginTeamAdmin() + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + privateChannel := th.CreatePrivateChannel() + publicChannel := th.CreatePublicChannel() - tt = testTable{ - {"Converting default channel to private should fail", defaultChannel, model.CHANNEL_PRIVATE}, - {"Updating privacy to an invalid setting should fail", publicChannel, "invalid"}, - } + tt := testTable{ + {"Converting default channel to private should fail", defaultChannel, model.CHANNEL_PRIVATE}, + {"Updating privacy to an invalid setting should fail", publicChannel, "invalid"}, + } - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - _, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) - CheckBadRequestStatus(t, resp) - }) - } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + _, resp := client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) + CheckBadRequestStatus(t, resp) + }) + } - tt = testTable{ - {"Default channel should stay public", defaultChannel, model.CHANNEL_OPEN}, - {"Public channel should stay public", publicChannel, model.CHANNEL_OPEN}, - {"Private channel should stay private", privateChannel, model.CHANNEL_PRIVATE}, - {"Public channel should convert to private", publicChannel, model.CHANNEL_PRIVATE}, - {"Private channel should convert to public", privateChannel, model.CHANNEL_OPEN}, - } + tt = testTable{ + {"Default channel should stay public", defaultChannel, model.CHANNEL_OPEN}, + {"Public channel should stay public", publicChannel, model.CHANNEL_OPEN}, + {"Private channel should stay private", privateChannel, model.CHANNEL_PRIVATE}, + {"Public channel should convert to private", publicChannel, model.CHANNEL_PRIVATE}, + {"Private channel should convert to public", privateChannel, model.CHANNEL_OPEN}, + } - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - updatedChannel, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) - CheckNoError(t, resp) - assert.Equal(t, tc.expectedPrivacy, updatedChannel.Type) - updatedChannel, err := th.App.GetChannel(tc.channel.Id) - require.Nil(t, err) - assert.Equal(t, tc.expectedPrivacy, updatedChannel.Type) - }) - } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + updatedChannel, resp := client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy) + CheckNoError(t, resp) + assert.Equal(t, tc.expectedPrivacy, updatedChannel.Type) + updatedChannel, err := th.App.GetChannel(tc.channel.Id) + require.Nil(t, err) + assert.Equal(t, tc.expectedPrivacy, updatedChannel.Type) + }) + } + }) t.Run("Enforces convert channel permissions", func(t *testing.T) { + privateChannel := th.CreatePrivateChannel() + publicChannel := th.CreatePublicChannel() + + th.LoginTeamAdmin() + th.RemovePermissionFromRole(model.PERMISSION_CONVERT_PUBLIC_CHANNEL_TO_PRIVATE.Id, model.TEAM_ADMIN_ROLE_ID) th.RemovePermissionFromRole(model.PERMISSION_CONVERT_PRIVATE_CHANNEL_TO_PUBLIC.Id, model.TEAM_ADMIN_ROLE_ID) - _, resp := Client.UpdateChannelPrivacy(publicChannel.Id, model.CHANNEL_PRIVATE) + _, resp := th.Client.UpdateChannelPrivacy(publicChannel.Id, model.CHANNEL_PRIVATE) CheckForbiddenStatus(t, resp) - _, resp = Client.UpdateChannelPrivacy(privateChannel.Id, model.CHANNEL_OPEN) + _, resp = th.Client.UpdateChannelPrivacy(privateChannel.Id, model.CHANNEL_OPEN) CheckForbiddenStatus(t, resp) th.AddPermissionToRole(model.PERMISSION_CONVERT_PUBLIC_CHANNEL_TO_PRIVATE.Id, model.TEAM_ADMIN_ROLE_ID) th.AddPermissionToRole(model.PERMISSION_CONVERT_PRIVATE_CHANNEL_TO_PUBLIC.Id, model.TEAM_ADMIN_ROLE_ID) - _, resp = Client.UpdateChannelPrivacy(privateChannel.Id, model.CHANNEL_OPEN) + _, resp = th.Client.UpdateChannelPrivacy(privateChannel.Id, model.CHANNEL_OPEN) CheckNoError(t, resp) - _, resp = Client.UpdateChannelPrivacy(publicChannel.Id, model.CHANNEL_PRIVATE) + _, resp = th.Client.UpdateChannelPrivacy(publicChannel.Id, model.CHANNEL_PRIVATE) CheckNoError(t, resp) }) - } func TestRestoreChannel(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - Client := th.Client publicChannel1 := th.CreatePublicChannel() - Client.DeleteChannel(publicChannel1.Id) + th.Client.DeleteChannel(publicChannel1.Id) privateChannel1 := th.CreatePrivateChannel() - Client.DeleteChannel(privateChannel1.Id) + th.Client.DeleteChannel(privateChannel1.Id) - _, resp := Client.RestoreChannel(publicChannel1.Id) + _, resp := th.Client.RestoreChannel(publicChannel1.Id) CheckForbiddenStatus(t, resp) - _, resp = Client.RestoreChannel(privateChannel1.Id) + _, resp = th.Client.RestoreChannel(privateChannel1.Id) CheckForbiddenStatus(t, resp) - th.LoginTeamAdmin() + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + defer func() { + client.DeleteChannel(publicChannel1.Id) + client.DeleteChannel(privateChannel1.Id) + }() - _, resp = Client.RestoreChannel(publicChannel1.Id) - CheckOKStatus(t, resp) + _, resp = client.RestoreChannel(publicChannel1.Id) + CheckOKStatus(t, resp) - _, resp = Client.RestoreChannel(privateChannel1.Id) - CheckOKStatus(t, resp) + _, resp = client.RestoreChannel(privateChannel1.Id) + CheckOKStatus(t, resp) + }) } func TestGetChannelByName(t *testing.T) { @@ -2052,7 +2065,6 @@ func TestGetChannelMembers(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { - members, resp := client.GetChannelMembers(th.BasicChannel.Id, 0, 60, "") CheckNoError(t, resp) require.Len(t, *members, 3, "should only be 3 users in channel") diff --git a/app/app.go b/app/app.go index 365b0f4be9..56072e50d2 100644 --- a/app/app.go +++ b/app/app.go @@ -237,6 +237,17 @@ func (a *App) getWarnMetricStatusAndDisplayTextsForId(warnMetricId string, T i18 //nolint:golint,unused,deadcode func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId string, isE0Edition bool) *model.AppError { + // get warn metrics bot + warnMetricsBot, err := a.GetWarnMetricsBot() + if err != nil { + return err + } + + warnMetric, ok := model.WarnMetricsTable[warnMetricId] + if !ok { + return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.invalid_metric.app_error", nil, "", http.StatusInternalServerError) + } + perPage := 25 userOptions := &model.UserGetOptions{ Page: 0, @@ -264,30 +275,12 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st } } - T := i18n.GetUserTranslations(sysAdmins[0].Locale) - warnMetricsBot := &model.Bot{ - Username: model.BOT_WARN_METRIC_BOT_USERNAME, - DisplayName: T("app.system.warn_metric.bot_displayname"), - Description: "", - OwnerId: sysAdmins[0].Id, - } - - bot, err := a.getOrCreateWarnMetricsBot(warnMetricsBot) - if err != nil { - return err - } - - warnMetric, ok := model.WarnMetricsTable[warnMetricId] - if !ok { - return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.invalid_metric.app_error", nil, "", http.StatusInternalServerError) - } - for _, sysAdmin := range sysAdmins { T := i18n.GetUserTranslations(sysAdmin.Locale) - bot.DisplayName = T("app.system.warn_metric.bot_displayname") - bot.Description = T("app.system.warn_metric.bot_description") + warnMetricsBot.DisplayName = T("app.system.warn_metric.bot_displayname") + warnMetricsBot.Description = T("app.system.warn_metric.bot_description") - channel, appErr := a.GetOrCreateDirectChannel(c, bot.UserId, sysAdmin.Id) + channel, appErr := a.GetOrCreateDirectChannel(c, warnMetricsBot.UserId, sysAdmin.Id) if appErr != nil { return appErr } @@ -298,7 +291,7 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st } botPost := &model.Post{ - UserId: bot.UserId, + UserId: warnMetricsBot.UserId, ChannelId: channel.Id, Type: model.POST_SYSTEM_WARN_METRIC_STATUS, Message: "", @@ -334,7 +327,7 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st }, Integration: &model.PostActionIntegration{ Context: model.StringInterface{ - "bot_user_id": bot.UserId, + "bot_user_id": warnMetricsBot.UserId, "force_ack": false, }, URL: postActionUrl, diff --git a/app/app_iface.go b/app/app_iface.go index c5f2d953c0..25744c5757 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -382,6 +382,8 @@ type AppIface interface { VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError //GetUserStatusesByIds used by apiV4 GetUserStatusesByIds(userIDs []string) ([]*model.Status, *model.AppError) + //nolint:golint,unused,deadcode + GetWarnMetricsBot() (*model.Bot, *model.AppError) AccountMigration() einterfaces.AccountMigrationInterface ActivateMfa(userID, token string) *model.AppError AddChannelsToRetentionPolicy(policyID string, channelIDs []string) *model.AppError @@ -736,6 +738,7 @@ type AppIface interface { GetStatusFromCache(userID string) *model.Status GetStatusesByIds(userIDs []string) (map[string]interface{}, *model.AppError) GetSubscriptionStats() (*model.SubscriptionStats, *model.AppError) + GetSystemBot() (*model.Bot, *model.AppError) GetTeam(teamID string) (*model.Team, *model.AppError) GetTeamByInviteId(inviteId string) (*model.Team, *model.AppError) GetTeamByName(name string) (*model.Team, *model.AppError) diff --git a/app/bot.go b/app/bot.go index ecba1f1ca5..6df5db4568 100644 --- a/app/bot.go +++ b/app/bot.go @@ -91,7 +91,65 @@ func (a *App) CreateBot(c *request.Context, bot *model.Bot) (*model.Bot, *model. } //nolint:golint,unused,deadcode -func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.AppError) { +func (a *App) GetWarnMetricsBot() (*model.Bot, *model.AppError) { + perPage := 1 + userOptions := &model.UserGetOptions{ + Page: 0, + PerPage: perPage, + Role: model.SYSTEM_ADMIN_ROLE_ID, + Inactive: false, + } + + sysAdminList, err := a.GetUsers(userOptions) + if err != nil { + return nil, err + } + + if len(sysAdminList) == 0 { + return nil, model.NewAppError("GetWarnMetricsBot", "app.bot.get_warn_metrics_bot.empty_admin_list.app_error", nil, "", http.StatusInternalServerError) + } + + T := i18n.GetUserTranslations(sysAdminList[0].Locale) + warnMetricsBot := &model.Bot{ + Username: model.BOT_WARN_METRIC_BOT_USERNAME, + DisplayName: T("app.system.warn_metric.bot_displayname"), + Description: "", + OwnerId: sysAdminList[0].Id, + } + + return a.getOrCreateBot(warnMetricsBot) +} + +func (a *App) GetSystemBot() (*model.Bot, *model.AppError) { + perPage := 1 + userOptions := &model.UserGetOptions{ + Page: 0, + PerPage: perPage, + Role: model.SYSTEM_ADMIN_ROLE_ID, + Inactive: false, + } + + sysAdminList, err := a.GetUsers(userOptions) + if err != nil { + return nil, err + } + + if len(sysAdminList) == 0 { + return nil, model.NewAppError("GetSystemBot", "app.bot.get_system_bot.empty_admin_list.app_error", nil, "", http.StatusInternalServerError) + } + + T := i18n.GetUserTranslations(sysAdminList[0].Locale) + systemBot := &model.Bot{ + Username: model.BOT_SYSTEM_BOT_USERNAME, + DisplayName: T("app.system.system_bot.bot_displayname"), + Description: "", + OwnerId: sysAdminList[0].Id, + } + + return a.getOrCreateBot(systemBot) +} + +func (a *App) getOrCreateBot(botDef *model.Bot) (*model.Bot, *model.AppError) { botUser, appErr := a.GetUserByUsername(botDef.Username) if appErr != nil { if appErr.StatusCode != http.StatusNotFound { @@ -116,9 +174,9 @@ func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.A default: code = "app.user.save.existing.app_error" } - return nil, model.NewAppError("getOrCreateWarnMetricsBot", code, nil, invErr.Error(), http.StatusBadRequest) + return nil, model.NewAppError("getOrCreateBot", code, nil, invErr.Error(), http.StatusBadRequest) default: - return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.user.save.app_error", nil, nErr.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("getOrCreateBot", "app.user.save.app_error", nil, nErr.Error(), http.StatusInternalServerError) } } botDef.UserId = user.Id @@ -132,14 +190,14 @@ func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.A case errors.As(nErr, &nAppErr): // in case we haven't converted to plain error. return nil, nAppErr default: // last fallback in case it doesn't map to an existing app error. - return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.bot.createbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("getOrCreateBot", "app.bot.createbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) } } return savedBot, nil } if botUser == nil { - return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.bot.createbot.internal_error", nil, "", http.StatusInternalServerError) + return nil, model.NewAppError("getOrCreateBot", "app.bot.createbot.internal_error", nil, "", http.StatusInternalServerError) } //return the bot for this user diff --git a/app/bot_test.go b/app/bot_test.go index d0d333ab36..cd4d09a1c2 100644 --- a/app/bot_test.go +++ b/app/bot_test.go @@ -915,6 +915,44 @@ func TestDeleteBotIconImage(t *testing.T) { }) } +func TestGetSystemBot(t *testing.T) { + t.Run("An error should be returned if there are no sysadmins in the instance", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + require.Nil(t, th.App.PermanentDeleteAllUsers(th.Context)) + + _, err := th.App.GetSystemBot() + require.NotNil(t, err) + require.Equal(t, "app.bot.get_system_bot.empty_admin_list.app_error", err.Id) + }) + + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("The bot should be created the first time it's retrieved", func(t *testing.T) { + // assert no bot with username exists + _, err := th.App.GetUserByUsername(model.BOT_SYSTEM_BOT_USERNAME) + require.NotNil(t, err) + + bot, err := th.App.GetSystemBot() + require.Nil(t, err) + require.Equal(t, bot.Username, model.BOT_SYSTEM_BOT_USERNAME) + }) + + t.Run("The bot should be correctly retrieved if it exists already", func(t *testing.T) { + // assert that the bot is now present + botUser, err := th.App.GetUserByUsername(model.BOT_SYSTEM_BOT_USERNAME) + require.Nil(t, err) + require.True(t, botUser.IsBot) + + bot, err := th.App.GetSystemBot() + require.Nil(t, err) + require.Equal(t, bot.Username, model.BOT_SYSTEM_BOT_USERNAME) + require.Equal(t, bot.UserId, botUser.Id) + }) +} + func sToP(s string) *string { return &s } diff --git a/app/channel.go b/app/channel.go index 7908b8127a..055491cf35 100644 --- a/app/channel.go +++ b/app/channel.go @@ -710,6 +710,21 @@ func (a *App) UpdateChannelPrivacy(c *request.Context, oldChannel *model.Channel } func (a *App) postChannelPrivacyMessage(c *request.Context, user *model.User, channel *model.Channel) *model.AppError { + var authorId string + var authorUsername string + if user != nil { + authorId = user.Id + authorUsername = user.Username + } else { + systemBot, err := a.GetSystemBot() + if err != nil { + return model.NewAppError("postChannelPrivacyMessage", "api.channel.post_channel_privacy_message.error", nil, err.Error(), http.StatusInternalServerError) + } + + authorId = systemBot.UserId + authorUsername = systemBot.Username + } + message := (map[string]string{ model.CHANNEL_OPEN: i18n.T("api.channel.change_channel_privacy.private_to_public"), model.CHANNEL_PRIVATE: i18n.T("api.channel.change_channel_privacy.public_to_private"), @@ -718,9 +733,9 @@ func (a *App) postChannelPrivacyMessage(c *request.Context, user *model.User, ch ChannelId: channel.Id, Message: message, Type: model.POST_CHANGE_CHANNEL_PRIVACY, - UserId: user.Id, + UserId: authorId, Props: model.StringInterface{ - "username": user.Username, + "username": authorUsername, }, } @@ -746,14 +761,18 @@ func (a *App) RestoreChannel(c *request.Context, channel *model.Channel, userID message.Add("channel_id", channel.Id) a.Publish(message) - user, nErr := a.Srv().Store.User().Get(context.Background(), userID) - if nErr != nil { - var nfErr *store.ErrNotFound - switch { - case errors.As(nErr, &nfErr): - return nil, model.NewAppError("RestoreChannel", MissingAccountError, nil, nfErr.Error(), http.StatusNotFound) - default: - return nil, model.NewAppError("RestoreChannel", "app.user.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) + var user *model.User + if userID != "" { + var nErr error + user, nErr = a.Srv().Store.User().Get(context.Background(), userID) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("RestoreChannel", MissingAccountError, nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("RestoreChannel", "app.user.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } } } @@ -773,6 +792,28 @@ func (a *App) RestoreChannel(c *request.Context, channel *model.Channel, userID if _, err := a.CreatePost(c, post, channel, false, true); err != nil { mlog.Warn("Failed to post unarchive message", mlog.Err(err)) } + } else { + a.Srv().Go(func() { + systemBot, err := a.GetSystemBot() + if err != nil { + mlog.Error("Failed to post unarchive message", mlog.Err(err)) + return + } + + post := &model.Post{ + ChannelId: channel.Id, + Message: i18n.T("api.channel.restore_channel.unarchived", map[string]interface{}{"Username": systemBot.Username}), + Type: model.POST_CHANNEL_RESTORED, + UserId: systemBot.UserId, + Props: model.StringInterface{ + "username": systemBot.Username, + }, + } + + if _, err := a.CreatePost(c, post, channel, false, true); err != nil { + mlog.Error("Failed to post unarchive message", mlog.Err(err)) + } + }) } return channel, nil @@ -1297,6 +1338,28 @@ func (a *App) DeleteChannel(c *request.Context, channel *model.Channel, userID s if _, err := a.CreatePost(c, post, channel, false, true); err != nil { mlog.Warn("Failed to post archive message", mlog.Err(err)) } + } else { + a.Srv().Go(func() { + systemBot, err := a.GetSystemBot() + if err != nil { + mlog.Error("Failed to post archive message", mlog.Err(err)) + return + } + + post := &model.Post{ + ChannelId: channel.Id, + Message: fmt.Sprintf(i18n.T("api.channel.delete_channel.archived"), systemBot.Username), + Type: model.POST_CHANNEL_DELETED, + UserId: systemBot.UserId, + Props: model.StringInterface{ + "username": systemBot.Username, + }, + } + + if _, err := a.CreatePost(c, post, channel, false, true); err != nil { + mlog.Error("Failed to post archive message", mlog.Err(err)) + } + }) } now := model.GetMillis() @@ -1468,7 +1531,9 @@ func (a *App) AddChannelMember(c *request.Context, userID string, channel *model } if opts.UserRequestorID == "" || userID == opts.UserRequestorID { - a.postJoinChannelMessage(c, user, channel) + if err := a.postJoinChannelMessage(c, user, channel); err != nil { + mlog.Error("Failed to post join channel message", mlog.Err(err)) + } } else { a.Srv().Go(func() { a.PostAddToChannelMessage(c, userRequestor, user, channel, opts.PostRootID) @@ -2179,6 +2244,16 @@ func (a *App) postAddToTeamMessage(c *request.Context, user *model.User, addedUs } func (a *App) postRemoveFromChannelMessage(c *request.Context, removerUserId string, removedUser *model.User, channel *model.Channel) *model.AppError { + messageUserId := removerUserId + if messageUserId == "" { + systemBot, err := a.GetSystemBot() + if err != nil { + return model.NewAppError("postRemoveFromChannelMessage", "api.channel.post_user_add_remove_message_and_forget.error", nil, err.Error(), http.StatusInternalServerError) + } + + messageUserId = systemBot.UserId + } + post := &model.Post{ ChannelId: channel.Id, // Message here embeds `@username`, not just `username`, to ensure that mentions @@ -2186,7 +2261,7 @@ func (a *App) postRemoveFromChannelMessage(c *request.Context, removerUserId str // The client renders its own system message, ignoring this value altogether. Message: fmt.Sprintf(i18n.T("api.channel.remove_member.removed"), fmt.Sprintf("@%s", removedUser.Username)), Type: model.POST_REMOVE_FROM_CHANNEL, - UserId: removerUserId, + UserId: messageUserId, Props: model.StringInterface{ "removedUserId": removedUser.Id, "removedUsername": removedUser.Username, @@ -2308,7 +2383,9 @@ func (a *App) RemoveUserFromChannel(c *request.Context, userIDToRemove string, r } } else { a.Srv().Go(func() { - a.postRemoveFromChannelMessage(c, removerUserId, user, channel) + if err := a.postRemoveFromChannelMessage(c, removerUserId, user, channel); err != nil { + mlog.Error("Failed to post user removal message", mlog.Err(err)) + } }) } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 0312805f39..2d360d8198 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -8809,6 +8809,28 @@ func (a *OpenTracingAppLayer) GetSuggestions(c *request.Context, commandArgs *mo return resultVar0 } +func (a *OpenTracingAppLayer) GetSystemBot() (*model.Bot, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetSystemBot") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetSystemBot() + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetTeam(teamID string) (*model.Team, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetTeam") @@ -10136,6 +10158,28 @@ func (a *OpenTracingAppLayer) GetViewUsersRestrictions(userID string) (*model.Vi return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) GetWarnMetricsBot() (*model.Bot, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetWarnMetricsBot") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetWarnMetricsBot() + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetWarnMetricsStatus() (map[string]*model.WarnMetricStatus, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetWarnMetricsStatus") diff --git a/i18n/en.json b/i18n/en.json index 6b1cc7f6cf..3378d5840b 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4366,6 +4366,14 @@ "id": "app.bot.get_disable_bot_sysadmin_message", "translation": "{{if .disableBotsSetting}}{{if .printAllBots}}{{.UserName}} was deactivated. They managed the following bot accounts which have now been disabled.\n\n{{.BotNames}}{{else}}{{.UserName}} was deactivated. They managed {{.NumBots}} bot accounts which have now been disabled, including the following:\n\n{{.BotNames}}{{end}}You can take ownership of each bot by enabling it at **Integrations > Bot Accounts** and creating new tokens for the bot.\n\nFor more information, see our [documentation](https://docs.mattermost.com/developer/bot-accounts.html#what-happens-when-a-user-who-owns-bot-accounts-is-disabled).{{else}}{{if .printAllBots}}{{.UserName}} was deactivated. They managed the following bot accounts which are still enabled.\n\n{{.BotNames}}\n{{else}}{{.UserName}} was deactivated. They managed {{.NumBots}} bot accounts which are still enabled, including the following:\n\n{{.BotNames}}{{end}}We strongly recommend you to take ownership of each bot by re-enabling it at **Integrations > Bot Accounts** and creating new tokens for the bot.\n\nFor more information, see our [documentation](https://docs.mattermost.com/developer/bot-accounts.html#what-happens-when-a-user-who-owns-bot-accounts-is-disabled).\n\nIf you want bot accounts to disable automatically after owner deactivation, set “Disable bot accounts when owner is deactivated” in **System Console > Integrations > Bot Accounts** to true.{{end}}" }, + { + "id": "app.bot.get_system_bot.empty_admin_list.app_error", + "translation": "List of admins is empty." + }, + { + "id": "app.bot.get_warn_metrics_bot.empty_admin_list.app_error", + "translation": "List of admins is empty." + }, { "id": "app.bot.getbot.internal_error", "translation": "Unable to get the bot." @@ -6018,6 +6026,10 @@ "id": "app.system.save.app_error", "translation": "We encountered an error saving the system property." }, + { + "id": "app.system.system_bot.bot_displayname", + "translation": "System" + }, { "id": "app.system.warn_metric.bot_description", "translation": "[Learn more about the Mattermost Advisor](https://about.mattermost.com/default-channel-handle-documentation)" diff --git a/model/bot.go b/model/bot.go index 23d1c5b7d0..b193fc3d55 100644 --- a/model/bot.go +++ b/model/bot.go @@ -17,6 +17,7 @@ const ( BOT_DESCRIPTION_MAX_RUNES = 1024 BOT_CREATOR_ID_MAX_RUNES = KEY_VALUE_PLUGIN_ID_MAX_RUNES // UserId or PluginId BOT_WARN_METRIC_BOT_USERNAME = "mattermost-advisor" + BOT_SYSTEM_BOT_USERNAME = "system-bot" ) // Bot is a special type of User meant for programmatic interactions.