1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-24 12:35:25 +01:00

Refactor RemoveUsersAsync method to return just the OrgUserId and update related logic.

This commit is contained in:
Rui Tome 2024-11-13 11:40:05 +00:00
parent c04c4fdfaa
commit 5a493c43eb
No known key found for this signature in database
GPG Key ID: 526239D96A8EC066
4 changed files with 52 additions and 15 deletions

View File

@ -538,7 +538,7 @@ public class OrganizationUsersController : Controller
var userId = _userService.GetProperUserId(User);
var result = await _removeOrganizationUserCommand.RemoveUsersAsync(orgId, model.Ids, userId.Value);
return new ListResponseModel<OrganizationUserBulkResponseModel>(result.Select(r =>
new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2)));
new OrganizationUserBulkResponseModel(r.OrganizationUserId, r.ErrorMessage)));
}
[RequireFeature(FeatureFlagKeys.AccountDeprovisioning)]

View File

@ -1,5 +1,4 @@
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Enums;
namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
@ -9,6 +8,13 @@ public interface IRemoveOrganizationUserCommand
Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, EventSystemUser eventSystemUser);
Task RemoveUserAsync(Guid organizationId, Guid userId);
Task<List<Tuple<OrganizationUser, string>>> RemoveUsersAsync(Guid organizationId,
IEnumerable<Guid> organizationUserIds, Guid? deletingUserId);
/// <summary>
/// Removes multiple users from an organization.
/// </summary>
/// <returns>
/// A list of tuples containing the organization user id and the error message for each user that could not be removed, otherwise empty.
/// </returns>
Task<IEnumerable<(Guid OrganizationUserId, string ErrorMessage)>> RemoveUsersAsync(
Guid organizationId, IEnumerable<Guid> organizationUserIds, Guid? deletingUserId);
}

View File

@ -72,9 +72,8 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed);
}
public async Task<List<Tuple<OrganizationUser, string>>> RemoveUsersAsync(Guid organizationId,
IEnumerable<Guid> organizationUsersId,
Guid? deletingUserId)
public async Task<IEnumerable<(Guid OrganizationUserId, string ErrorMessage)>> RemoveUsersAsync(
Guid organizationId, IEnumerable<Guid> organizationUsersId, Guid? deletingUserId)
{
var orgUsers = await _organizationUserRepository.GetManyAsync(organizationUsersId);
var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId)
@ -99,7 +98,7 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
var managementStatus = _featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) ?
await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(organizationId, filteredUsers.Select(u => u.Id)) :
filteredUsers.ToDictionary(u => u.Id, u => false);
var result = new List<Tuple<OrganizationUser, string>>();
var result = new List<(Guid OrganizationUserId, string ErrorMessage)>();
var deletedUserIds = new List<Guid>();
foreach (var orgUser in filteredUsers)
{
@ -126,15 +125,22 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
{
await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value);
}
result.Add(Tuple.Create(orgUser, string.Empty));
result.Add((orgUser.Id, string.Empty));
deletedUserIds.Add(orgUser.Id);
}
catch (BadRequestException e)
{
result.Add(Tuple.Create(orgUser, e.Message));
result.Add((orgUser.Id, e.Message));
}
}
if (deletedUserIds.Any())
{
DateTime? eventDate = DateTime.UtcNow;
await _organizationUserRepository.DeleteManyAsync(deletedUserIds);
await _eventService.LogOrganizationUserEventsAsync(
orgUsers.Where(u => deletedUserIds.Contains(u.Id))
.Select(u => (u, EventType.OrganizationUser_Removed, eventDate)));
}
return result;
@ -200,4 +206,29 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
organizationId.ToString());
await _pushNotificationService.PushSyncOrgKeysAsync(userId);
}
private async Task LogDeletedOrganizationUsersAsync(
IEnumerable<OrganizationUser> orgUsers,
IEnumerable<(Guid OrgUserId, string ErrorMessage)> results)
{
var eventDate = DateTime.UtcNow;
var events = new List<(OrganizationUser OrgUser, EventType Event, DateTime? EventDate)>();
foreach (var (orgUserId, errorMessage) in results)
{
var orgUser = orgUsers.FirstOrDefault(ou => ou.Id == orgUserId);
// If the user was not found or there was an error, we skip logging the event
if (orgUser == null || !string.IsNullOrEmpty(errorMessage))
{
continue;
}
events.Add((orgUser, EventType.OrganizationUser_Deleted, eventDate));
}
if (events.Any())
{
await _eventService.LogOrganizationUserEventsAsync(events);
}
}
}

View File

@ -298,7 +298,7 @@ public class RemoveOrganizationUserCommandTests
.Returns(true);
var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId);
Assert.Contains("You cannot remove yourself.", result[0].Item2);
Assert.Contains("You cannot remove yourself.", result.First().ErrorMessage);
}
[Theory, BitAutoData]
@ -319,7 +319,7 @@ public class RemoveOrganizationUserCommandTests
.Returns(true);
var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId);
Assert.Contains("Only owners can delete other owners.", result[0].Item2);
Assert.Contains("Only owners can delete other owners.", result.First().ErrorMessage);
}
[Theory, BitAutoData]
@ -347,7 +347,7 @@ public class RemoveOrganizationUserCommandTests
await sutProvider.GetDependency<IOrganizationUserRepository>().DidNotReceiveWithAnyArgs().DeleteAsync(default);
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogOrganizationUserEventAsync((OrganizationUser)default, EventType.OrganizationUser_Removed);
Assert.Contains("Managed members cannot be simply removed, their entire individual account must be deleted.", result[0].Item2);
Assert.Contains("Managed members cannot be simply removed, their entire individual account must be deleted.", result.First().ErrorMessage);
}
[Theory, BitAutoData]
@ -373,7 +373,7 @@ public class RemoveOrganizationUserCommandTests
var result = await sutProvider.Sut.RemoveUsersAsync(orgUser.OrganizationId, new[] { orgUser.Id }, null);
Assert.True(result.Count == 1 && result[0].Item1.Id == orgUser.Id && result[0].Item2 == string.Empty);
Assert.True(result.Count() == 1 && result.First().OrganizationUserId == orgUser.Id && result.First().ErrorMessage == string.Empty);
await sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).DeleteManyAsync(Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)));
await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed);
}