MM-12393 Server side of bot accounts. (#10378)

* bots model, store and api (#9903)

* bots model, store and api

Fixes: MM-13100, MM-13101, MM-13103, MM-13105, MMM-13119

* uncomment tests incorrectly commented, and fix merge issues

* add etags support

* add missing licenses

* remove unused sqlbuilder.go (for now...)

* rejig permissions

* split out READ_BOTS into READ_BOTS and READ_OTHERS_BOTS, the latter
implicitly allowing the former
* make MANAGE_OTHERS_BOTS imply MANAGE_BOTS

* conform to general rest api pattern

* eliminate redundant http.StatusOK

* Update api4/bot.go

Co-Authored-By: lieut-data <jesse.hallam@gmail.com>

* s/model.UserFromBotModel/model.UserFromBot/g

* Update model/bot.go

Co-Authored-By: lieut-data <jesse.hallam@gmail.com>

* Update model/client4.go

Co-Authored-By: lieut-data <jesse.hallam@gmail.com>

* move sessionHasPermissionToManageBot to app/authorization.go

* use api.ApiSessionRequired for createBot

* introduce BOT_DESCRIPTION_MAX_RUNES constant

* MM-13512 Prevent getting a user by email based on privacy settings (#10021)

* MM-13512 Prevent getting a user by email based on privacy settings

* Add additional config settings to tests

* upgrade db to 5.7 (#10019)

* MM-13526 Add validation when setting a user's Locale field (#10022)

* Fix typos (#10024)

* Fixing first user being created with system admin privilages without being explicity specified. (#10014)

* Revert "Support for Embeded chat (#9129)" (#10017)

This reverts commit 3fcecd521a.

* s/DisableBot/UpdateBotActive

* add permissions on upgrade

* Update NOTICE.txt (#10054)

- add new dependency (text)
- handle switch to forked dependency (go-gomail -> go-mail)
- misc copyright owner updates

* avoid leaking bot knowledge without permission

* [GH-6798] added a new api endpoint to get the bulk reactions for posts (#10049)

* 6798 added a new api to get the bulk reactions for posts

* 6798 added the permsission check before getting the reactions

* GH-6798 added a new app function for the new endpoint

* 6798 added a store method to get reactions for multiple posts

* 6798 connected the app function with the new store function

* 6798 fixed the review comments

* MM-13559 Update model.post.is_valid.file_ids.app_error text per report (#10055)

Ticket: https://mattermost.atlassian.net/browse/MM-13559
Report: https://github.com/mattermost/mattermost-server/issues/10023

* Trigger Login Hooks with OAuth (#10061)

* make BotStore.GetAll deterministic even on duplicate CreateAt

* fix spurious TestMuteCommandSpecificChannel test failure

See
https://community-daily.mattermost.com/core/pl/px9p8s3dzbg1pf3ddrm5cr36uw

* fix race in TestExportUserChannels

* TestExportUserChannels: remove SaveMember call, as it is redundant and used to be silently failing anyway

* MM-13117: bot tokens (#10111)

* eliminate redundant Client/AdminClient declarations

* harden TestUpdateChannelScheme to API failures

* eliminate unnecessary config restoration

* minor cleanup

* make TestGenerateMfaSecret config dependency explicit

* TestCreateUserAccessToken for bots

* TestGetUserAccessToken* for bots

* leverage SessionHasPermissionToUserOrBot for user token APIs

* Test(Revoke|Disable|Enable)UserAccessToken

* make EnableUserAccessTokens explicit, so as to not rely on local config.json

* uncomment TestResetPassword, but still skip

* mark assert(Invalid)Token as helper

* fix whitespace issues

* fix mangled comments

* MM-13116: bot plugin api (#10113)

* MM-13117: expose bot API to plugins

This also changes the `CreatorId` column definition to allow for plugin
ids, as the default unless the plugin overrides is to use the plugin id
here. This branch hasn't hit master yet, so no migration needed.

* gofmt issues

* expunge use of BotList in plugin/client API

* introduce model.BotGetOptions

* use botUserId term for clarity

* MM-13129 Adding functionality to deal with orphaned bots (#10238)

* Add way to list orphaned bots.

* Add /assign route to modify ownership of bot accounts.

* Apply suggestions from code review

Co-Authored-By: crspeller <crspeller@gmail.com>

* MM-13120: add IsBot field to returned user objects (#10103)

* MM-13104: forbid bot login (#10251)

* MM-13104: disallow bot login

* fix shadowing

* MM-13136 Disable user bots when user is disabled. (#10293)

* Disable user bots when user is disabled.

* Grammer.

Co-Authored-By: crspeller <crspeller@gmail.com>

* Fixing bot branch for test changes.

* Don't use external dependancies in bot plugin tests.

* Rename bot CreatorId to OwnerId

* Adding ability to re-enable bots

* Fixing IsBot to not attempt to be saved to DB.

* Adding diagnostics and licencing counting for bot accounts.

* Modifying gorp to allow reading of '-' fields.

* Removing unnessisary nil values from UserCountOptions.

* Changing comment to GoDoc format

* Improving user count SQL

* Some improvments from feedback.

* Omit empty on User.IsBot

app/analytics.go

@@ -19,7 +19,7 @@ const (
 func (a *App) GetAnalytics(name string, teamId string) (model.AnalyticsRows, *model.AppError) {
 	skipIntensiveQueries := false
 	var systemUserCount int64
-	r := <-a.Srv.Store.User().AnalyticsUniqueUserCount("")
+	r := <-a.Srv.Store.User().Count(model.UserCountOptions{})
 	if r.Err != nil {
 		return nil, r.Err
 	}
@@ -53,7 +53,9 @@ func (a *App) GetAnalytics(name string, teamId string) (model.AnalyticsRows, *mo
 		if teamId == "" {
 			userInactiveChan = a.Srv.Store.User().AnalyticsGetInactiveUsersCount()
 		} else {
-			userChan = a.Srv.Store.User().AnalyticsUniqueUserCount(teamId)
+			userChan = a.Srv.Store.User().Count(model.UserCountOptions{
+				TeamId: teamId,
+			})
 		}
 
 		var postChan store.StoreChannel

app/app_test.go

@@ -163,6 +163,11 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
 			model.PERMISSION_CREATE_USER_ACCESS_TOKEN.Id,
 			model.PERMISSION_READ_USER_ACCESS_TOKEN.Id,
 			model.PERMISSION_REVOKE_USER_ACCESS_TOKEN.Id,
+			model.PERMISSION_CREATE_BOT.Id,
+			model.PERMISSION_READ_BOTS.Id,
+			model.PERMISSION_READ_OTHERS_BOTS.Id,
+			model.PERMISSION_MANAGE_BOTS.Id,
+			model.PERMISSION_MANAGE_OTHERS_BOTS.Id,
 			model.PERMISSION_REMOVE_OTHERS_REACTIONS.Id,
 			model.PERMISSION_LIST_TEAM_CHANNELS.Id,
 			model.PERMISSION_JOIN_PUBLIC_CHANNELS.Id,
@@ -331,6 +336,11 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
 			model.PERMISSION_CREATE_USER_ACCESS_TOKEN.Id,
 			model.PERMISSION_READ_USER_ACCESS_TOKEN.Id,
 			model.PERMISSION_REVOKE_USER_ACCESS_TOKEN.Id,
+			model.PERMISSION_CREATE_BOT.Id,
+			model.PERMISSION_READ_BOTS.Id,
+			model.PERMISSION_READ_OTHERS_BOTS.Id,
+			model.PERMISSION_MANAGE_BOTS.Id,
+			model.PERMISSION_MANAGE_OTHERS_BOTS.Id,
 			model.PERMISSION_REMOVE_OTHERS_REACTIONS.Id,
 			model.PERMISSION_LIST_TEAM_CHANNELS.Id,
 			model.PERMISSION_JOIN_PUBLIC_CHANNELS.Id,
@@ -465,6 +475,11 @@ func TestDoEmojisPermissionsMigration(t *testing.T) {
 		model.PERMISSION_CREATE_USER_ACCESS_TOKEN.Id,
 		model.PERMISSION_READ_USER_ACCESS_TOKEN.Id,
 		model.PERMISSION_REVOKE_USER_ACCESS_TOKEN.Id,
+		model.PERMISSION_CREATE_BOT.Id,
+		model.PERMISSION_READ_BOTS.Id,
+		model.PERMISSION_READ_OTHERS_BOTS.Id,
+		model.PERMISSION_MANAGE_BOTS.Id,
+		model.PERMISSION_MANAGE_OTHERS_BOTS.Id,
 		model.PERMISSION_REMOVE_OTHERS_REACTIONS.Id,
 		model.PERMISSION_LIST_TEAM_CHANNELS.Id,
 		model.PERMISSION_JOIN_PUBLIC_CHANNELS.Id,

app/authentication.go

@@ -143,6 +143,10 @@ func (a *App) CheckUserPreflightAuthenticationCriteria(user *model.User, mfaToke
 		return err
 	}
 
+	if err := checkUserNotBot(user); err != nil {
+		return err
+	}
+
 	if err := checkUserLoginAttempts(user, *a.Config().ServiceSettings.MaximumLoginAttempts); err != nil {
 		return err
 	}
@@ -191,6 +195,13 @@ func checkUserNotDisabled(user *model.User) *model.AppError {
 	return nil
 }
 
+func checkUserNotBot(user *model.User) *model.AppError {
+	if user.IsBot {
+		return model.NewAppError("Login", "api.user.login.bot_login_forbidden.app_error", nil, "user_id="+user.Id, http.StatusUnauthorized)
+	}
+	return nil
+}
+
 func (a *App) authenticateUser(user *model.User, password, mfaToken string) (*model.User, *model.AppError) {
 	license := a.License()
 	ldapAvailable := *a.Config().LdapSettings.Enable && a.Ldap != nil && license != nil && *license.Features.LDAP

app/authorization.go

@@ -12,6 +12,10 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
+func (a *App) MakePermissionError(permission *model.Permission) *model.AppError {
+	return model.NewAppError("Permissions", "api.context.permissions.app_error", nil, "userId="+a.Session.UserId+", "+"permission="+permission.Id, http.StatusForbidden)
+}
+
 func (a *App) SessionHasPermissionTo(session model.Session, permission *model.Permission) bool {
 	return a.RolesGrantPermission(session.GetUserRoles(), permission.Id)
 }
@@ -98,6 +102,18 @@ func (a *App) SessionHasPermissionToUser(session model.Session, userId string) b
 	return false
 }
 
+func (a *App) SessionHasPermissionToUserOrBot(session model.Session, userId string) bool {
+	if a.SessionHasPermissionToUser(session, userId) {
+		return true
+	}
+
+	if err := a.SessionHasPermissionToManageBot(session, userId); err == nil {
+		return true
+	}
+
+	return false
+}
+
 func (a *App) HasPermissionTo(askingUserId string, permission *model.Permission) bool {
 	user, err := a.GetUser(askingUserId)
 	if err != nil {
@@ -205,3 +221,35 @@ func (a *App) RolesGrantPermission(roleNames []string, permissionId string) bool
 
 	return false
 }
+
+// SessionHasPermissionToManageBot returns nil if the session has access to manage the given bot.
+// This function deviates from other authorization checks in returning an error instead of just
+// a boolean, allowing the permission failure to be exposed with more granularity.
+func (a *App) SessionHasPermissionToManageBot(session model.Session, botUserId string) *model.AppError {
+	existingBot, err := a.GetBot(botUserId, true)
+	if err != nil {
+		return err
+	}
+
+	if existingBot.OwnerId == session.UserId {
+		if !a.SessionHasPermissionTo(session, model.PERMISSION_MANAGE_BOTS) {
+			if !a.SessionHasPermissionTo(session, model.PERMISSION_READ_BOTS) {
+				// If the user doesn't have permission to read bots, pretend as if
+				// the bot doesn't exist at all.
+				return model.MakeBotNotFoundError(botUserId)
+			}
+			return a.MakePermissionError(model.PERMISSION_MANAGE_BOTS)
+		}
+	} else {
+		if !a.SessionHasPermissionTo(session, model.PERMISSION_MANAGE_OTHERS_BOTS) {
+			if !a.SessionHasPermissionTo(session, model.PERMISSION_READ_OTHERS_BOTS) {
+				// If the user doesn't have permission to read others' bots,
+				// pretend as if the bot doesn't exist at all.
+				return model.MakeBotNotFoundError(botUserId)
+			}
+			return a.MakePermissionError(model.PERMISSION_MANAGE_OTHERS_BOTS)
+		}
+	}
+
+	return nil
+}

app/bot.go

@@ -0,0 +1,180 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"github.com/mattermost/mattermost-server/mlog"
+	"github.com/mattermost/mattermost-server/model"
+)
+
+// CreateBot creates the given bot and corresponding user.
+func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) {
+	result := <-a.Srv.Store.User().Save(model.UserFromBot(bot))
+	if result.Err != nil {
+		return nil, result.Err
+	}
+	bot.UserId = result.Data.(*model.User).Id
+
+	result = <-a.Srv.Store.Bot().Save(bot)
+	if result.Err != nil {
+		<-a.Srv.Store.User().PermanentDelete(bot.UserId)
+		return nil, result.Err
+	}
+
+	return result.Data.(*model.Bot), nil
+}
+
+// PatchBot applies the given patch to the bot and corresponding user.
+func (a *App) PatchBot(botUserId string, botPatch *model.BotPatch) (*model.Bot, *model.AppError) {
+	bot, err := a.GetBot(botUserId, true)
+	if err != nil {
+		return nil, err
+	}
+
+	bot.Patch(botPatch)
+
+	result := <-a.Srv.Store.User().Get(botUserId)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+	user := result.Data.(*model.User)
+
+	patchedUser := model.UserFromBot(bot)
+	user.Id = patchedUser.Id
+	user.Username = patchedUser.Username
+	user.Email = patchedUser.Email
+	user.FirstName = patchedUser.FirstName
+	if result = <-a.Srv.Store.User().Update(user, true); result.Err != nil {
+		return nil, result.Err
+	}
+
+	result = <-a.Srv.Store.Bot().Update(bot)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+
+	return result.Data.(*model.Bot), nil
+}
+
+// GetBot returns the given bot.
+func (a *App) GetBot(botUserId string, includeDeleted bool) (*model.Bot, *model.AppError) {
+	result := <-a.Srv.Store.Bot().Get(botUserId, includeDeleted)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+
+	return result.Data.(*model.Bot), nil
+}
+
+// GetBots returns the requested page of bots.
+func (a *App) GetBots(options *model.BotGetOptions) (model.BotList, *model.AppError) {
+	result := <-a.Srv.Store.Bot().GetAll(options)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+
+	return result.Data.([]*model.Bot), nil
+}
+
+// UpdateBotActive marks a bot as active or inactive, along with its corresponding user.
+func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model.AppError) {
+	result := <-a.Srv.Store.User().Get(botUserId)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+	user := result.Data.(*model.User)
+
+	if _, err := a.UpdateActive(user, active); err != nil {
+		return nil, err
+	}
+
+	result = <-a.Srv.Store.Bot().Get(botUserId, true)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+	bot := result.Data.(*model.Bot)
+
+	changed := true
+	if active && bot.DeleteAt != 0 {
+		bot.DeleteAt = 0
+	} else if !active && bot.DeleteAt == 0 {
+		bot.DeleteAt = model.GetMillis()
+	} else {
+		changed = false
+	}
+
+	if changed {
+		result := <-a.Srv.Store.Bot().Update(bot)
+		if result.Err != nil {
+			return nil, result.Err
+		}
+		bot = result.Data.(*model.Bot)
+	}
+
+	return bot, nil
+}
+
+// PermanentDeleteBot permanently deletes a bot and its corresponding user.
+func (a *App) PermanentDeleteBot(botUserId string) *model.AppError {
+	if result := <-a.Srv.Store.Bot().PermanentDelete(botUserId); result.Err != nil {
+		return result.Err
+	}
+
+	if result := <-a.Srv.Store.User().PermanentDelete(botUserId); result.Err != nil {
+		return result.Err
+	}
+
+	return nil
+}
+
+// UpdateBotOwner changes a bot's owner to the given value
+func (a *App) UpdateBotOwner(botUserId, newOwnerId string) (*model.Bot, *model.AppError) {
+	result := <-a.Srv.Store.Bot().Get(botUserId, true)
+	if result.Err != nil {
+		return nil, result.Err
+	}
+	bot := result.Data.(*model.Bot)
+
+	bot.OwnerId = newOwnerId
+
+	if result = <-a.Srv.Store.Bot().Update(bot); result.Err != nil {
+		return nil, result.Err
+	}
+
+	return result.Data.(*model.Bot), nil
+}
+
+// disableUserBots disables all bots owned by the given user
+func (a *App) disableUserBots(userId string) *model.AppError {
+	perPage := 20
+	for {
+		options := &model.BotGetOptions{
+			OwnerId:        userId,
+			IncludeDeleted: false,
+			OnlyOrphaned:   false,
+			Page:           0,
+			PerPage:        perPage,
+		}
+		userBots, err := a.GetBots(options)
+		if err != nil {
+			return err
+		}
+
+		for _, bot := range userBots {
+			_, err := a.UpdateBotActive(bot.UserId, false)
+			if err != nil {
+				mlog.Error("Unable to deactivate bot.", mlog.String("bot_user_id", bot.UserId), mlog.Err(err))
+			}
+		}
+
+		// Get next set of bots if we got the max number of bots
+		if len(userBots) == perPage {
+			options.Page += 1
+			continue
+		}
+		break
+	}
+
+	return nil
+}

app/bot_test.go

@@ -0,0 +1,549 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost-server/model"
+)
+
+func TestCreateBot(t *testing.T) {
+	t.Run("invalid bot", func(t *testing.T) {
+		t.Run("relative to user", func(t *testing.T) {
+			th := Setup(t).InitBasic()
+			defer th.TearDown()
+
+			_, err := th.App.CreateBot(&model.Bot{
+				Username:    "invalid username",
+				Description: "a bot",
+				OwnerId:     th.BasicUser.Id,
+			})
+			require.NotNil(t, err)
+			require.Equal(t, "model.user.is_valid.username.app_error", err.Id)
+		})
+
+		t.Run("relative to bot", func(t *testing.T) {
+			th := Setup(t).InitBasic()
+			defer th.TearDown()
+
+			_, err := th.App.CreateBot(&model.Bot{
+				Username:    "username",
+				Description: strings.Repeat("x", 1025),
+				OwnerId:     th.BasicUser.Id,
+			})
+			require.NotNil(t, err)
+			require.Equal(t, "model.bot.is_valid.description.app_error", err.Id)
+		})
+	})
+
+	t.Run("create bot", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    "username",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(bot.UserId)
+		assert.Equal(t, "username", bot.Username)
+		assert.Equal(t, "a bot", bot.Description)
+		assert.Equal(t, th.BasicUser.Id, bot.OwnerId)
+	})
+
+	t.Run("create bot, username already used by a non-bot user", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		_, err := th.App.CreateBot(&model.Bot{
+			Username:    th.BasicUser.Username,
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.NotNil(t, err)
+		require.Equal(t, "store.sql_user.save.username_exists.app_error", err.Id)
+	})
+}
+
+func TestPatchBot(t *testing.T) {
+	t.Run("invalid patch for user", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    "username",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(bot.UserId)
+
+		botPatch := &model.BotPatch{
+			Username:    sToP("invalid username"),
+			DisplayName: sToP("an updated bot"),
+			Description: sToP("updated bot"),
+		}
+
+		_, err = th.App.PatchBot(bot.UserId, botPatch)
+		require.NotNil(t, err)
+		require.Equal(t, "model.user.is_valid.username.app_error", err.Id)
+	})
+
+	t.Run("invalid patch for bot", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    "username",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(bot.UserId)
+
+		botPatch := &model.BotPatch{
+			Username:    sToP("username"),
+			DisplayName: sToP("display name"),
+			Description: sToP(strings.Repeat("x", 1025)),
+		}
+
+		_, err = th.App.PatchBot(bot.UserId, botPatch)
+		require.NotNil(t, err)
+		require.Equal(t, "model.bot.is_valid.description.app_error", err.Id)
+	})
+
+	t.Run("patch bot", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot := &model.Bot{
+			Username:    "username",
+			DisplayName: "bot",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		}
+
+		createdBot, err := th.App.CreateBot(bot)
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(createdBot.UserId)
+
+		botPatch := &model.BotPatch{
+			Username:    sToP("username2"),
+			DisplayName: sToP("updated bot"),
+			Description: sToP("an updated bot"),
+		}
+
+		patchedBot, err := th.App.PatchBot(createdBot.UserId, botPatch)
+		require.Nil(t, err)
+
+		createdBot.Username = "username2"
+		createdBot.DisplayName = "updated bot"
+		createdBot.Description = "an updated bot"
+		createdBot.UpdateAt = patchedBot.UpdateAt
+		require.Equal(t, createdBot, patchedBot)
+	})
+
+	t.Run("patch bot, username already used by a non-bot user", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    "username",
+			DisplayName: "bot",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(bot.UserId)
+
+		botPatch := &model.BotPatch{
+			Username: sToP(th.BasicUser2.Username),
+		}
+
+		_, err = th.App.PatchBot(bot.UserId, botPatch)
+		require.NotNil(t, err)
+		require.Equal(t, "store.sql_user.update.username_taken.app_error", err.Id)
+	})
+}
+
+func TestGetBot(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	bot1, err := th.App.CreateBot(&model.Bot{
+		Username:    "username",
+		Description: "a bot",
+		OwnerId:     th.BasicUser.Id,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot1.UserId)
+
+	bot2, err := th.App.CreateBot(&model.Bot{
+		Username:    "username2",
+		Description: "a second bot",
+		OwnerId:     th.BasicUser.Id,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot2.UserId)
+
+	deletedBot, err := th.App.CreateBot(&model.Bot{
+		Username:    "username3",
+		Description: "a deleted bot",
+		OwnerId:     th.BasicUser.Id,
+	})
+	require.Nil(t, err)
+	deletedBot, err = th.App.UpdateBotActive(deletedBot.UserId, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(deletedBot.UserId)
+
+	t.Run("get unknown bot", func(t *testing.T) {
+		_, err := th.App.GetBot(model.NewId(), false)
+		require.NotNil(t, err)
+		require.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
+	})
+
+	t.Run("get bot1", func(t *testing.T) {
+		bot, err := th.App.GetBot(bot1.UserId, false)
+		require.Nil(t, err)
+		assert.Equal(t, bot1, bot)
+	})
+
+	t.Run("get bot2", func(t *testing.T) {
+		bot, err := th.App.GetBot(bot2.UserId, false)
+		require.Nil(t, err)
+		assert.Equal(t, bot2, bot)
+	})
+
+	t.Run("get deleted bot", func(t *testing.T) {
+		_, err := th.App.GetBot(deletedBot.UserId, false)
+		require.NotNil(t, err)
+		require.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
+	})
+
+	t.Run("get deleted bot, include deleted", func(t *testing.T) {
+		bot, err := th.App.GetBot(deletedBot.UserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, deletedBot, bot)
+	})
+}
+
+func TestGetBots(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	OwnerId1 := model.NewId()
+	OwnerId2 := model.NewId()
+
+	bot1, err := th.App.CreateBot(&model.Bot{
+		Username:    "username",
+		Description: "a bot",
+		OwnerId:     OwnerId1,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot1.UserId)
+
+	deletedBot1, err := th.App.CreateBot(&model.Bot{
+		Username:    "username4",
+		Description: "a deleted bot",
+		OwnerId:     OwnerId1,
+	})
+	require.Nil(t, err)
+	deletedBot1, err = th.App.UpdateBotActive(deletedBot1.UserId, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(deletedBot1.UserId)
+
+	bot2, err := th.App.CreateBot(&model.Bot{
+		Username:    "username2",
+		Description: "a second bot",
+		OwnerId:     OwnerId1,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot2.UserId)
+
+	bot3, err := th.App.CreateBot(&model.Bot{
+		Username:    "username3",
+		Description: "a third bot",
+		OwnerId:     OwnerId1,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot3.UserId)
+
+	bot4, err := th.App.CreateBot(&model.Bot{
+		Username:    "username5",
+		Description: "a fourth bot",
+		OwnerId:     OwnerId2,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot4.UserId)
+
+	deletedBot2, err := th.App.CreateBot(&model.Bot{
+		Username:    "username6",
+		Description: "a deleted bot",
+		OwnerId:     OwnerId2,
+	})
+	require.Nil(t, err)
+	deletedBot2, err = th.App.UpdateBotActive(deletedBot2.UserId, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(deletedBot2.UserId)
+
+	t.Run("get bots, page=0, perPage=10", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        "",
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot1, bot2, bot3, bot4}, bots)
+	})
+
+	t.Run("get bots, page=0, perPage=1", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        1,
+			OwnerId:        "",
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot1}, bots)
+	})
+
+	t.Run("get bots, page=1, perPage=2", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           1,
+			PerPage:        2,
+			OwnerId:        "",
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot3, bot4}, bots)
+	})
+
+	t.Run("get bots, page=2, perPage=2", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           2,
+			PerPage:        2,
+			OwnerId:        "",
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{}, bots)
+	})
+
+	t.Run("get bots, page=0, perPage=10, include deleted", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        "",
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot1, deletedBot1, bot2, bot3, bot4, deletedBot2}, bots)
+	})
+
+	t.Run("get bots, page=0, perPage=1, include deleted", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        1,
+			OwnerId:        "",
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot1}, bots)
+	})
+
+	t.Run("get bots, page=1, perPage=2, include deleted", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           1,
+			PerPage:        2,
+			OwnerId:        "",
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot2, bot3}, bots)
+	})
+
+	t.Run("get bots, page=2, perPage=2, include deleted", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           2,
+			PerPage:        2,
+			OwnerId:        "",
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		assert.Equal(t, model.BotList{bot4, deletedBot2}, bots)
+	})
+
+	t.Run("get offset=0, limit=10, creator id 1", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        OwnerId1,
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		require.Equal(t, model.BotList{bot1, bot2, bot3}, bots)
+	})
+
+	t.Run("get offset=0, limit=10, creator id 2", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        OwnerId2,
+			IncludeDeleted: false,
+		})
+		require.Nil(t, err)
+		require.Equal(t, model.BotList{bot4}, bots)
+	})
+
+	t.Run("get offset=0, limit=10, include deleted, creator id 1", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        OwnerId1,
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		require.Equal(t, model.BotList{bot1, deletedBot1, bot2, bot3}, bots)
+	})
+
+	t.Run("get offset=0, limit=10, include deleted, creator id 2", func(t *testing.T) {
+		bots, err := th.App.GetBots(&model.BotGetOptions{
+			Page:           0,
+			PerPage:        10,
+			OwnerId:        OwnerId2,
+			IncludeDeleted: true,
+		})
+		require.Nil(t, err)
+		require.Equal(t, model.BotList{bot4, deletedBot2}, bots)
+	})
+}
+
+func TestUpdateBotActive(t *testing.T) {
+	t.Run("unknown bot", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		_, err := th.App.UpdateBotActive(model.NewId(), false)
+		require.NotNil(t, err)
+		require.Equal(t, "store.sql_user.missing_account.const", err.Id)
+	})
+
+	t.Run("disable/enable bot", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    "username",
+			Description: "a bot",
+			OwnerId:     th.BasicUser.Id,
+		})
+		require.Nil(t, err)
+		defer th.App.PermanentDeleteBot(bot.UserId)
+
+		disabledBot, err := th.App.UpdateBotActive(bot.UserId, false)
+		require.Nil(t, err)
+		require.NotEqual(t, 0, disabledBot.DeleteAt)
+
+		// Disabling should be idempotent
+		disabledBotAgain, err := th.App.UpdateBotActive(bot.UserId, false)
+		require.Nil(t, err)
+		require.Equal(t, disabledBot.DeleteAt, disabledBotAgain.DeleteAt)
+
+		reenabledBot, err := th.App.UpdateBotActive(bot.UserId, true)
+		require.Nil(t, err)
+		require.EqualValues(t, 0, reenabledBot.DeleteAt)
+
+		// Re-enabling should be idempotent
+		reenabledBotAgain, err := th.App.UpdateBotActive(bot.UserId, true)
+		require.Nil(t, err)
+		require.Equal(t, reenabledBot.DeleteAt, reenabledBotAgain.DeleteAt)
+	})
+}
+
+func TestPermanentDeleteBot(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	bot, err := th.App.CreateBot(&model.Bot{
+		Username:    "username",
+		Description: "a bot",
+		OwnerId:     th.BasicUser.Id,
+	})
+	require.Nil(t, err)
+
+	require.Nil(t, th.App.PermanentDeleteBot(bot.UserId))
+
+	_, err = th.App.GetBot(bot.UserId, false)
+	require.NotNil(t, err)
+	require.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
+}
+
+func TestDisableUserBots(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	ownerId1 := model.NewId()
+	ownerId2 := model.NewId()
+
+	bots := []*model.Bot{}
+	defer func() {
+		for _, bot := range bots {
+			th.App.PermanentDeleteBot(bot.UserId)
+		}
+	}()
+
+	for i := 0; i < 46; i++ {
+		bot, err := th.App.CreateBot(&model.Bot{
+			Username:    fmt.Sprintf("username%v", i),
+			Description: "a bot",
+			OwnerId:     ownerId1,
+		})
+		require.Nil(t, err)
+		bots = append(bots, bot)
+	}
+	require.Len(t, bots, 46)
+
+	u2bot1, err := th.App.CreateBot(&model.Bot{
+		Username:    "username_nodisable",
+		Description: "a bot",
+		OwnerId:     ownerId2,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(u2bot1.UserId)
+
+	err = th.App.disableUserBots(ownerId1)
+	require.Nil(t, err)
+
+	// Check all bots and corrensponding users are disabled for creator 1
+	for _, bot := range bots {
+		retbot, err2 := th.App.GetBot(bot.UserId, true)
+		require.Nil(t, err2)
+		require.NotZero(t, retbot.DeleteAt, bot.Username)
+	}
+
+	// Check bots and corresponding user not disabled for creator 2
+	bot, err := th.App.GetBot(u2bot1.UserId, true)
+	require.Nil(t, err)
+	require.Zero(t, bot.DeleteAt)
+
+	user, err := th.App.GetUser(u2bot1.UserId)
+	require.Nil(t, err)
+	require.Zero(t, user.DeleteAt)
+
+	// Bad id doesn't do anything or break horribly
+	err = th.App.disableUserBots(model.NewId())
+	require.Nil(t, err)
+}
+
+func sToP(s string) *string {
+	return &s
+}

app/diagnostics.go

@@ -122,6 +122,7 @@ func pluginActivated(pluginStates map[string]*model.PluginState, pluginId string
 
 func (a *App) trackActivity() {
 	var userCount int64
+	var botAccountsCount int64
 	var activeUsersDailyCount int64
 	var activeUsersMonthlyCount int64
 	var inactiveUserCount int64
@@ -147,10 +148,19 @@ func (a *App) trackActivity() {
 		activeUsersMonthlyCount = r.Data.(int64)
 	}
 
-	if ucr := <-a.Srv.Store.User().GetTotalUsersCount(); ucr.Err == nil {
+	if ucr := <-a.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted: true,
+	}); ucr.Err == nil {
 		userCount = ucr.Data.(int64)
 	}
 
+	if bc := <-a.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeBotAccounts:  true,
+		ExcludeRegularUsers: true,
+	}); bc.Err == nil {
+		botAccountsCount = bc.Data.(int64)
+	}
+
 	if iucr := <-a.Srv.Store.User().AnalyticsGetInactiveUsersCount(); iucr.Err == nil {
 		inactiveUserCount = iucr.Data.(int64)
 	}
@@ -197,6 +207,7 @@ func (a *App) trackActivity() {
 
 	a.SendDiagnostic(TRACK_ACTIVITY, map[string]interface{}{
 		"registered_users":             userCount,
+		"bot_accounts":                 botAccountsCount,
 		"active_users_daily":           activeUsersDailyCount,
 		"active_users_monthly":         activeUsersMonthlyCount,
 		"registered_deactivated_users": inactiveUserCount,
@@ -281,6 +292,7 @@ func (a *App) trackConfig() {
 		"enable_email_invitations":                                *cfg.ServiceSettings.EnableEmailInvitations,
 		"experimental_channel_organization":                       *cfg.ServiceSettings.ExperimentalChannelOrganization,
 		"experimental_ldap_group_sync":                            *cfg.ServiceSettings.ExperimentalLdapGroupSync,
+		"disable_bots_when_owner_is_deactivated":                  *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated,
 	})
 
 	a.SendDiagnostic(TRACK_CONFIG_TEAM, map[string]interface{}{

app/import_functions_test.go

@@ -553,7 +553,10 @@ func TestImportImportUser(t *testing.T) {
 
 	// Check how many users are in the database.
 	var userCount int64
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		userCount = r.Data.(int64)
 	} else {
 		t.Fatalf("Failed to get user count.")
@@ -568,7 +571,10 @@ func TestImportImportUser(t *testing.T) {
 	}
 
 	// Check that no more users are in the DB.
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		if r.Data.(int64) != userCount {
 			t.Fatalf("Unexpected number of users")
 		}
@@ -586,7 +592,10 @@ func TestImportImportUser(t *testing.T) {
 	}
 
 	// Check that no more users are in the DB.
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		if r.Data.(int64) != userCount {
 			t.Fatalf("Unexpected number of users")
 		}
@@ -603,7 +612,10 @@ func TestImportImportUser(t *testing.T) {
 	}
 
 	// Check that no more users are in the DB.
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		if r.Data.(int64) != userCount {
 			t.Fatalf("Unexpected number of users")
 		}
@@ -628,7 +640,10 @@ func TestImportImportUser(t *testing.T) {
 	}
 
 	// Check that one more user is in the DB.
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		if r.Data.(int64) != userCount+1 {
 			t.Fatalf("Unexpected number of users")
 		}
@@ -685,7 +700,10 @@ func TestImportImportUser(t *testing.T) {
 	}
 
 	// Check user count the same.
-	if r := <-th.App.Srv.Store.User().GetTotalUsersCount(); r.Err == nil {
+	if r := <-th.App.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted:     true,
+		IncludeBotAccounts: false,
+	}); r.Err == nil {
 		if r.Data.(int64) != userCount+1 {
 			t.Fatalf("Unexpected number of users")
 		}

app/license.go

@@ -52,7 +52,7 @@ func (a *App) SaveLicense(licenseBytes []byte) (*model.License, *model.AppError)
 	}
 	license := model.LicenseFromJson(strings.NewReader(licenseStr))
 
-	result := <-a.Srv.Store.User().AnalyticsUniqueUserCount("")
+	result := <-a.Srv.Store.User().Count(model.UserCountOptions{})
 	if result.Err != nil {
 		return nil, model.NewAppError("addLicense", "api.license.add_license.invalid_count.app_error", nil, result.Err.Error(), http.StatusBadRequest)
 	}

app/login.go

@@ -57,6 +57,11 @@ func (a *App) AuthenticateUserForLogin(id, loginId, password, mfaToken string, l
 	// then trust the proxy and cert that the correct user is supplied and allow
 	// them access
 	if *a.Config().ExperimentalSettings.ClientSideCertEnable && *a.Config().ExperimentalSettings.ClientSideCertCheck == model.CLIENT_SIDE_CERT_CHECK_PRIMARY_AUTH {
+		// Unless the user is a bot.
+		if err = checkUserNotBot(user); err != nil {
+			return nil, err
+		}
+
 		return user, nil
 	}
 

app/oauth.go

@@ -551,6 +551,13 @@ func (a *App) LoginByOAuth(service string, userData io.Reader, teamId string) (*
 			return nil, err
 		}
 	} else {
+		// OAuth doesn't run through CheckUserPreflightAuthenticationCriteria, so prevent bot login
+		// here manually. Technically, the auth data above will fail to match a bot in the first
+		// place, but explicit is always better.
+		if user.IsBot {
+			return nil, model.NewAppError("loginByOAuth", "api.user.login_by_oauth.bot_login_forbidden.app_error", nil, "", http.StatusForbidden)
+		}
+
 		if err = a.UpdateOAuthUserAttrs(bytes.NewReader(buf.Bytes()), user, provider, service); err != nil {
 			return nil, err
 		}

app/plugin_api.go

@@ -686,3 +686,35 @@ func (api *PluginAPI) LogError(msg string, keyValuePairs ...interface{}) {
 func (api *PluginAPI) LogWarn(msg string, keyValuePairs ...interface{}) {
 	api.logger.Warn(msg, keyValuePairs...)
 }
+
+func (api *PluginAPI) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) {
+	// Bots created by a plugin should use the plugin's ID for the creator field, unless
+	// otherwise specified by the plugin.
+	if bot.OwnerId == "" {
+		bot.OwnerId = api.id
+	}
+
+	return api.app.CreateBot(bot)
+}
+
+func (api *PluginAPI) PatchBot(userId string, botPatch *model.BotPatch) (*model.Bot, *model.AppError) {
+	return api.app.PatchBot(userId, botPatch)
+}
+
+func (api *PluginAPI) GetBot(userId string, includeDeleted bool) (*model.Bot, *model.AppError) {
+	return api.app.GetBot(userId, includeDeleted)
+}
+
+func (api *PluginAPI) GetBots(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) {
+	bots, err := api.app.GetBots(options)
+
+	return []*model.Bot(bots), err
+}
+
+func (api *PluginAPI) UpdateBotActive(userId string, active bool) (*model.Bot, *model.AppError) {
+	return api.app.UpdateBotActive(userId, active)
+}
+
+func (api *PluginAPI) PermanentDeleteBot(userId string) *model.AppError {
+	return api.app.PermanentDeleteBot(userId)
+}

app/plugin_api_test.go

@@ -882,6 +882,151 @@ func TestPluginAPI_SearchTeams(t *testing.T) {
 	})
 }
 
+func TestPluginBots(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	setupPluginApiTest(t,
+		`
+		package main
+
+		import (
+			"github.com/mattermost/mattermost-server/plugin"
+			"github.com/mattermost/mattermost-server/model"
+		)
+
+		type MyPlugin struct {
+			plugin.MattermostPlugin
+		}
+
+		func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) {
+			createdBot, err := p.API.CreateBot(&model.Bot{
+				Username: "bot",
+				Description: "a plugin bot",
+			})
+			if err != nil {
+				return nil, err.Error() + "failed to create bot"
+			}
+
+			fetchedBot, err := p.API.GetBot(createdBot.UserId, false)
+			if err != nil {
+				return nil, err.Error() + "failed to get bot"
+			}
+			if fetchedBot.Description != "a plugin bot" {
+				return nil, "GetBot did not return the expected bot Description"
+			}
+			if fetchedBot.OwnerId != "testpluginbots" {
+				return nil, "GetBot did not return the expected bot OwnerId"
+			}
+
+			updatedDescription := createdBot.Description + ", updated"
+			patchedBot, err := p.API.PatchBot(createdBot.UserId, &model.BotPatch{
+				Description: &updatedDescription,
+			})
+			if err != nil {
+				return nil, err.Error() + "failed to patch bot"
+			}
+
+			fetchedBot, err = p.API.GetBot(patchedBot.UserId, false)
+			if err != nil {
+				return nil, err.Error() + "failed to get bot"
+			}
+
+			if fetchedBot.UserId != patchedBot.UserId {
+				return nil, "GetBot did not return the expected bot"
+			}
+			if fetchedBot.Description != "a plugin bot, updated" {
+				return nil, "GetBot did not return the updated bot Description"
+			}
+
+			fetchedBots, err := p.API.GetBots(&model.BotGetOptions{
+				Page: 0,
+				PerPage: 1,
+				OwnerId: "",
+				IncludeDeleted: false,
+			})
+			if err != nil {
+				return nil, err.Error() + "failed to get bots"
+			}
+
+			if len(fetchedBots) != 1 {
+				return nil, "GetBots did not return a single bot"
+			}
+			if fetchedBot.UserId != fetchedBots[0].UserId {
+				return nil, "GetBots did not return the expected bot"
+			}
+
+			_, err = p.API.UpdateBotActive(fetchedBot.UserId, false)
+			if err != nil {
+				return nil, err.Error() + "failed to disable bot"
+			}
+
+			fetchedBot, err = p.API.GetBot(patchedBot.UserId, false)
+			if err == nil {
+				return nil, "expected not to find disabled bot"
+			}
+
+			_, err = p.API.UpdateBotActive(fetchedBot.UserId, true)
+			if err != nil {
+				return nil, err.Error() + "failed to disable bot"
+			}
+
+			fetchedBot, err = p.API.GetBot(patchedBot.UserId, false)
+			if err != nil {
+				return nil, err.Error() + "failed to get bot after enabling"
+			}
+			if fetchedBot.UserId != patchedBot.UserId {
+				return nil, "GetBot did not return the expected bot after enabling"
+			}
+
+			err = p.API.PermanentDeleteBot(patchedBot.UserId)
+			if err != nil {
+				return nil, err.Error() + "failed to delete bot"
+			}
+
+			_, err = p.API.GetBot(patchedBot.UserId, false)
+			if err == nil {
+				return nil, err.Error() + "found bot after permanently deleting"
+			}
+
+			createdBotWithOverriddenCreator, err := p.API.CreateBot(&model.Bot{
+				Username: "bot",
+				Description: "a plugin bot",
+				OwnerId: "abc123",
+			})
+			if err != nil {
+				return nil, err.Error() + "failed to create bot with overridden creator"
+			}
+
+			fetchedBot, err = p.API.GetBot(createdBotWithOverriddenCreator.UserId, false)
+			if err != nil {
+				return nil, err.Error() + "failed to get bot"
+			}
+			if fetchedBot.Description != "a plugin bot" {
+				return nil, "GetBot did not return the expected bot Description"
+			}
+			if fetchedBot.OwnerId != "abc123" {
+				return nil, "GetBot did not return the expected bot OwnerId"
+			}
+
+			return nil, ""
+		}
+
+		func main() {
+			plugin.ClientMain(&MyPlugin{})
+		}
+		`,
+		`{"id": "testpluginbots", "backend": {"executable": "backend.exe"}}`,
+		"testpluginbots",
+		th.App,
+	)
+
+	hooks, err := th.App.GetPluginsEnvironment().HooksForPlugin("testpluginbots")
+	assert.NoError(t, err)
+	_, errString := hooks.MessageWillBePosted(nil, nil)
+	assert.Empty(t, errString)
+}
+
 func TestPluginAPI_GetTeamMembersForUser(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()

app/security_update_check.go

@@ -63,7 +63,9 @@ func (s *Server) DoSecurityUpdateCheck() {
 					<-s.Store.System().Update(systemSecurityLastTime)
 				}
 
-				if ucr := <-s.Store.User().GetTotalUsersCount(); ucr.Err == nil {
+				if ucr := <-s.Store.User().Count(model.UserCountOptions{
+					IncludeDeleted: true,
+				}); ucr.Err == nil {
 					v.Set(PROP_SECURITY_USER_COUNT, strconv.FormatInt(ucr.Data.(int64), 10))
 				}
 

app/user.go

@@ -171,7 +171,9 @@ func (a *App) IsUserSignUpAllowed() *model.AppError {
 
 func (a *App) IsFirstUserAccount() bool {
 	if a.SessionCacheLength() == 0 {
-		cr := <-a.Srv.Store.User().GetTotalUsersCount()
+		cr := <-a.Srv.Store.User().Count(model.UserCountOptions{
+			IncludeDeleted: true,
+		})
 		if cr.Err != nil {
 			mlog.Error(fmt.Sprint(cr.Err))
 			return false
@@ -195,7 +197,9 @@ func (a *App) CreateUser(user *model.User) (*model.User, *model.AppError) {
 
 	// Below is a special case where the first user in the entire
 	// system is granted the system_admin role
-	result := <-a.Srv.Store.User().GetTotalUsersCount()
+	result := <-a.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeDeleted: true,
+	})
 	if result.Err != nil {
 		return nil, result.Err
 	}
@@ -867,6 +871,40 @@ func (a *App) UpdatePasswordAsUser(userId, currentPassword, newPassword string)
 	return a.UpdatePasswordSendEmail(user, newPassword, T("api.user.update_password.menu"))
 }
 
+func (a *App) userDeactivated(user *model.User) *model.AppError {
+	if err := a.RevokeAllSessions(user.Id); err != nil {
+		return err
+	}
+
+	a.SetStatusOffline(user.Id, false)
+
+	if *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated {
+		a.disableUserBots(user.Id)
+	}
+
+	return nil
+}
+
+func (a *App) invalidateUserChannelMembersCaches(user *model.User) *model.AppError {
+	teamsForUser, err := a.GetTeamsForUser(user.Id)
+	if err != nil {
+		return err
+	}
+
+	for _, team := range teamsForUser {
+		channelsForUser, err := a.GetChannelsForUser(team.Id, user.Id, false)
+		if err != nil {
+			return err
+		}
+
+		for _, channel := range *channelsForUser {
+			a.InvalidateCacheForChannelMembers(channel.Id)
+		}
+	}
+
+	return nil
+}
+
 func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.AppError) {
 	if active {
 		user.DeleteAt = 0
@@ -878,35 +916,16 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A
 	if result.Err != nil {
 		return nil, result.Err
 	}
-
-	if user.DeleteAt > 0 {
-		if err := a.RevokeAllSessions(user.Id); err != nil {
-			return nil, err
-		}
-	}
-
 	ruser := result.Data.([2]*model.User)[0]
 
 	if !active {
-		a.SetStatusOffline(ruser.Id, false)
-	}
-
-	teamsForUser, err := a.GetTeamsForUser(user.Id)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, team := range teamsForUser {
-		channelsForUser, err := a.GetChannelsForUser(team.Id, user.Id, false)
-		if err != nil {
+		if err := a.userDeactivated(ruser); err != nil {
 			return nil, err
 		}
-
-		for _, channel := range *channelsForUser {
-			a.InvalidateCacheForChannelMembers(channel.Id)
-		}
 	}
 
+	a.invalidateUserChannelMembersCaches(user)
+
 	a.sendUpdatedUserEvent(*ruser)
 
 	return ruser, nil
@@ -1500,8 +1519,11 @@ func (a *App) GetVerifyEmailToken(token string) (*model.Token, *model.AppError)
 	return rtoken, nil
 }
 
+// GetTotalUsersStats is used for the DM list total
 func (a *App) GetTotalUsersStats() (*model.UsersStats, *model.AppError) {
-	result := <-a.Srv.Store.User().GetTotalUsersCount()
+	result := <-a.Srv.Store.User().Count(model.UserCountOptions{
+		IncludeBotAccounts: true,
+	})
 	if result.Err != nil {
 		return nil, result.Err
 	}

app/user_test.go

@@ -172,6 +172,51 @@ func TestUpdateUserActive(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestUpdateActiveBotsSideEffect(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	bot, err := th.App.CreateBot(&model.Bot{
+		Username:    "username",
+		Description: "a bot",
+		OwnerId:     th.BasicUser.Id,
+	})
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteBot(bot.UserId)
+
+	// Automatic deactivation disabled
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = false
+	})
+
+	th.App.UpdateActive(th.BasicUser, false)
+
+	retbot1, err := th.App.GetBot(bot.UserId, true)
+	require.Nil(t, err)
+	require.Zero(t, retbot1.DeleteAt)
+	user1, err := th.App.GetUser(bot.UserId)
+	require.Nil(t, err)
+	require.Zero(t, user1.DeleteAt)
+
+	th.App.UpdateActive(th.BasicUser, true)
+
+	// Automatic deactivation enabled
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated = true
+	})
+
+	th.App.UpdateActive(th.BasicUser, false)
+
+	retbot2, err := th.App.GetBot(bot.UserId, true)
+	require.Nil(t, err)
+	require.NotZero(t, retbot2.DeleteAt)
+	user2, err := th.App.GetUser(bot.UserId)
+	require.Nil(t, err)
+	require.NotZero(t, user2.DeleteAt)
+
+	th.App.UpdateActive(th.BasicUser, true)
+}
+
 func TestUpdateOAuthUserAttrs(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()
@@ -292,7 +337,7 @@ func TestUpdateUserEmail(t *testing.T) {
 
 	user := th.CreateUser()
 
-	t.Run("RequireVerification", func(t *testing.T){
+	t.Run("RequireVerification", func(t *testing.T) {
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.EmailSettings.RequireEmailVerification = true
 		})
@@ -318,7 +363,7 @@ func TestUpdateUserEmail(t *testing.T) {
 		assert.True(t, user2.EmailVerified)
 	})
 
-	t.Run("RequireVerificationAlreadyUsedEmail", func(t *testing.T){
+	t.Run("RequireVerificationAlreadyUsedEmail", func(t *testing.T) {
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.EmailSettings.RequireEmailVerification = true
 		})
@@ -332,7 +377,7 @@ func TestUpdateUserEmail(t *testing.T) {
 		assert.Nil(t, user3)
 	})
 
-	t.Run("NoVerification", func(t *testing.T){
+	t.Run("NoVerification", func(t *testing.T) {
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.EmailSettings.RequireEmailVerification = false
 		})
@@ -648,7 +693,7 @@ func TestPasswordRecovery(t *testing.T) {
 	token, err = th.App.CreatePasswordRecoveryToken(th.BasicUser.Id, th.BasicUser.Email)
 	assert.Nil(t, err)
 
-	th.App.UpdateConfig(func (c *model.Config){
+	th.App.UpdateConfig(func(c *model.Config) {
 		*c.EmailSettings.RequireEmailVerification = false
 	})
 
@@ -659,4 +704,3 @@ func TestPasswordRecovery(t *testing.T) {
 	err = th.App.ResetPasswordFromToken(token.Token, "abcdefgh")
 	assert.NotNil(t, err)
 }
-