MM-52638- Last admin cannot be demoted (#24087)

* don't allow last sysadmin to change roles

* cleanup, add comment

* only allow admin downgrade if more than one admin

* remove unused variable

* i18n-extract, unit test fixes

* Update user.go

* remove blank line

* update tests check all return values

* revert channel_store.go

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Scott Bishel
2024-05-31 06:13:46 -06:00
коммит произвёл GitHub
родитель 72644e72f6
Коммит b788760e17
3 изменённых файлов: 66 добавлений и 0 удалений

Просмотреть файл

@@ -1670,6 +1670,21 @@ func (a *App) UpdateUserRolesWithUser(c request.CTX, user *model.User, newRoles
return nil, err
}
if user.IsSystemAdmin() && !strings.Contains(newRoles, model.SystemAdminRoleId) {
// if user being updated is SysAdmin, make sure its not the last one.
options := model.UserCountOptions{
IncludeBotAccounts: false,
Roles: []string{model.SystemAdminRoleId},
}
count, err := a.Srv().Store().User().Count(options)
if err != nil {
return nil, model.NewAppError("UpdateUserRoles", "app.user.update.countAdmins.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
if count <= 1 {
return nil, model.NewAppError("UpdateUserRoles", "app.user.update.lastAdmin.app_error", nil, "", http.StatusBadRequest)
}
}
user.Roles = newRoles
uchan := make(chan store.StoreResult[*model.UserUpdate], 1)
go func() {

Просмотреть файл

@@ -1687,6 +1687,49 @@ func TestUpdateUserRolesWithUser(t *testing.T) {
// Test bad role.
_, err = th.App.UpdateUserRolesWithUser(th.Context, user, "does not exist", false)
require.NotNil(t, err)
//Test reset to User role
user, err = th.App.UpdateUserRolesWithUser(th.Context, user, model.SystemUserRoleId, false)
require.Nil(t, err)
assert.Equal(t, user.Roles, model.SystemUserRoleId)
}
func TestUpdateLastAdminUserRolesWithUser(t *testing.T) {
// InitBasic is used to let the first CreateUser call not be
// a system_admin
th := Setup(t).InitBasic()
defer th.TearDown()
t.Run("Cannot remove if only admin", func(t *testing.T) {
// Attempt to downgrade sysadmin.
user, appErr := th.App.UpdateUserRolesWithUser(th.Context, th.SystemAdminUser, model.SystemUserRoleId, false)
require.NotNil(t, appErr)
require.Nil(t, user)
})
t.Run("Cannot remove if only non-Bot admin", func(t *testing.T) {
bot := th.CreateBot()
user, appErr := th.App.UpdateUserRoles(th.Context, bot.UserId, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
require.Nil(t, appErr)
require.NotNil(t, user)
// Attempt to downgrade sysadmin.
user, appErr = th.App.UpdateUserRolesWithUser(th.Context, th.SystemAdminUser, model.SystemUserRoleId, false)
require.NotNil(t, appErr)
require.Nil(t, user)
})
t.Run("Can remove if not only non-Bot admin", func(t *testing.T) {
systemAdminUser2 := th.CreateUser()
user, appErr := th.App.UpdateUserRoles(th.Context, systemAdminUser2.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
require.Nil(t, appErr)
require.NotNil(t, user)
// Attempt to downgrade sysadmin.
user, appErr = th.App.UpdateUserRolesWithUser(th.Context, th.SystemAdminUser, model.SystemUserRoleId, false)
require.Nil(t, appErr)
require.NotNil(t, user)
})
}
func TestDeactivateMfa(t *testing.T) {

Просмотреть файл

@@ -6846,6 +6846,10 @@
"id": "app.user.store_is_empty.app_error",
"translation": "Failed to check if user store is empty."
},
{
"id": "app.user.update.countAdmins.app_error",
"translation": "Error determining number of System Admin accounts."
},
{
"id": "app.user.update.find.app_error",
"translation": "Unable to find the existing account to update."
@@ -6854,6 +6858,10 @@
"id": "app.user.update.finding.app_error",
"translation": "We encountered an error finding the account."
},
{
"id": "app.user.update.lastAdmin.app_error",
"translation": "Cannot demote last System Admin."
},
{
"id": "app.user.update_active.user_limit.exceeded",
"translation": "Can't activate user. Server exceeds safe user limit. Contact your administrator with: ERROR_SAFETY_LIMITS_EXCEEDED."