[MM-14725] Inform System Admins when a user who managed bot ac… (#11669)
* Add notification, to sysadmins, when a user is disabled and the user owns bots. * If the user doesn't have any bots, there is no need to send a notifcation to sysadmins * Remove comment * Update documention link * Send as System * Query teams for each sysadmin and add first response to hardcoded link. * Remove fmt.print debug statements * remove link with hard-coded team. Will add this as future enhancement Expose GetDisableBotSysadminMessage function so it can be tested Add test new function for testing new notification feature * Fix shadow error * Swap sentences in the message when System Console > Bot Accounts > Disable bot accounts when owner is deactivated: is set to false * Another message correction * Rename Custom Integrations to Integrations * rename botsDisabled variable to disableBotsSetting * - increase the number of bots and sysadmins queried to 1000. - limit the number of bots printed in the post to 10, but mention the total bots owned by the user * Enable translations for sysadmin messages * - Rename function. The actual purpose for the function is to notify sysadmins that a user, that owned bots, was disabled. - convert GetDisableBotSysadminMessage from a function to a method. This allows getting *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated in the method and avoids having to pass the value as an input to a function * fix "make i18n-extract" error. Reorganize .json file * Correct the upper range value to be the minimum of number of elements in the userBots array, or the upper limit (10) * replace t.fatal with require statements fix golangci-lint errors * Create separate message when user managed less than or fewer than 10 bots Test cases for both message types * fix i18n sorting * Using pagination to get the bots and sysadmins instead of setting arbitrary value for PerPage and only retrieving first page * only use one translation ID for the message. push all logic into the template so translators can view the logic add disableBotsSetting and printAllBots variables to the translation map * Break the for loop once len(bots) < perPage value, instead of breaking once there are no bots. This saves one additional call
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
345b0c560a
Коммит
876cf82bad
111
app/bot.go
111
app/bot.go
@@ -198,6 +198,117 @@ func (a *App) disableUserBots(userId string) *model.AppError {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (a *App) notifySysadminsBotOwnerDeactivated(userId string) *model.AppError {
|
||||||
|
perPage := 25
|
||||||
|
botOptions := &model.BotGetOptions{
|
||||||
|
OwnerId: userId,
|
||||||
|
IncludeDeleted: false,
|
||||||
|
OnlyOrphaned: false,
|
||||||
|
Page: 0,
|
||||||
|
PerPage: perPage,
|
||||||
|
}
|
||||||
|
// get owner bots
|
||||||
|
var userBots []*model.Bot
|
||||||
|
for {
|
||||||
|
bots, err := a.GetBots(botOptions)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
userBots = append(userBots, bots...)
|
||||||
|
|
||||||
|
if len(bots) < perPage {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
botOptions.Page += 1
|
||||||
|
}
|
||||||
|
|
||||||
|
// user does not own bots
|
||||||
|
if len(userBots) == 0 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
userOptions := &model.UserGetOptions{
|
||||||
|
Page: 0,
|
||||||
|
PerPage: perPage,
|
||||||
|
Role: model.SYSTEM_ADMIN_ROLE_ID,
|
||||||
|
Inactive: false,
|
||||||
|
}
|
||||||
|
// get sysadmins
|
||||||
|
var sysAdmins []*model.User
|
||||||
|
for {
|
||||||
|
sysAdminsList, err := a.GetUsers(userOptions)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
sysAdmins = append(sysAdmins, sysAdminsList...)
|
||||||
|
|
||||||
|
if len(sysAdminsList) < perPage {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
userOptions.Page += 1
|
||||||
|
}
|
||||||
|
|
||||||
|
// user being disabled
|
||||||
|
user, err := a.GetUser(userId)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// for each sysadmin, notify user that owns bots was disabled
|
||||||
|
for _, sysAdmin := range sysAdmins {
|
||||||
|
channel, appErr := a.GetOrCreateDirectChannel(sysAdmin.Id, sysAdmin.Id)
|
||||||
|
if appErr != nil {
|
||||||
|
return appErr
|
||||||
|
}
|
||||||
|
|
||||||
|
post := &model.Post{
|
||||||
|
UserId: sysAdmin.Id,
|
||||||
|
ChannelId: channel.Id,
|
||||||
|
Message: a.getDisableBotSysadminMessage(user, userBots),
|
||||||
|
Type: model.POST_SYSTEM_GENERIC,
|
||||||
|
}
|
||||||
|
|
||||||
|
_, appErr = a.CreatePost(post, channel, false)
|
||||||
|
if appErr != nil {
|
||||||
|
return appErr
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (a *App) getDisableBotSysadminMessage(user *model.User, userBots model.BotList) string {
|
||||||
|
disableBotsSetting := *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated
|
||||||
|
|
||||||
|
var printAllBots = true
|
||||||
|
numBotsToPrint := len(userBots)
|
||||||
|
|
||||||
|
if numBotsToPrint > 10 {
|
||||||
|
numBotsToPrint = 10
|
||||||
|
printAllBots = false
|
||||||
|
}
|
||||||
|
|
||||||
|
var message, botList string
|
||||||
|
for _, bot := range userBots[:numBotsToPrint] {
|
||||||
|
botList += fmt.Sprintf("* %v\n", bot.Username)
|
||||||
|
}
|
||||||
|
|
||||||
|
T := utils.GetUserTranslations(user.Locale)
|
||||||
|
message = T("app.bot.get_disable_bot_sysadmin_message",
|
||||||
|
map[string]interface{}{
|
||||||
|
"UserName": user.Username,
|
||||||
|
"NumBots": len(userBots),
|
||||||
|
"BotNames": botList,
|
||||||
|
"disableBotsSetting": disableBotsSetting,
|
||||||
|
"printAllBots": printAllBots,
|
||||||
|
})
|
||||||
|
|
||||||
|
return message
|
||||||
|
}
|
||||||
|
|
||||||
// ConvertUserToBot converts a user to bot.
|
// ConvertUserToBot converts a user to bot.
|
||||||
func (a *App) ConvertUserToBot(user *model.User) (*model.Bot, *model.AppError) {
|
func (a *App) ConvertUserToBot(user *model.User) (*model.Bot, *model.AppError) {
|
||||||
return a.Srv.Store.Bot().Save(model.BotFromUser(user))
|
return a.Srv.Store.Bot().Save(model.BotFromUser(user))
|
||||||
|
|||||||
133
app/bot_test.go
133
app/bot_test.go
@@ -575,6 +575,139 @@ func TestDisableUserBots(t *testing.T) {
|
|||||||
require.Nil(t, err)
|
require.Nil(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestNotifySysadminsBotOwnerDisabled(t *testing.T) {
|
||||||
|
th := Setup(t).InitBasic()
|
||||||
|
defer th.TearDown()
|
||||||
|
|
||||||
|
userBots := []*model.Bot{}
|
||||||
|
defer func() {
|
||||||
|
for _, bot := range userBots {
|
||||||
|
th.App.PermanentDeleteBot(bot.UserId)
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
|
// // Create two sysadmins
|
||||||
|
sysadmin1 := model.User{
|
||||||
|
Email: "sys1@example.com",
|
||||||
|
Nickname: "nn_sysadmin1",
|
||||||
|
Password: "hello1",
|
||||||
|
Username: "un_sysadmin1",
|
||||||
|
Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID}
|
||||||
|
_, err := th.App.CreateUser(&sysadmin1)
|
||||||
|
require.Nil(t, err, "failed to create user")
|
||||||
|
th.App.UpdateUserRoles(sysadmin1.Id, model.SYSTEM_USER_ROLE_ID+" "+model.SYSTEM_ADMIN_ROLE_ID, false)
|
||||||
|
|
||||||
|
sysadmin2 := model.User{
|
||||||
|
Email: "sys2@example.com",
|
||||||
|
Nickname: "nn_sysadmin2",
|
||||||
|
Password: "hello1",
|
||||||
|
Username: "un_sysadmin2",
|
||||||
|
Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID}
|
||||||
|
_, err = th.App.CreateUser(&sysadmin2)
|
||||||
|
require.Nil(t, err, "failed to create user")
|
||||||
|
th.App.UpdateUserRoles(sysadmin2.Id, model.SYSTEM_USER_ROLE_ID+" "+model.SYSTEM_ADMIN_ROLE_ID, false)
|
||||||
|
|
||||||
|
// create user to be disabled
|
||||||
|
user1, err := th.App.CreateUser(&model.User{
|
||||||
|
Email: "user1@example.com",
|
||||||
|
Username: "user1_disabled",
|
||||||
|
Nickname: "user1",
|
||||||
|
Password: "Password1",
|
||||||
|
})
|
||||||
|
require.Nil(t, err, "failed to create user")
|
||||||
|
|
||||||
|
// create user that doesn't own any bots
|
||||||
|
user2, err := th.App.CreateUser(&model.User{
|
||||||
|
Email: "user2@example.com",
|
||||||
|
Username: "user2_disabled",
|
||||||
|
Nickname: "user2",
|
||||||
|
Password: "Password1",
|
||||||
|
})
|
||||||
|
require.Nil(t, err, "failed to create user")
|
||||||
|
|
||||||
|
const numBotsToPrint = 10
|
||||||
|
|
||||||
|
// create bots owned by user (equal to numBotsToPrint)
|
||||||
|
var bot *model.Bot
|
||||||
|
for i := 0; i < numBotsToPrint; i++ {
|
||||||
|
bot, err = th.App.CreateBot(&model.Bot{
|
||||||
|
Username: fmt.Sprintf("bot%v", i),
|
||||||
|
Description: "a bot",
|
||||||
|
OwnerId: user1.Id,
|
||||||
|
})
|
||||||
|
require.Nil(t, err)
|
||||||
|
userBots = append(userBots, bot)
|
||||||
|
}
|
||||||
|
assert.Len(t, userBots, 10)
|
||||||
|
|
||||||
|
// get DM channels for sysadmin1 and sysadmin2
|
||||||
|
channelSys1, appErr := th.App.GetOrCreateDirectChannel(sysadmin1.Id, sysadmin1.Id)
|
||||||
|
require.Nil(t, appErr)
|
||||||
|
channelSys2, appErr := th.App.GetOrCreateDirectChannel(sysadmin2.Id, sysadmin2.Id)
|
||||||
|
require.Nil(t, appErr)
|
||||||
|
|
||||||
|
// send notification for user without bots
|
||||||
|
err = th.App.notifySysadminsBotOwnerDeactivated(user2.Id)
|
||||||
|
require.Nil(t, err)
|
||||||
|
|
||||||
|
// get posts from sysadmin1 and sysadmin2 DM channels
|
||||||
|
posts1, err := th.App.GetPosts(channelSys1.Id, 0, 5)
|
||||||
|
require.Nil(t, err)
|
||||||
|
assert.Len(t, posts1.Order, 0)
|
||||||
|
|
||||||
|
posts2, err := th.App.GetPosts(channelSys2.Id, 0, 5)
|
||||||
|
require.Nil(t, err)
|
||||||
|
assert.Len(t, posts2.Order, 0)
|
||||||
|
|
||||||
|
// send notification for user with bots
|
||||||
|
err = th.App.notifySysadminsBotOwnerDeactivated(user1.Id)
|
||||||
|
require.Nil(t, err)
|
||||||
|
|
||||||
|
// get posts from sysadmin1 and sysadmin2 DM channels
|
||||||
|
posts1, err = th.App.GetPosts(channelSys1.Id, 0, 5)
|
||||||
|
require.Nil(t, err)
|
||||||
|
assert.Len(t, posts1.Order, 1)
|
||||||
|
|
||||||
|
posts2, err = th.App.GetPosts(channelSys2.Id, 0, 5)
|
||||||
|
require.Nil(t, err)
|
||||||
|
assert.Len(t, posts2.Order, 1)
|
||||||
|
|
||||||
|
post := posts1.Posts[posts1.Order[0]].Message
|
||||||
|
assert.Equal(t, "user1_disabled was deactivated. They managed the following bot accounts which have now been disabled.\n\n* bot0\n* bot1\n* bot2\n* bot3\n* bot4\n* bot5\n* bot6\n* bot7\n* bot8\n* bot9\nYou 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).", post)
|
||||||
|
|
||||||
|
// print all bots
|
||||||
|
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = true })
|
||||||
|
message := th.App.getDisableBotSysadminMessage(user1, userBots)
|
||||||
|
assert.Equal(t, "user1_disabled was deactivated. They managed the following bot accounts which have now been disabled.\n\n* bot0\n* bot1\n* bot2\n* bot3\n* bot4\n* bot5\n* bot6\n* bot7\n* bot8\n* bot9\nYou 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).", message)
|
||||||
|
|
||||||
|
// print all bots
|
||||||
|
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = false })
|
||||||
|
message = th.App.getDisableBotSysadminMessage(user1, userBots)
|
||||||
|
assert.Equal(t, "user1_disabled was deactivated. They managed the following bot accounts which are still enabled.\n\n* bot0\n* bot1\n* bot2\n* bot3\n* bot4\n* bot5\n* bot6\n* bot7\n* bot8\n* bot9\n\nWe 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 user deactivation, set “Disable bot accounts after user deactivation” in **System Console > Integrations > Bot Accounts** to true.", message)
|
||||||
|
|
||||||
|
// create additional bot to go over the printable limit
|
||||||
|
for i := numBotsToPrint; i < numBotsToPrint+1; i++ {
|
||||||
|
bot, err = th.App.CreateBot(&model.Bot{
|
||||||
|
Username: fmt.Sprintf("bot%v", i),
|
||||||
|
Description: "a bot",
|
||||||
|
OwnerId: user1.Id,
|
||||||
|
})
|
||||||
|
require.Nil(t, err)
|
||||||
|
userBots = append(userBots, bot)
|
||||||
|
}
|
||||||
|
assert.Len(t, userBots, 11)
|
||||||
|
|
||||||
|
// truncate number bots printed
|
||||||
|
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = true })
|
||||||
|
message = th.App.getDisableBotSysadminMessage(user1, userBots)
|
||||||
|
assert.Equal(t, "user1_disabled was deactivated. They managed 11 bot accounts which have now been disabled, including the following:\n\n* bot0\n* bot1\n* bot2\n* bot3\n* bot4\n* bot5\n* bot6\n* bot7\n* bot8\n* bot9\nYou 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).", message)
|
||||||
|
|
||||||
|
// truncate number bots printed
|
||||||
|
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = false })
|
||||||
|
message = th.App.getDisableBotSysadminMessage(user1, userBots)
|
||||||
|
assert.Equal(t, "user1_disabled was deactivated. They managed 11 bot accounts which are still enabled, including the following:\n\n* bot0\n* bot1\n* bot2\n* bot3\n* bot4\n* bot5\n* bot6\n* bot7\n* bot8\n* bot9\nWe 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 user deactivation, set “Disable bot accounts after user deactivation” in **System Console > Integrations > Bot Accounts** to true.", message)
|
||||||
|
}
|
||||||
|
|
||||||
func TestConvertUserToBot(t *testing.T) {
|
func TestConvertUserToBot(t *testing.T) {
|
||||||
t.Run("invalid user", func(t *testing.T) {
|
t.Run("invalid user", func(t *testing.T) {
|
||||||
t.Run("invalid user id", func(t *testing.T) {
|
t.Run("invalid user id", func(t *testing.T) {
|
||||||
|
|||||||
12
app/user.go
12
app/user.go
@@ -950,6 +950,18 @@ func (a *App) userDeactivated(userId string) *model.AppError {
|
|||||||
|
|
||||||
a.SetStatusOffline(userId, false)
|
a.SetStatusOffline(userId, false)
|
||||||
|
|
||||||
|
user, err := a.GetUser(userId)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// when disable a user, userDeactivated is called for the user and the
|
||||||
|
// bots the user owns. Only notify once, when the user is the owner, not the
|
||||||
|
// owners bots
|
||||||
|
if !user.IsBot {
|
||||||
|
a.notifySysadminsBotOwnerDeactivated(userId)
|
||||||
|
}
|
||||||
|
|
||||||
if *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated {
|
if *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated {
|
||||||
a.disableUserBots(userId)
|
a.disableUserBots(userId)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2834,6 +2834,10 @@
|
|||||||
"id": "app.admin.test_site_url.failure",
|
"id": "app.admin.test_site_url.failure",
|
||||||
"translation": "This is not a valid live URL"
|
"translation": "This is not a valid live URL"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"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 user deactivation, set “Disable bot accounts after user deactivation” in **System Console > Integrations > Bot Accounts** to true.{{end}}"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"id": "app.channel.create_channel.no_team_id.app_error",
|
"id": "app.channel.create_channel.no_team_id.app_error",
|
||||||
"translation": "Must specify the team ID to create a channel"
|
"translation": "Must specify the team ID to create a channel"
|
||||||
|
|||||||
@@ -243,6 +243,7 @@ func (o *Post) IsValid(maxPostSize int) *AppError {
|
|||||||
switch o.Type {
|
switch o.Type {
|
||||||
case
|
case
|
||||||
POST_DEFAULT,
|
POST_DEFAULT,
|
||||||
|
POST_SYSTEM_GENERIC,
|
||||||
POST_JOIN_LEAVE,
|
POST_JOIN_LEAVE,
|
||||||
POST_AUTO_RESPONDER,
|
POST_AUTO_RESPONDER,
|
||||||
POST_ADD_REMOVE,
|
POST_ADD_REMOVE,
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user