From 4f5fc0b8a10841adc74228b6ccb845b78cbd0dd1 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 29 Aug 2023 15:51:20 +0200 Subject: [PATCH] Bulk delete functionality for management api (#14735) * Bulk delete * Bulk delete * Added bulk delete user groups * Clean --------- Co-authored-by: Nikolaj --- .../User/BulkDeleteUsersController.cs | 35 +++++++++++++ .../Controllers/User/DeleteUsersController.cs | 10 +++- .../Controllers/User/UsersControllerBase.cs | 4 ++ .../BulkDeleteUserGroupsController.cs | 33 +++++++++++++ .../User/DeleteUsersRequestModel.cs | 6 +++ .../User/DisableUserRequestModel.cs | 2 +- .../UserGroup/DeleteUserGroupsRequestModel.cs | 6 +++ .../UserGroupDeletedNotification.cs | 5 ++ .../Services/IUserGroupService.cs | 5 +- src/Umbraco.Core/Services/IUserService.cs | 4 +- .../UserGroupOperationStatus.cs | 2 +- .../OperationStatus/UserOperationStatus.cs | 1 + src/Umbraco.Core/Services/UserGroupService.cs | 49 ++++++++++++------- src/Umbraco.Core/Services/UserService.cs | 45 +++++++++++++---- .../Services/UserServiceCrudTests.Delete.cs | 6 +-- 15 files changed, 175 insertions(+), 38 deletions(-) create mode 100644 src/Umbraco.Cms.Api.Management/Controllers/User/BulkDeleteUsersController.cs create mode 100644 src/Umbraco.Cms.Api.Management/Controllers/UserGroup/BulkDeleteUserGroupsController.cs create mode 100644 src/Umbraco.Cms.Api.Management/ViewModels/User/DeleteUsersRequestModel.cs create mode 100644 src/Umbraco.Cms.Api.Management/ViewModels/UserGroup/DeleteUserGroupsRequestModel.cs diff --git a/src/Umbraco.Cms.Api.Management/Controllers/User/BulkDeleteUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/User/BulkDeleteUsersController.cs new file mode 100644 index 0000000000..1e90e9c710 --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/Controllers/User/BulkDeleteUsersController.cs @@ -0,0 +1,35 @@ +using Asp.Versioning; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Umbraco.Cms.Api.Management.ViewModels.User; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Api.Management.Controllers.User; + +[ApiVersion("1.0")] +public class BulkDeleteUsersController : UserControllerBase +{ + private readonly IUserService _userService; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + + public BulkDeleteUsersController(IUserService userService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _userService = userService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } + + [HttpDelete] + [MapToApiVersion("1.0")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] + public async Task DeleteUsers(DeleteUsersRequestModel model) + { + UserOperationStatus result = await _userService.DeleteAsync(CurrentUserKey(_backOfficeSecurityAccessor), model.UserIds); + + return result is UserOperationStatus.Success + ? Ok() + : UserOperationStatusResult(result); + } +} diff --git a/src/Umbraco.Cms.Api.Management/Controllers/User/DeleteUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/User/DeleteUsersController.cs index 73a615fbfb..c71a933dfb 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/User/DeleteUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/User/DeleteUsersController.cs @@ -1,5 +1,6 @@ using Asp.Versioning; using Microsoft.AspNetCore.Mvc; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -8,15 +9,20 @@ namespace Umbraco.Cms.Api.Management.Controllers.User; [ApiVersion("1.0")] public class DeleteUserController : UserControllerBase { - public DeleteUserController(IUserService userService) => _userService = userService; + public DeleteUserController(IUserService userService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _userService = userService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } private readonly IUserService _userService; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; [MapToApiVersion("1.0")] [HttpDelete("{id:guid}")] public async Task DeleteUser(Guid id) { - UserOperationStatus result = await _userService.DeleteAsync(id); + UserOperationStatus result = await _userService.DeleteAsync(CurrentUserKey(_backOfficeSecurityAccessor), id); return result is UserOperationStatus.Success ? Ok() diff --git a/src/Umbraco.Cms.Api.Management/Controllers/User/UsersControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/User/UsersControllerBase.cs index 6385dc3ac4..52cac2e793 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/User/UsersControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/User/UsersControllerBase.cs @@ -64,6 +64,10 @@ public abstract class UserControllerBase : ManagementApiControllerBase .WithTitle("Cannot disable") .WithDetail("A user cannot disable itself.") .Build()), + UserOperationStatus.CannotDeleteSelf => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Cannot delete") + .WithDetail("A user cannot delete itself.") + .Build()), UserOperationStatus.OldPasswordRequired => BadRequest(new ProblemDetailsBuilder() .WithTitle("Old password required") .WithDetail("The old password is required to change the password of the specified user.") diff --git a/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/BulkDeleteUserGroupsController.cs b/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/BulkDeleteUserGroupsController.cs new file mode 100644 index 0000000000..1acfc6f3b8 --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/BulkDeleteUserGroupsController.cs @@ -0,0 +1,33 @@ +using Asp.Versioning; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Umbraco.Cms.Api.Management.ViewModels.UserGroup; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Api.Management.Controllers.UserGroup; + +[ApiVersion("1.0")] +public class BulkDeleteUserGroupsController : UserGroupControllerBase +{ + private readonly IUserGroupService _userGroupService; + + public BulkDeleteUserGroupsController(IUserGroupService userGroupService) + { + _userGroupService = userGroupService; + } + + [HttpDelete] + [MapToApiVersion("1.0")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + public async Task BulkDelete(DeleteUserGroupsRequestModel model) + { + Attempt result = await _userGroupService.DeleteAsync(model.UserGroupIds); + + return result.Success + ? Ok() + : UserGroupOperationStatusResult(result.Result); + } +} diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/User/DeleteUsersRequestModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/User/DeleteUsersRequestModel.cs new file mode 100644 index 0000000000..12c774670f --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/ViewModels/User/DeleteUsersRequestModel.cs @@ -0,0 +1,6 @@ +namespace Umbraco.Cms.Api.Management.ViewModels.User; + +public class DeleteUsersRequestModel +{ + public HashSet UserIds { get; set; } = new(); +} diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/User/DisableUserRequestModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/User/DisableUserRequestModel.cs index 2ecf0d3ebc..f510ea5ed1 100644 --- a/src/Umbraco.Cms.Api.Management/ViewModels/User/DisableUserRequestModel.cs +++ b/src/Umbraco.Cms.Api.Management/ViewModels/User/DisableUserRequestModel.cs @@ -3,4 +3,4 @@ public class DisableUserRequestModel { public HashSet UserIds { get; set; } = new(); - } +} diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/UserGroup/DeleteUserGroupsRequestModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/UserGroup/DeleteUserGroupsRequestModel.cs new file mode 100644 index 0000000000..9679f4da8a --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/ViewModels/UserGroup/DeleteUserGroupsRequestModel.cs @@ -0,0 +1,6 @@ +namespace Umbraco.Cms.Api.Management.ViewModels.UserGroup; + +public class DeleteUserGroupsRequestModel +{ + public HashSet UserGroupIds { get; set; } = new(); +} diff --git a/src/Umbraco.Core/Notifications/UserGroupDeletedNotification.cs b/src/Umbraco.Core/Notifications/UserGroupDeletedNotification.cs index 0555611f3a..cbbaa583ff 100644 --- a/src/Umbraco.Core/Notifications/UserGroupDeletedNotification.cs +++ b/src/Umbraco.Core/Notifications/UserGroupDeletedNotification.cs @@ -12,4 +12,9 @@ public sealed class UserGroupDeletedNotification : DeletedNotification target, EventMessages messages) + : base(target, messages) + { + } } diff --git a/src/Umbraco.Core/Services/IUserGroupService.cs b/src/Umbraco.Core/Services/IUserGroupService.cs index 364e70f394..81142b828d 100644 --- a/src/Umbraco.Core/Services/IUserGroupService.cs +++ b/src/Umbraco.Core/Services/IUserGroupService.cs @@ -82,9 +82,10 @@ public interface IUserGroupService /// /// Deletes a UserGroup /// - /// The key of the user group to delete. + /// The keys of the user groups to delete. /// An attempt indicating if the operation was a success as well as a more detailed . - Task> DeleteAsync(Guid key); + Task> DeleteAsync(ISet userGroupKeys); + Task> DeleteAsync(Guid userGroupKey) => DeleteAsync(new HashSet(){userGroupKey}); /// /// Updates the users to have the groups specified. diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index 79e1738e95..aaf5625029 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -67,7 +67,9 @@ public interface IUserService : IMembershipUserService Task SetAvatarAsync(Guid userKey, Guid temporaryFileKey); - Task DeleteAsync(Guid key); + Task DeleteAsync(Guid userKey, ISet keys); + + Task DeleteAsync(Guid userKey, Guid key) => DeleteAsync(userKey, new HashSet { key }); Task DisableAsync(Guid userKey, ISet keys); diff --git a/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs index f25f0de64e..b27ee81794 100644 --- a/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs @@ -18,5 +18,5 @@ public enum UserGroupOperationStatus LanguageNotFound, NameTooLong, AliasTooLong, - MissingName, + MissingName } diff --git a/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs index 739305f0c2..e10166661b 100644 --- a/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs @@ -19,6 +19,7 @@ public enum UserOperationStatus CannotInvite, CannotDelete, CannotDisableSelf, + CannotDeleteSelf, CannotDisableInvitedUser, OldPasswordRequired, InvalidAvatar, diff --git a/src/Umbraco.Core/Services/UserGroupService.cs b/src/Umbraco.Core/Services/UserGroupService.cs index 848f461e23..31a5d1e0bb 100644 --- a/src/Umbraco.Core/Services/UserGroupService.cs +++ b/src/Umbraco.Core/Services/UserGroupService.cs @@ -135,35 +135,48 @@ internal sealed class UserGroupService : RepositoryService, IUserGroupService } /// - public async Task> DeleteAsync(Guid key) + public async Task> DeleteAsync(ISet keys) { - IUserGroup? userGroup = await GetAsync(key); - - Attempt validationResult = ValidateUserGroupDeletion(userGroup); - if (validationResult.Success is false) + if (keys.Any() is false) { - return validationResult; + return Attempt.Succeed(UserGroupOperationStatus.Success); } - EventMessages eventMessages = EventMessagesFactory.Get(); + IUserGroup[] userGroupsToDelete = (await GetAsync(keys)).ToArray(); - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + if (userGroupsToDelete.Length != keys.Count) { - var deletingNotification = new UserGroupDeletingNotification(userGroup!, eventMessages); + return Attempt.Fail(UserGroupOperationStatus.NotFound); + } - if (await scope.Notifications.PublishCancelableAsync(deletingNotification)) + foreach (IUserGroup userGroup in userGroupsToDelete) + { + Attempt validationResult = ValidateUserGroupDeletion(userGroup); + if (validationResult.Success is false) { - scope.Complete(); - return Attempt.Fail(UserGroupOperationStatus.CancelledByNotification); + return validationResult; } - - _userGroupRepository.Delete(userGroup!); - - scope.Notifications.Publish(new UserGroupDeletedNotification(userGroup!, eventMessages).WithStateFrom(deletingNotification)); - - scope.Complete(); } + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var deletingNotification = new UserGroupDeletingNotification(userGroupsToDelete, eventMessages); + + if (await scope.Notifications.PublishCancelableAsync(deletingNotification)) + { + scope.Complete(); + return Attempt.Fail(UserGroupOperationStatus.CancelledByNotification); + } + + foreach (IUserGroup userGroup in userGroupsToDelete) + { + _userGroupRepository.Delete(userGroup); + } + + scope.Notifications.Publish(new UserGroupDeletedNotification(userGroupsToDelete, eventMessages).WithStateFrom(deletingNotification)); + + scope.Complete(); + return Attempt.Succeed(UserGroupOperationStatus.Success); } diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 6a2f7c440d..ea4abdef5f 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -1324,24 +1324,49 @@ internal class UserService : RepositoryService, IUserService }; } - public async Task DeleteAsync(Guid key) + public async Task DeleteAsync(Guid userKey, ISet keys) { - using ICoreScope scope = ScopeProvider.CreateCoreScope(); - IUser? user = await GetAsync(key); + if(keys.Any() is false) + { + return UserOperationStatus.Success; + } - if (user is null) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + IUser? performingUser = await GetAsync(userKey); + + if (performingUser is null) + { + return UserOperationStatus.MissingUser; + } + + if (keys.Contains(performingUser.Key)) + { + return UserOperationStatus.CannotDeleteSelf; + } + + IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + IUser[] usersToDisable = (await userStore.GetUsersAsync(keys.ToArray())).ToArray(); + + if (usersToDisable.Length != keys.Count) { return UserOperationStatus.UserNotFound; } - // Check user hasn't logged in. If they have they may have made content changes which will mean - // the Id is associated with audit trails, versions etc. and can't be removed. - if (user.LastLoginDate is not null && user.LastLoginDate != default(DateTime)) + foreach (IUser user in usersToDisable) { - return UserOperationStatus.CannotDelete; - } + // Check user hasn't logged in. If they have they may have made content changes which will mean + // the Id is associated with audit trails, versions etc. and can't be removed. + if (user.LastLoginDate is not null && user.LastLoginDate != default(DateTime)) + { + return UserOperationStatus.CannotDelete; + } - Delete(user, true); + user.IsApproved = false; + user.InvitedDate = null; + + Delete(user, true); + } scope.Complete(); return UserOperationStatus.Success; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs index 8936b93e90..98875c21ef 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs @@ -12,7 +12,7 @@ public partial class UserServiceCrudTests public async Task Delete_Returns_Not_Found_If_Not_Found() { var userService = CreateUserService(); - var result = await userService.DeleteAsync(Guid.NewGuid()); + var result = await userService.DeleteAsync(Constants.Security.SuperUserKey, Guid.NewGuid()); Assert.AreEqual(UserOperationStatus.UserNotFound, result); } @@ -36,7 +36,7 @@ public partial class UserServiceCrudTests createdUser!.LastLoginDate = DateTime.Now; userService.Save(createdUser); - var result = await userService.DeleteAsync(createdUser.Key); + var result = await userService.DeleteAsync(Constants.Security.SuperUserKey, createdUser.Key); Assert.AreEqual(UserOperationStatus.CannotDelete, result); // Asset that it is in fact not deleted @@ -61,7 +61,7 @@ public partial class UserServiceCrudTests var creationResult = await userService.CreateAsync(Constants.Security.SuperUserKey, userCreateModel, true); Assert.IsTrue(creationResult.Success); - var deletionResult = await userService.DeleteAsync(creationResult.Result.CreatedUser!.Key); + var deletionResult = await userService.DeleteAsync(Constants.Security.SuperUserKey, creationResult.Result.CreatedUser!.Key); Assert.AreEqual(UserOperationStatus.Success, deletionResult); // Make sure it's actually deleted var postDeletedUser = await userService.GetAsync(creationResult.Result.CreatedUser.Key);