Upstream/bitwarden-server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00
Code Issues Projects Releases Wiki Activity

Enhance RemoveOrganizationUserCommand to block removing managed users when account deprovisioning is enabled

Browse Source
This commit is contained in:
Rui Tome 2024-11-12 15:40:18 +00:00
parent 702a81b161
commit c04c4fdfaa
No known key found for this signature in database
GPG Key ID: 526239D96A8EC066
2 changed files with 135 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs
View File

@ -17,6 +17,8 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
private readonly IPushRegistrationService _pushRegistrationService;
private readonly ICurrentContext _currentContext;
private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery;
private readonly IGetOrganizationUsersManagementStatusQuery _getOrganizationUsersManagementStatusQuery;
private readonly IFeatureService _featureService;
public RemoveOrganizationUserCommand(
IDeviceRepository deviceRepository,
@ -25,7 +27,9 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
IPushNotificationService pushNotificationService,
IPushRegistrationService pushRegistrationService,
ICurrentContext currentContext,
IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery)
IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery,
IGetOrganizationUsersManagementStatusQuery getOrganizationUsersManagementStatusQuery,
IFeatureService featureService)
{
_deviceRepository = deviceRepository;
_organizationUserRepository = organizationUserRepository;
@ -34,6 +38,8 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
_pushRegistrationService = pushRegistrationService;
_currentContext = currentContext;
_hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery;
_getOrganizationUsersManagementStatusQuery = getOrganizationUsersManagementStatusQuery;
_featureService = featureService;
}
public async Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId)
@ -90,6 +96,9 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
deletingUserIsOwner = await _currentContext.OrganizationOwner(organizationId);
}
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 deletedUserIds = new List<Guid>();
foreach (var orgUser in filteredUsers)
@ -106,13 +115,18 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
throw new BadRequestException("Only owners can delete other owners.");
}
if (managementStatus.TryGetValue(orgUser.Id, out var isManaged) && isManaged)
{
throw new BadRequestException("Managed members cannot be simply removed, their entire individual account must be deleted.");
}
await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed);
if (orgUser.UserId.HasValue)
{
await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value);
}
result.Add(Tuple.Create(orgUser, ""));
result.Add(Tuple.Create(orgUser, string.Empty));
deletedUserIds.Add(orgUser.Id);
}
catch (BadRequestException e)
@ -154,6 +168,15 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
}
}
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
{
var managementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(orgUser.OrganizationId, new[] { orgUser.Id });
if (managementStatus.TryGetValue(orgUser.Id, out var isManaged) && isManaged)
{
throw new BadRequestException("Managed members cannot be simply removed, their entire individual account must be deleted.");
}
}
await _organizationUserRepository.DeleteAsync(orgUser);
if (orgUser.UserId.HasValue)

110
test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs
View File

@ -216,6 +216,60 @@ public class RemoveOrganizationUserCommandTests
Arg.Any<bool>());
}
[Theory, BitAutoData]
public async Task RemoveUser_RemovingManagedUser_WithAccountDeprovisioningEnabled_ThrowsException(
[OrganizationUser(status: OrganizationUserStatusType.Accepted, OrganizationUserType.User)] OrganizationUser orgUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByIdAsync(orgUser.Id)
.Returns(orgUser);
sutProvider.GetDependency<ICurrentContext>()
.OrganizationOwner(orgUser.OrganizationId)
.Returns(false);
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
.Returns(true);
sutProvider.GetDependency<IGetOrganizationUsersManagementStatusQuery>()
.GetUsersOrganizationManagementStatusAsync(orgUser.OrganizationId, Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new Dictionary<Guid, bool> { { orgUser.Id, true } });
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.RemoveUserAsync(orgUser.OrganizationId, orgUser.Id, null));
Assert.Contains("Managed members cannot be simply removed, their entire individual account must be deleted.", exception.Message);
await sutProvider.GetDependency<IOrganizationUserRepository>().DidNotReceiveWithAnyArgs().DeleteAsync(default);
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogOrganizationUserEventAsync((OrganizationUser)default, EventType.OrganizationUser_Removed);
}
[Theory, BitAutoData]
public async Task RemoveUser_RemovingUnmanagedUser_WithAccountDeprovisioningEnabled_Success(
[OrganizationUser(status: OrganizationUserStatusType.Accepted, OrganizationUserType.User)] OrganizationUser orgUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByIdAsync(orgUser.Id)
.Returns(orgUser);
sutProvider.GetDependency<ICurrentContext>()
.OrganizationOwner(orgUser.OrganizationId)
.Returns(false);
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
.Returns(true);
sutProvider.GetDependency<IGetOrganizationUsersManagementStatusQuery>()
.GetUsersOrganizationManagementStatusAsync(orgUser.OrganizationId, Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new Dictionary<Guid, bool> { { orgUser.Id, false } });
await sutProvider.Sut.RemoveUserAsync(orgUser.OrganizationId, orgUser.Id, null);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.DeleteAsync(orgUser);
await sutProvider.GetDependency<IEventService>()
.Received(1)
.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed);
}
[Theory, BitAutoData]
public async Task RemoveUsers_FilterInvalid_ThrowsException(OrganizationUser organizationUser, OrganizationUser deletingUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
@ -268,6 +322,62 @@ public class RemoveOrganizationUserCommandTests
Assert.Contains("Only owners can delete other owners.", result[0].Item2);
}
[Theory, BitAutoData]
public async Task RemoveUsers_RemovingManagedUser_WithAccountDeprovisioningEnabled_ThrowsException(
[OrganizationUser(status: OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
.Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyAsync(Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new[] { orgUser });
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(orgUser.OrganizationId, Arg.Any<IEnumerable<Guid>>())
.Returns(true);
sutProvider.GetDependency<IGetOrganizationUsersManagementStatusQuery>()
.GetUsersOrganizationManagementStatusAsync(orgUser.OrganizationId, Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new Dictionary<Guid, bool> { { orgUser.Id, true } });
var result = await sutProvider.Sut.RemoveUsersAsync(orgUser.OrganizationId, new[] { orgUser.Id }, null);
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);
}
[Theory, BitAutoData]
public async Task RemoveUsers_RemovingUnmanagedUser_WithAccountDeprovisioningEnabled_Success(
[OrganizationUser(status: OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser orgUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
.Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyAsync(Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new[] { orgUser });
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(orgUser.OrganizationId, Arg.Any<IEnumerable<Guid>>())
.Returns(true);
sutProvider.GetDependency<IGetOrganizationUsersManagementStatusQuery>()
.GetUsersOrganizationManagementStatusAsync(orgUser.OrganizationId, Arg.Is<IEnumerable<Guid>>(i => i.Contains(orgUser.Id)))
.Returns(new Dictionary<Guid, bool> { { orgUser.Id, false } });
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);
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);
}
[Theory, BitAutoData]
public async Task RemoveUsers_LastOwner_ThrowsException(
[OrganizationUser(status: OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUser,