opensource/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-08-30 17:06:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

[MM-69183] Gate expensive user/guest count queries behind admin check in getServerLimits (#36999) (#37161)

Browse Source 
Automatic Merge
This commit is contained in:
Mattermost Build
2026-06-25 08:05:18 +02:00
committed by GitHub
parent 3fbf151b6c
commit 3e198e4d55
4 changed files with 73 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/channels/api4/limits.go
+4 -1
View File
@@ -19,7 +19,10 @@ func (api *API) InitLimits() {
func getServerLimits(c *Context, w http.ResponseWriter, r *http.Request) {
isAdmin := c.IsSystemAdmin() && c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleReadUserManagementUsers)
serverLimits, err := c.App.GetServerLimits()
// Only admins receive (and need) the user/guest counts, so only compute them for
// admins. This keeps the expensive count queries off the per-login/per-refresh hot
// path that non-admin clients hit via loadMe()/loadConfigAndMe().
serverLimits, err := c.App.GetServerLimits(isAdmin)
if err != nil {
c.Err = err
return
server/channels/app/limits.go
+15 -2
View File
@@ -14,7 +14,14 @@ const (
maxUsersHardLimit = 250
)
func (a *App) GetServerLimits() (*model.ServerLimits, *model.AppError) {
// GetServerLimits returns the server's seat/post-history limits. The license-derived
// limit fields and post-history fields are always computed (they are cheap and needed
// by all users). The active user and single-channel guest counts are only computed when
// includeUserCounts is true, because those queries are expensive and the counts are only
// consumed by admin-gated UI and internal seat-limit checks. Callers that do not need the
// counts (e.g. non-admin API requests) should pass false to keep the expensive queries off
// the hot path.
func (a *App) GetServerLimits(includeUserCounts bool) (*model.ServerLimits, *model.AppError) {
limits := &model.ServerLimits{}
license := a.License()
@@ -47,6 +54,12 @@ func (a *App) GetServerLimits() (*model.ServerLimits, *model.AppError) {
limits.LastAccessiblePostTime = lastAccessibleTime
}
// The user/guest count queries are expensive (the single-channel guest count is a
// full ChannelMembers scan). Only run them when the caller actually needs the counts.
if !includeUserCounts {
return limits, nil
}
activeUserCount, appErr := a.Srv().Store().User().Count(model.UserCountOptions{})
if appErr != nil {
return nil, model.NewAppError("GetServerLimits", "app.limits.get_app_limits.user_count.store_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
@@ -99,7 +112,7 @@ func (a *App) GetPostHistoryLimit() int64 {
}
func (a *App) isAtUserLimit() (bool, *model.AppError) {
userLimits, appErr := a.GetServerLimits()
userLimits, appErr := a.GetServerLimits(true)
if appErr != nil {
return false, appErr
}
server/channels/app/limits_test.go
+52 -32
View File
@@ -22,7 +22,7 @@ func TestGetServerLimits(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// InitBasic creates 3 users by default
@@ -31,25 +31,45 @@ func TestGetServerLimits(t *testing.T) {
require.Equal(t, int64(250), serverLimits.MaxUsersHardLimit)
})
t.Run("user counts are skipped when includeUserCounts is false", func(t *testing.T) {
th := Setup(t).InitBasic(t)
th.App.Srv().SetLicense(nil)
// With counts included we get the real active user count (InitBasic creates 3 users).
withCounts, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), withCounts.ActiveUserCount)
// Without counts the expensive count queries are skipped, so the count is zero even
// though users exist. The cheap license-derived limits are still returned.
withoutCounts, appErr := th.App.GetServerLimits(false)
require.Nil(t, appErr)
require.Equal(t, int64(0), withoutCounts.ActiveUserCount)
require.Equal(t, int64(0), withoutCounts.SingleChannelGuestCount)
require.Equal(t, int64(200), withoutCounts.MaxUsersLimit)
require.Equal(t, int64(250), withoutCounts.MaxUsersHardLimit)
})
t.Run("user count should increase on creating new user and decrease on permanently deleting", func(t *testing.T) {
th := Setup(t).InitBasic(t)
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we create a new user
newUser := th.CreateUser(t)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(4), serverLimits.ActiveUserCount)
// now we'll delete the user
_ = th.App.PermanentDeleteUser(th.Context, newUser)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
})
@@ -59,20 +79,20 @@ func TestGetServerLimits(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we create a new user
newGuestUser := th.CreateGuest(t)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(4), serverLimits.ActiveUserCount)
// now we'll delete the user
_ = th.App.PermanentDeleteUser(th.Context, newGuestUser)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
})
@@ -82,21 +102,21 @@ func TestGetServerLimits(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we create a new user
newUser := th.CreateUser(t)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(4), serverLimits.ActiveUserCount)
// now we'll delete the user
_, appErr = th.App.UpdateActive(th.Context, newUser, false)
require.Nil(t, appErr)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
})
@@ -106,21 +126,21 @@ func TestGetServerLimits(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we create a new user
newGuestUser := th.CreateGuest(t)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(4), serverLimits.ActiveUserCount)
// now we'll delete the user
_, appErr = th.App.UpdateActive(th.Context, newGuestUser, false)
require.Nil(t, appErr)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
})
@@ -130,20 +150,20 @@ func TestGetServerLimits(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we create a new bot
newBot := th.CreateBot(t)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
// now we'll delete the bot
_ = th.App.PermanentDeleteBot(th.Context, newBot.UserId)
serverLimits, appErr = th.App.GetServerLimits()
serverLimits, appErr = th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(3), serverLimits.ActiveUserCount)
})
@@ -155,7 +175,7 @@ func TestGetServerLimits(t *testing.T) {
license.IsSeatCountEnforced = false
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Greater(t, serverLimits.ActiveUserCount, int64(0))
@@ -174,7 +194,7 @@ func TestGetServerLimits(t *testing.T) {
license.ExtraUsers = &extraUsers
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// InitBasic creates 3 users by default
@@ -193,7 +213,7 @@ func TestGetServerLimits(t *testing.T) {
license.ExtraUsers = nil // Not configured
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// InitBasic creates 3 users by default
@@ -213,7 +233,7 @@ func TestGetServerLimits(t *testing.T) {
license.ExtraUsers = &extraUsers
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// InitBasic creates 3 users by default
@@ -230,7 +250,7 @@ func TestGetServerLimits(t *testing.T) {
license.Features.Users = nil
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Greater(t, serverLimits.ActiveUserCount, int64(0))
@@ -247,7 +267,7 @@ func TestGetServerLimits(t *testing.T) {
license.Features.Users = &userLimit
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Greater(t, serverLimits.ActiveUserCount, int64(0))
@@ -510,7 +530,7 @@ func TestExtraUsersBehavior(t *testing.T) {
license.ExtraUsers = tt.extraUsers
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, tt.expectedBaseLimit, serverLimits.MaxUsersLimit)
@@ -524,7 +544,7 @@ func TestExtraUsersBehavior(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Unlicensed servers use hard-coded limits without extra users
@@ -541,7 +561,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
th.App.Srv().SetLicense(nil)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Unlicensed servers should have no post history limits
@@ -556,7 +576,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
license.Limits = nil // No limits configured
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Should have no post history limits when Limits is nil
@@ -573,7 +593,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
}
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Should have no post history limits when PostHistory is 0
@@ -606,7 +626,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
}
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Should have proper post history limits set
@@ -637,7 +657,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
}
th.App.Srv().SetLicense(license)
_, appErr := th.App.GetServerLimits()
_, appErr := th.App.GetServerLimits(true)
require.NotNil(t, appErr)
require.Contains(t, appErr.Message, "Unable to find the system variable")
})
@@ -664,7 +684,7 @@ func TestGetServerLimitsWithPostHistory(t *testing.T) {
}
th.App.Srv().SetLicense(license)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
// Should have post history limit set but LastAccessiblePostTime should be 0 (all posts accessible)
@@ -745,7 +765,7 @@ func TestGetServerLimitsWithSingleChannelGuests(t *testing.T) {
th.LinkUserToTeam(t, guest, th.BasicTeam)
th.AddUserToChannel(t, guest, th.BasicChannel)
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Greater(t, serverLimits.SingleChannelGuestCount, int64(0))
@@ -761,7 +781,7 @@ func TestGetServerLimitsWithSingleChannelGuests(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true })
serverLimits, appErr := th.App.GetServerLimits()
serverLimits, appErr := th.App.GetServerLimits(true)
require.Nil(t, appErr)
require.Equal(t, int64(0), serverLimits.SingleChannelGuestCount)
server/channels/app/user.go
+2 -2
View File
@@ -408,7 +408,7 @@ func (a *App) createUserOrGuest(rctx request.CTX, user *model.User, guest bool)
}, plugin.UserHasBeenCreatedID)
})
userLimits, limitErr := a.GetServerLimits()
userLimits, limitErr := a.GetServerLimits(true)
if limitErr != nil {
// we don't want to break the create user flow just because of this.
// So, we log the error, not return
@@ -1225,7 +1225,7 @@ func (a *App) UpdateActive(rctx request.CTX, user *model.User, active bool) (*mo
}
if active {
userLimits, appErr := a.GetServerLimits()
userLimits, appErr := a.GetServerLimits(true)
if appErr != nil {
rctx.Logger().Error("Error fetching user limits in UpdateActive", mlog.Err(appErr))
} else {