diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 2a06e6e95b..e71951d5b0 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -131,7 +131,7 @@ namespace Bit.Api.Controllers } [HttpPost("reinvite")] - public async Task BulkReinvite(string orgId, [FromBody]OrganizationUserBulkReinviteRequestModel model) + public async Task BulkReinvite(string orgId, [FromBody]OrganizationUserBulkRequestModel model) { var orgGuidId = new Guid(orgId); if (!_currentContext.ManageUsers(orgGuidId)) @@ -283,5 +283,19 @@ namespace Bit.Api.Controllers var userId = _userService.GetProperUserId(User); await _organizationService.DeleteUserAsync(orgGuidId, new Guid(id), userId.Value); } + + [HttpDelete("")] + [HttpPost("delete")] + public async Task BulkDelete(string orgId, [FromBody]OrganizationUserBulkRequestModel model) + { + var orgGuidId = new Guid(orgId); + if (!_currentContext.ManageUsers(orgGuidId)) + { + throw new NotFoundException(); + } + + var userId = _userService.GetProperUserId(User); + await _organizationService.DeleteUsersAsync(orgGuidId, model.Ids, userId.Value); + } } } diff --git a/src/Core/Models/Api/Request/Organizations/OrganizationUserRequestModels.cs b/src/Core/Models/Api/Request/Organizations/OrganizationUserRequestModels.cs index 428df040e3..6d480ae209 100644 --- a/src/Core/Models/Api/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Core/Models/Api/Request/Organizations/OrganizationUserRequestModels.cs @@ -91,7 +91,7 @@ namespace Bit.Core.Models.Api public string ResetPasswordKey { get; set; } } - public class OrganizationUserBulkReinviteRequestModel + public class OrganizationUserBulkRequestModel { [Required] public IEnumerable Ids { get; set; } diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index e9200efb50..801c18371a 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -43,6 +43,7 @@ namespace Bit.Core.Services Task SaveUserAsync(OrganizationUser user, Guid? savingUserId, IEnumerable collections); Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId); Task DeleteUserAsync(Guid organizationId, Guid userId); + Task DeleteUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? deleteingUserId); Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId); Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid organizationUserId, string resetPasswordKey, Guid? callingUserId); Task GenerateLicenseAsync(Guid organizationId, Guid installationId); diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index b5816d9565..86b55843f3 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1308,10 +1308,7 @@ namespace Bit.Core.Services await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); await _mailService.SendOrganizationConfirmedEmailAsync(org.Name, user.Email); - // push - var deviceIds = await GetUserDeviceIdsAsync(orgUser.UserId.Value); - await _pushRegistrationService.AddUserRegistrationOrganizationAsync(deviceIds, organizationId.ToString()); - await _pushNotificationService.PushSyncOrgKeysAsync(orgUser.UserId.Value); + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); return orgUser; } @@ -1334,9 +1331,8 @@ namespace Bit.Core.Services await ValidateOrganizationUserUpdatePermissions(savingUserId.Value, user.OrganizationId, user.Type, originalUser.Type); } - var confirmedOwners = (await GetConfirmedOwnersAsync(user.OrganizationId)).ToList(); if (user.Type != OrganizationUserType.Owner && - confirmedOwners.Count == 1 && confirmedOwners[0].Id == user.Id) + !await HasConfirmedOwnersExceptAsync(user.OrganizationId, Enumerable.Empty())) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -1363,19 +1359,13 @@ namespace Bit.Core.Services throw new BadRequestException("You cannot remove yourself."); } - if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue) + if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue && + !await UserIsOwnerAsync(organizationId, deletingUserId.Value)) { - var deletingUserOrgs = await _organizationUserRepository.GetManyByUserAsync(deletingUserId.Value); - var anyOwners = deletingUserOrgs.Any( - u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Owner); - if (!anyOwners) - { - throw new BadRequestException("Only owners can delete other owners."); - } + throw new BadRequestException("Only owners can delete other owners."); } - var confirmedOwners = (await GetConfirmedOwnersAsync(organizationId)).ToList(); - if (confirmedOwners.Count == 1 && confirmedOwners[0].Id == organizationUserId) + if (!await HasConfirmedOwnersExceptAsync(organizationId, new[] {organizationUserId})) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -1385,11 +1375,7 @@ namespace Bit.Core.Services if (orgUser.UserId.HasValue) { - // push - var deviceIds = await GetUserDeviceIdsAsync(orgUser.UserId.Value); - await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(deviceIds, - organizationId.ToString()); - await _pushNotificationService.PushSyncOrgKeysAsync(orgUser.UserId.Value); + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); } } @@ -1401,8 +1387,7 @@ namespace Bit.Core.Services throw new NotFoundException(); } - var confirmedOwners = (await GetConfirmedOwnersAsync(organizationId)).ToList(); - if (confirmedOwners.Count == 1 && confirmedOwners[0].Id == orgUser.Id) + if (!await HasConfirmedOwnersExceptAsync(organizationId, new[] {orgUser.Id})) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -1412,14 +1397,64 @@ namespace Bit.Core.Services if (orgUser.UserId.HasValue) { - // push - var deviceIds = await GetUserDeviceIdsAsync(orgUser.UserId.Value); - await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(deviceIds, - organizationId.ToString()); - await _pushNotificationService.PushSyncOrgKeysAsync(orgUser.UserId.Value); + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); } } + public async Task DeleteUsersAsync(Guid organizationId, IEnumerable organizationUsersId, + Guid? deletingUserId) + { + var orgUsers = await _organizationUserRepository.GetManyAsync(organizationUsersId); + var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId) + .ToList(); + + if (!filteredUsers.Any()) + { + throw new BadRequestException("Users invalid."); + } + + if (deletingUserId.HasValue && filteredUsers.Exists(u => u.UserId == deletingUserId.Value)) + { + throw new BadRequestException("You cannot remove yourself."); + } + + var owners = filteredUsers.Where(u => u.Type == OrganizationUserType.Owner); + if (!owners.Any() && deletingUserId.HasValue && !await UserIsOwnerAsync(organizationId, deletingUserId.Value)) + { + throw new BadRequestException("Only owners can delete other owners."); + } + + if (!await HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId)) + { + throw new BadRequestException("Organization must have at least one confirmed owner."); + } + + foreach (var orgUser in filteredUsers) + { + // TODO: We should replace this call with `DeleteManyAsync`. + await _organizationUserRepository.DeleteAsync(orgUser); + await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed); + + if (orgUser.UserId.HasValue) + { + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); + } + } + } + + private async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId) + { + var confirmedOwners = await GetConfirmedOwnersAsync(organizationId); + var confirmedOwnersIds = confirmedOwners.Select(u => u.Id); + return confirmedOwnersIds.Except(organizationUsersId).Any(); + } + + private async Task UserIsOwnerAsync(Guid organizationId, Guid deletingUserId) + { + var deletingUserOrgs = await _organizationUserRepository.GetManyByUserAsync(deletingUserId); + return deletingUserOrgs.Any(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Owner); + } + public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) { if (loggedInUserId.HasValue) @@ -1716,6 +1751,15 @@ namespace Bit.Core.Services return owners.Where(o => o.Status == OrganizationUserStatusType.Confirmed); } + private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId) + { + var deviceIds = await GetUserDeviceIdsAsync(userId); + await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(deviceIds, + organizationId.ToString()); + await _pushNotificationService.PushSyncOrgKeysAsync(userId); + } + + private async Task> GetUserDeviceIdsAsync(Guid userId) { var devices = await _deviceRepository.GetManyByUserIdAsync(userId); diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index b36021d35a..47978055db 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Bit.Core.Models.Data; using Bit.Core.Models.Table; @@ -372,9 +373,169 @@ namespace Bit.Core.Test.Services newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId; savingUser.Type = OrganizationUserType.Owner; organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); + organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) + .Returns(new List { savingUser }); organizationUserRepository.GetManyByUserAsync(savingUser.UserId.Value).Returns(new List { savingUser }); await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections); } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUser_InvalidUser(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUserAsync(Guid.NewGuid(), organizationUser.Id, deletingUser.UserId)); + Assert.Contains("User not valid.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUser_RemoveYourself(OrganizationUser deletingUser, SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, deletingUser.Id, deletingUser.UserId)); + Assert.Contains("You cannot remove yourself.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUser_NonOwnerRemoveOwner(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUser.Type = OrganizationUserType.Owner; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId)); + Assert.Contains("Only owners can delete other owners.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUser_LastOwner(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUser.Type = OrganizationUserType.Owner; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) + .Returns(new[] { organizationUser }); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, null)); + Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUser_Success(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + deletingUser.Type = OrganizationUserType.Owner; + deletingUser.Status = OrganizationUserStatusType.Confirmed; + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); + organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) + .Returns(new[] {deletingUser, organizationUser}); + + await sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUsers_FilterInvalid(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationUsers = new[] { organizationUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(organizationUserIds).Returns(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId)); + Assert.Contains("Users invalid.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUsers_RemoveYourself(OrganizationUser deletingUser, SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationUsers = new[] { deletingUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(organizationUserIds).Returns(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId)); + Assert.Contains("You cannot remove yourself.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUsers_NonOwnerRemoveOwner(OrganizationUser deletingUser, OrganizationUser orgUser1, OrganizationUser orgUser2, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + deletingUser.Type = OrganizationUserType.Admin; + orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; + var organizationUsers = new[] { orgUser1, orgUser2 }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(organizationUserIds).Returns(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId)); + Assert.Contains("Only owners can delete other owners.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUsers_LastOwner(OrganizationUser orgUser, SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + orgUser.Type = OrganizationUserType.Owner; + orgUser.Status = OrganizationUserStatusType.Confirmed; + var organizationUsers = new[] { orgUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(organizationUserIds).Returns(organizationUsers); + organizationUserRepository.GetManyByOrganizationAsync(orgUser.OrganizationId, OrganizationUserType.Owner).Returns(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.DeleteUsersAsync(orgUser.OrganizationId, organizationUserIds, null)); + Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUsers_Success(OrganizationUser deletingUser, OrganizationUser orgUser1, OrganizationUser orgUser2, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + deletingUser.Type = OrganizationUserType.Owner; + deletingUser.Status = OrganizationUserStatusType.Confirmed; + orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; + orgUser1.Type = OrganizationUserType.Owner; + var organizationUsers = new[] { orgUser1, orgUser2 }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(organizationUserIds).Returns(organizationUsers); + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); + organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) + .Returns(new[] {deletingUser, orgUser1}); + + await sutProvider.Sut.DeleteUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); + } } }