diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs index 7d96ec531c..25844061c7 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs @@ -55,15 +55,31 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz public async Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId) { - await DeleteManyUsersAsync(organizationId, new[] { organizationUserId }, deletingUserId); + var result = await InternalDeleteManyUsersAsync(organizationId, new[] { organizationUserId }, deletingUserId); + + var exception = result.Single().exception; + + if (exception != null) + { + throw exception; + } } public async Task> DeleteManyUsersAsync(Guid organizationId, IEnumerable orgUserIds, Guid? deletingUserId) + { + var userDeletionResults = await InternalDeleteManyUsersAsync(organizationId, orgUserIds, deletingUserId); + + return userDeletionResults + .Select(result => (result.OrganizationUserId, result.exception?.Message)) + .ToList(); + } + + private async Task> InternalDeleteManyUsersAsync(Guid organizationId, IEnumerable orgUserIds, Guid? deletingUserId) { var orgUsers = await _organizationUserRepository.GetManyAsync(orgUserIds); var users = await GetUsersAsync(orgUsers); var managementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(organizationId, orgUserIds); - var userDeletionResults = new List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, string? ErrorMessage)>(); + var userDeletionResults = new List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, Exception? exception)>(); foreach (var orgUserId in orgUserIds) { @@ -89,11 +105,11 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz await CancelPremiumAsync(user); - userDeletionResults.Add((orgUserId, orgUser, user, string.Empty)); + userDeletionResults.Add((orgUserId, orgUser, user, null)); } - catch (Exception ex) + catch (Exception exception) { - userDeletionResults.Add((orgUserId, orgUser, user, ex.Message)); + userDeletionResults.Add((orgUserId, orgUser, user, exception)); } } @@ -101,9 +117,7 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz await LogDeletedOrganizationUsersAsync(userDeletionResults); - return userDeletionResults - .Select(i => (i.OrganizationUserId, i.ErrorMessage)) - .ToList(); + return userDeletionResults; } private async Task> GetUsersAsync(ICollection orgUsers) @@ -185,12 +199,13 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz } private async Task LogDeletedOrganizationUsersAsync( - List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, string? ErrorMessage)> results) + List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, Exception? exception)> results) { var eventDate = DateTime.UtcNow; var events = results - .Where(result => string.IsNullOrEmpty(result.ErrorMessage) + .Where(result => + result.exception == null && result.orgUser != null) .Select(result => (result.orgUser!, (EventType)EventType.OrganizationUser_Deleted, (DateTime?)eventDate)) .ToList(); @@ -200,11 +215,11 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz await _eventService.LogOrganizationUserEventsAsync(events); } } - private async Task HandleUserDeletionsAsync(List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, string? ErrorMessage)> userDeletionResults) + private async Task HandleUserDeletionsAsync(List<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, Exception? exception)> userDeletionResults) { var usersToDelete = userDeletionResults .Where(result => - string.IsNullOrEmpty(result.ErrorMessage) + result.exception == null && result.user != null) .Select(i => i.user!); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs index 032911628b..a0a441ce3a 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs @@ -4,6 +4,7 @@ using Bit.Core.AdminConsole.Repositories; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; @@ -17,6 +18,59 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers; [SutProviderCustomize] public class DeleteManagedOrganizationUserAccountCommandTests { + + [Theory] + [BitAutoData] + public async Task DeleteUserAsync_WithValidUser_DeletesUserAndLogsEvents( + SutProvider sutProvider, User user, Guid organizationId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser) + { + // Arrange + orgUser.OrganizationId = organizationId; + orgUser.UserId = user.Id; + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(new List { orgUser }); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Is>(ids => ids.Contains(user.Id))) + .Returns(new[] { user }); + + sutProvider.GetDependency() + .GetUsersOrganizationManagementStatusAsync(organizationId, Arg.Any>()) + .Returns(new Dictionary { { orgUser.Id, true } }); + + // Act + await sutProvider.Sut.DeleteUserAsync(organizationId, orgUser.Id, null); + + // Assert + await sutProvider.GetDependency().Received(1).DeleteManyAsync(Arg.Is>(users => users.Any(u => u.Id == user.Id))); + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventsAsync( + Arg.Is>(events => + events.Count(e => e.Item1.Id == orgUser.Id && e.Item2 == EventType.OrganizationUser_Deleted) == 1)); + } + + [Theory] + [BitAutoData] + public async Task DeleteUserAsync_WhenError_ShouldThrowException( + SutProvider sutProvider, User user, Guid organizationId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser) + { + // Arrange + orgUser.OrganizationId = organizationId; + orgUser.UserId = user.Id; + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(new List { }); + + // Act + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteUserAsync(organizationId, orgUser.Id, null)); + + // Assert + } + [Theory] [BitAutoData] public async Task DeleteManyUsersAsync_WithValidUsers_DeletesUsersAndLogsEvents( @@ -47,7 +101,7 @@ public class DeleteManagedOrganizationUserAccountCommandTests // Assert Assert.Equal(2, results.Count()); - Assert.All(results, r => Assert.Empty(r.Item2)); + Assert.All(results, r => Assert.Null(r.Item2)); await sutProvider.GetDependency().Received(1).GetManyAsync(userIds); await sutProvider.GetDependency().Received(1).DeleteManyAsync(Arg.Is>(users => users.Any(u => u.Id == user1.Id) && users.Any(u => u.Id == user2.Id))); @@ -359,7 +413,7 @@ public class DeleteManagedOrganizationUserAccountCommandTests var orgUser2ErrorMessage = results.First(r => r.Item1 == orgUser2.Id).Item2; var orgUser3ErrorMessage = results.First(r => r.Item1 == orgUser3.Id).Item2; - Assert.Empty(orgUser1ErrorMessage); + Assert.Null(orgUser1ErrorMessage); Assert.Equal("Member not found.", orgUser2ErrorMessage); Assert.Equal("Member is not managed by the organization.", orgUser3ErrorMessage);