From afc393d8e999dec05f6ba1b884b5c4e3bba30292 Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Tue, 10 Aug 2021 10:12:43 +0200 Subject: [PATCH] Fix possible endless loop (#18079) * Fix endless loop * Add test --- app/app.go | 4 +++- app/server_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/app.go b/app/app.go index 49dc3aed5c..b1950fc270 100644 --- a/app/app.go +++ b/app/app.go @@ -250,7 +250,7 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st return err } - if len(sysAdminsList) == 0 { + if len(sysAdmins) == 0 && len(sysAdminsList) == 0 { return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.empty_admin_list.app_error", nil, "", http.StatusInternalServerError) } sysAdmins = append(sysAdmins, sysAdminsList...) @@ -259,6 +259,8 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st mlog.Debug("Number of system admins is less than page limit", mlog.Int("count", len(sysAdminsList))) break } + + userOptions.Page++ } for _, sysAdmin := range sysAdmins { diff --git a/app/server_test.go b/app/server_test.go index c32bcb66a3..17993013c1 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -26,6 +26,7 @@ import ( "github.com/mattermost/mattermost-server/v6/config" "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/services/users" "github.com/mattermost/mattermost-server/v6/shared/filestore" "github.com/mattermost/mattermost-server/v6/shared/mlog" "github.com/mattermost/mattermost-server/v6/store/storetest" @@ -753,4 +754,34 @@ func TestAdminAdvisor(t *testing.T) { assert.Nil(t, err, "No error should be generated") assert.Equal(t, 0, len(posts.Posts)) }) + + t.Run("Should not break in case of many sysadmins", func(t *testing.T) { + var userList []*model.User + for i := 0; i < 50; i++ { + user := model.User{ + Email: strings.ToLower(NewTestId()) + "success+test@example.com", + Nickname: "Admin", + Username: "admin" + NewTestId(), + Password: "password", + AuthService: "", + Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, + } + ruser, err := th.App.srv.userService.CreateUser(&user, users.UserCreateOptions{FromImport: true}) + assert.NoError(t, err, "User should be created") + userList = append(userList, ruser) + defer th.App.PermanentDeleteUser(th.Context, ruser) + } + + th.App.notifyAdminsOfWarnMetricStatus(th.Context, model.SystemMetricSupportEmailNotConfigured, true) + + bot, err := th.App.GetUserByUsername(model.BotWarnMetricBotUsername) + assert.NotNil(t, bot, "Bot should have been created now") + assert.Nil(t, err, "No error should be generated") + + for _, user := range userList { + channel, err := th.App.getDirectChannel(bot.Id, user.Id) + assert.NotNil(t, channel, "DM channel should exist between Admin Advisor and system admin") + assert.Nil(t, err, "No error should be generated") + } + }) }