diff --git a/server/channels/app/user.go b/server/channels/app/user.go index e3cbd5342b..a9ea14c05a 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -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() { diff --git a/server/channels/app/user_test.go b/server/channels/app/user_test.go index 9519d88108..3a62336045 100644 --- a/server/channels/app/user_test.go +++ b/server/channels/app/user_test.go @@ -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) { diff --git a/server/i18n/en.json b/server/i18n/en.json index 1ba6c0f42f..6c5f55011f 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -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."