nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-05-28 23:04:50 -05:00
Code

[PM-15621] Fix single-user deletion bug.

Browse Source
This commit is contained in:
Jimmy Vo 2025-02-13 13:51:01 -05:00
parent 0f2356ad17
commit ed7a3ef593
No known key found for this signature in database
GPG Key ID: 7CB834D6F4FFCA11
2 changed files with 83 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs
View File

@ -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<IEnumerable<(Guid OrganizationUserId, string? ErrorMessage)>> DeleteManyUsersAsync(Guid organizationId, IEnumerable<Guid> orgUserIds, Guid? deletingUserId)
{
var userDeletionResults = await InternalDeleteManyUsersAsync(organizationId, orgUserIds, deletingUserId);
return userDeletionResults
.Select(result => (result.OrganizationUserId, result.exception?.Message))
.ToList();
}
private async Task<IEnumerable<(Guid OrganizationUserId, OrganizationUser? orgUser, User? user, Exception? exception)>> InternalDeleteManyUsersAsync(Guid organizationId, IEnumerable<Guid> 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<IEnumerable<User>> GetUsersAsync(ICollection<OrganizationUser> 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!);

58
test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs
View File

@ -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<DeleteManagedOrganizationUserAccountCommand> sutProvider, User user, Guid organizationId,
[OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser)
{
// Arrange
orgUser.OrganizationId = organizationId;
orgUser.UserId = user.Id;
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyAsync(Arg.Any<IEnumerable<Guid>>())
.Returns(new List<OrganizationUser> { orgUser });
sutProvider.GetDependency<IUserRepository>()
.GetManyAsync(Arg.Is<IEnumerable<Guid>>(ids => ids.Contains(user.Id)))
.Returns(new[] { user });
sutProvider.GetDependency<IGetOrganizationUsersManagementStatusQuery>()
.GetUsersOrganizationManagementStatusAsync(organizationId, Arg.Any<IEnumerable<Guid>>())
.Returns(new Dictionary<Guid, bool> { { orgUser.Id, true } });
// Act
await sutProvider.Sut.DeleteUserAsync(organizationId, orgUser.Id, null);
// Assert
await sutProvider.GetDependency<IUserRepository>().Received(1).DeleteManyAsync(Arg.Is<IEnumerable<User>>(users => users.Any(u => u.Id == user.Id)));
await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventsAsync(
Arg.Is<IEnumerable<(OrganizationUser, EventType, DateTime?)>>(events =>
events.Count(e => e.Item1.Id == orgUser.Id && e.Item2 == EventType.OrganizationUser_Deleted) == 1));
}
[Theory]
[BitAutoData]
public async Task DeleteUserAsync_WhenError_ShouldThrowException(
SutProvider<DeleteManagedOrganizationUserAccountCommand> sutProvider, User user, Guid organizationId,
[OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser)
{
// Arrange
orgUser.OrganizationId = organizationId;
orgUser.UserId = user.Id;
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyAsync(Arg.Any<IEnumerable<Guid>>())
.Returns(new List<OrganizationUser> { });
// Act
await Assert.ThrowsAsync<NotFoundException>(() => 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<IOrganizationUserRepository>().Received(1).GetManyAsync(userIds);
await sutProvider.GetDependency<IUserRepository>().Received(1).DeleteManyAsync(Arg.Is<IEnumerable<User>>(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);