From 93e49ffe742fc65ed3b7a0d11498c9a61bb16782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:33:00 +0100 Subject: [PATCH] =?UTF-8?q?[AC-607]=C2=A0Extract=20IOrganizationService.De?= =?UTF-8?q?leteUserAsync=20into=20IRemoveOrganizationUserCommand=20(#4803)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add HasConfirmedOwnersExceptQuery class, interface and unit tests * Register IHasConfirmedOwnersExceptQuery for dependency injection * Replace OrganizationService.HasConfirmedOwnersExceptAsync with HasConfirmedOwnersExceptQuery * Refactor DeleteManagedOrganizationUserAccountCommand to use IHasConfirmedOwnersExceptQuery * Fix unit tests * Extract IOrganizationService.RemoveUserAsync into IRemoveOrganizationUserCommand; Update unit tests * Extract IOrganizationService.RemoveUsersAsync into IRemoveOrganizationUserCommand; Update unit tests * Refactor RemoveUserAsync(Guid organizationId, Guid userId) to use ValidateDeleteUser * Refactor RemoveOrganizationUserCommandTests to use more descriptive method names * Refactor controller actions to accept Guid directly instead of parsing strings * Add unit tests for removing OrganizationUser by UserId * Refactor remove OrganizationUser by UserId method * Add summary to IHasConfirmedOwnersExceptQuery --- .../RemoveOrganizationFromProviderCommand.cs | 8 +- ...oveOrganizationFromProviderCommandTests.cs | 9 +- .../OrganizationUsersController.cs | 17 +- .../Controllers/OrganizationsController.cs | 16 +- .../Public/Controllers/MembersController.cs | 7 +- .../Auth/Controllers/TwoFactorController.cs | 7 +- ...teManagedOrganizationUserAccountCommand.cs | 11 +- .../HasConfirmedOwnersExceptQuery.cs | 41 ++ .../IHasConfirmedOwnersExceptQuery.cs | 12 + .../IRemoveOrganizationUserCommand.cs | 6 +- .../RemoveOrganizationUserCommand.cs | 155 ++++++- .../UpdateOrganizationUserCommand.cs | 7 +- .../Services/IOrganizationService.cs | 8 - .../Implementations/OrganizationService.cs | 161 +------ .../Services/Implementations/PolicyService.cs | 10 +- .../Implementations/EmergencyAccessService.cs | 8 +- ...OrganizationServiceCollectionExtensions.cs | 1 + src/Core/Services/IUserService.cs | 6 +- .../Services/Implementations/UserService.cs | 20 +- .../OrganizationsControllerTests.cs | 16 +- .../OrganizationsControllerTests.cs | 34 +- ...agedOrganizationUserAccountCommandTests.cs | 8 +- .../HasConfirmedOwnersExceptQueryTests.cs | 139 ++++++ .../RemoveOrganizationUserCommandTests.cs | 289 +++++++++++-- .../UpdateOrganizationUserCommandTests.cs | 5 +- .../Services/OrganizationServiceTests.cs | 405 +++--------------- .../Services/PolicyServiceTests.cs | 13 +- test/Core.Test/Services/UserServiceTests.cs | 4 +- 28 files changed, 781 insertions(+), 642 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQuery.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IHasConfirmedOwnersExceptQuery.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQueryTests.cs diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs index 866d18f9a..045fd5059 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Constants; @@ -27,6 +28,7 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv private readonly IFeatureService _featureService; private readonly IProviderBillingService _providerBillingService; private readonly ISubscriberService _subscriberService; + private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; public RemoveOrganizationFromProviderCommand( IEventService eventService, @@ -37,7 +39,8 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv IStripeAdapter stripeAdapter, IFeatureService featureService, IProviderBillingService providerBillingService, - ISubscriberService subscriberService) + ISubscriberService subscriberService, + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) { _eventService = eventService; _mailService = mailService; @@ -48,6 +51,7 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv _featureService = featureService; _providerBillingService = providerBillingService; _subscriberService = subscriberService; + _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; } public async Task RemoveOrganizationFromProvider( @@ -63,7 +67,7 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv throw new BadRequestException("Failed to remove organization. Please contact support."); } - if (!await _organizationService.HasConfirmedOwnersExceptAsync( + if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync( providerOrganization.OrganizationId, Array.Empty(), includeProvider: false)) diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs index 064dae26d..e984259e9 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs @@ -3,6 +3,7 @@ using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Constants; using Bit.Core.Billing.Enums; @@ -75,7 +76,7 @@ public class RemoveOrganizationFromProviderCommandTests { providerOrganization.ProviderId = provider.Id; - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( providerOrganization.OrganizationId, [], includeProvider: false) @@ -98,7 +99,7 @@ public class RemoveOrganizationFromProviderCommandTests organization.GatewayCustomerId = null; organization.GatewaySubscriptionId = null; - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( providerOrganization.OrganizationId, [], includeProvider: false) @@ -141,7 +142,7 @@ public class RemoveOrganizationFromProviderCommandTests { providerOrganization.ProviderId = provider.Id; - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( providerOrganization.OrganizationId, [], includeProvider: false) @@ -208,7 +209,7 @@ public class RemoveOrganizationFromProviderCommandTests var teamsMonthlyPlan = StaticStore.GetPlan(PlanType.TeamsMonthly); - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( providerOrganization.OrganizationId, [], includeProvider: false) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 83beb7e07..af0aede5a 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -51,6 +51,7 @@ public class OrganizationUsersController : Controller private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IDeleteManagedOrganizationUserAccountCommand _deleteManagedOrganizationUserAccountCommand; public OrganizationUsersController( @@ -71,6 +72,7 @@ public class OrganizationUsersController : Controller ISsoConfigRepository ssoConfigRepository, IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, + IRemoveOrganizationUserCommand removeOrganizationUserCommand, IDeleteManagedOrganizationUserAccountCommand deleteManagedOrganizationUserAccountCommand) { _organizationRepository = organizationRepository; @@ -90,6 +92,7 @@ public class OrganizationUsersController : Controller _ssoConfigRepository = ssoConfigRepository; _organizationUserUserDetailsQuery = organizationUserUserDetailsQuery; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; + _removeOrganizationUserCommand = removeOrganizationUserCommand; _deleteManagedOrganizationUserAccountCommand = deleteManagedOrganizationUserAccountCommand; } @@ -502,30 +505,28 @@ public class OrganizationUsersController : Controller [HttpDelete("{id}")] [HttpPost("{id}/remove")] - public async Task Remove(string orgId, string id) + public async Task Remove(Guid orgId, Guid id) { - var orgGuidId = new Guid(orgId); - if (!await _currentContext.ManageUsers(orgGuidId)) + if (!await _currentContext.ManageUsers(orgId)) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User); - await _organizationService.RemoveUserAsync(orgGuidId, new Guid(id), userId.Value); + await _removeOrganizationUserCommand.RemoveUserAsync(orgId, id, userId.Value); } [HttpDelete("")] [HttpPost("remove")] - public async Task> BulkRemove(string orgId, [FromBody] OrganizationUserBulkRequestModel model) + public async Task> BulkRemove(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) { - var orgGuidId = new Guid(orgId); - if (!await _currentContext.ManageUsers(orgGuidId)) + if (!await _currentContext.ManageUsers(orgId)) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User); - var result = await _organizationService.RemoveUsersAsync(orgGuidId, model.Ids, userId.Value); + var result = await _removeOrganizationUserCommand.RemoveUsersAsync(orgId, model.Ids, userId.Value); return new ListResponseModel(result.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index e5dbcd10b..6bcf75b35 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -13,6 +13,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Business.Tokenables; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationApiKeys.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; @@ -55,6 +56,7 @@ public class OrganizationsController : Controller private readonly IProviderRepository _providerRepository; private readonly IProviderBillingService _providerBillingService; private readonly IDataProtectorTokenFactory _orgDeleteTokenDataFactory; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; public OrganizationsController( IOrganizationRepository organizationRepository, @@ -74,7 +76,8 @@ public class OrganizationsController : Controller IPushNotificationService pushNotificationService, IProviderRepository providerRepository, IProviderBillingService providerBillingService, - IDataProtectorTokenFactory orgDeleteTokenDataFactory) + IDataProtectorTokenFactory orgDeleteTokenDataFactory, + IRemoveOrganizationUserCommand removeOrganizationUserCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -94,6 +97,7 @@ public class OrganizationsController : Controller _providerRepository = providerRepository; _providerBillingService = providerBillingService; _orgDeleteTokenDataFactory = orgDeleteTokenDataFactory; + _removeOrganizationUserCommand = removeOrganizationUserCommand; } [HttpGet("{id}")] @@ -229,24 +233,22 @@ public class OrganizationsController : Controller } [HttpPost("{id}/leave")] - public async Task Leave(string id) + public async Task Leave(Guid id) { - var orgGuidId = new Guid(id); - if (!await _currentContext.OrganizationUser(orgGuidId)) + if (!await _currentContext.OrganizationUser(id)) { throw new NotFoundException(); } var user = await _userService.GetUserByPrincipalAsync(User); - var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(orgGuidId); + var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(id); if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.KeyConnector && user.UsesKeyConnector) { throw new BadRequestException("Your organization's Single Sign-On settings prevent you from leaving."); } - - await _organizationService.RemoveUserAsync(orgGuidId, user.Id); + await _removeOrganizationUserCommand.RemoveUserAsync(id, user.Id); } [HttpDelete("{id}")] diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs index 0eb5409ee..7515111d2 100644 --- a/src/Api/AdminConsole/Public/Controllers/MembersController.cs +++ b/src/Api/AdminConsole/Public/Controllers/MembersController.cs @@ -28,6 +28,7 @@ public class MembersController : Controller private readonly IPaymentService _paymentService; private readonly IOrganizationRepository _organizationRepository; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; public MembersController( IOrganizationUserRepository organizationUserRepository, @@ -40,7 +41,8 @@ public class MembersController : Controller IApplicationCacheService applicationCacheService, IPaymentService paymentService, IOrganizationRepository organizationRepository, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, + IRemoveOrganizationUserCommand removeOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; _groupRepository = groupRepository; @@ -53,6 +55,7 @@ public class MembersController : Controller _paymentService = paymentService; _organizationRepository = organizationRepository; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; + _removeOrganizationUserCommand = removeOrganizationUserCommand; } /// @@ -233,7 +236,7 @@ public class MembersController : Controller { return new NotFoundResult(); } - await _organizationService.RemoveUserAsync(_currentContext.OrganizationId.Value, id, null); + await _removeOrganizationUserCommand.RemoveUserAsync(_currentContext.OrganizationId.Value, id, null); return new OkResult(); } diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 149237e28..f2578fbc6 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -141,7 +141,7 @@ public class TwoFactorController : Controller throw new BadRequestException("UserVerificationToken", "User verification failed."); } - await _userService.DisableTwoFactorProviderAsync(user, model.Type.Value, _organizationService); + await _userService.DisableTwoFactorProviderAsync(user, model.Type.Value); return new TwoFactorProviderResponseModel(model.Type.Value, user); } @@ -396,7 +396,7 @@ public class TwoFactorController : Controller public async Task PutDisable([FromBody] TwoFactorProviderRequestModel model) { var user = await CheckAsync(model, false); - await _userService.DisableTwoFactorProviderAsync(user, model.Type.Value, _organizationService); + await _userService.DisableTwoFactorProviderAsync(user, model.Type.Value); var response = new TwoFactorProviderResponseModel(model.Type.Value, user); return response; } @@ -437,8 +437,7 @@ public class TwoFactorController : Controller [AllowAnonymous] public async Task PostRecover([FromBody] TwoFactorRecoveryRequestModel model) { - if (!await _userService.RecoverTwoFactorAsync(model.Email, model.MasterPasswordHash, model.RecoveryCode, - _organizationService)) + if (!await _userService.RecoverTwoFactorAsync(model.Email, model.MasterPasswordHash, model.RecoveryCode)) { await Task.Delay(2000); throw new BadRequestException(string.Empty, "Invalid information. Try again."); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs index d70d061c8..0bcd16cee 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommand.cs @@ -18,7 +18,8 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IUserRepository _userRepository; private readonly ICurrentContext _currentContext; - private readonly IOrganizationService _organizationService; + private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; + public DeleteManagedOrganizationUserAccountCommand( IUserService userService, IEventService eventService, @@ -26,7 +27,7 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz IOrganizationUserRepository organizationUserRepository, IUserRepository userRepository, ICurrentContext currentContext, - IOrganizationService organizationService) + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) { _userService = userService; _eventService = eventService; @@ -34,7 +35,7 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz _organizationUserRepository = organizationUserRepository; _userRepository = userRepository; _currentContext = currentContext; - _organizationService = organizationService; + _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; } public async Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId) @@ -46,7 +47,7 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz } var managementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(organizationId, new[] { organizationUserId }); - var hasOtherConfirmedOwners = await _organizationService.HasConfirmedOwnersExceptAsync(organizationId, new[] { organizationUserId }, includeProvider: true); + var hasOtherConfirmedOwners = await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, new[] { organizationUserId }, includeProvider: true); await ValidateDeleteUserAsync(organizationId, organizationUser, deletingUserId, managementStatus, hasOtherConfirmedOwners); @@ -67,7 +68,7 @@ public class DeleteManagedOrganizationUserAccountCommand : IDeleteManagedOrganiz var users = await _userRepository.GetManyAsync(userIds); var managementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(organizationId, orgUserIds); - var hasOtherConfirmedOwners = await _organizationService.HasConfirmedOwnersExceptAsync(organizationId, orgUserIds, includeProvider: true); + var hasOtherConfirmedOwners = await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, orgUserIds, includeProvider: true); var results = new List<(Guid OrganizationUserId, string? ErrorMessage)>(); foreach (var orgUserId in orgUserIds) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQuery.cs new file mode 100644 index 000000000..746042ea3 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQuery.cs @@ -0,0 +1,41 @@ +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; + +public class HasConfirmedOwnersExceptQuery : IHasConfirmedOwnersExceptQuery +{ + private readonly IProviderUserRepository _providerUserRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; + + public HasConfirmedOwnersExceptQuery( + IProviderUserRepository providerUserRepository, + IOrganizationUserRepository organizationUserRepository) + { + _providerUserRepository = providerUserRepository; + _organizationUserRepository = organizationUserRepository; + } + + public async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true) + { + var confirmedOwners = await GetConfirmedOwnersAsync(organizationId); + var confirmedOwnersIds = confirmedOwners.Select(u => u.Id); + bool hasOtherOwner = confirmedOwnersIds.Except(organizationUsersId).Any(); + if (!hasOtherOwner && includeProvider) + { + return (await _providerUserRepository.GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed)).Any(); + } + return hasOtherOwner; + } + + private async Task> GetConfirmedOwnersAsync(Guid organizationId) + { + var owners = await _organizationUserRepository.GetManyByOrganizationAsync(organizationId, + OrganizationUserType.Owner); + return owners.Where(o => o.Status == OrganizationUserStatusType.Confirmed); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IHasConfirmedOwnersExceptQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IHasConfirmedOwnersExceptQuery.cs new file mode 100644 index 000000000..2964f7116 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IHasConfirmedOwnersExceptQuery.cs @@ -0,0 +1,12 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IHasConfirmedOwnersExceptQuery +{ + /// + /// Checks if an organization has any confirmed owners except for the ones in the list. + /// + /// The organization ID. + /// The organization user IDs to exclude. + /// Whether to include the provider users in the count. + Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs index 3213762ea..583645a89 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs @@ -1,4 +1,5 @@ -using Bit.Core.Enums; +using Bit.Core.Entities; +using Bit.Core.Enums; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; @@ -7,4 +8,7 @@ public interface IRemoveOrganizationUserCommand Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId); Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, EventSystemUser eventSystemUser); + Task RemoveUserAsync(Guid organizationId, Guid userId); + Task>> RemoveUsersAsync(Guid organizationId, + IEnumerable organizationUserIds, Guid? deletingUserId); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs index e2aea0249..09444306e 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs @@ -1,4 +1,6 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -8,38 +10,171 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand { + private readonly IDeviceRepository _deviceRepository; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IOrganizationService _organizationService; + private readonly IEventService _eventService; + private readonly IPushNotificationService _pushNotificationService; + private readonly IPushRegistrationService _pushRegistrationService; + private readonly ICurrentContext _currentContext; + private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; public RemoveOrganizationUserCommand( + IDeviceRepository deviceRepository, IOrganizationUserRepository organizationUserRepository, - IOrganizationService organizationService - ) + IEventService eventService, + IPushNotificationService pushNotificationService, + IPushRegistrationService pushRegistrationService, + ICurrentContext currentContext, + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) { + _deviceRepository = deviceRepository; _organizationUserRepository = organizationUserRepository; - _organizationService = organizationService; + _eventService = eventService; + _pushNotificationService = pushNotificationService; + _pushRegistrationService = pushRegistrationService; + _currentContext = currentContext; + _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; } public async Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId) { - await ValidateDeleteUserAsync(organizationId, organizationUserId); + var organizationUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); + ValidateDeleteUser(organizationId, organizationUser); - await _organizationService.RemoveUserAsync(organizationId, organizationUserId, deletingUserId); + await RepositoryDeleteUserAsync(organizationUser, deletingUserId); + + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); } public async Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, EventSystemUser eventSystemUser) { - await ValidateDeleteUserAsync(organizationId, organizationUserId); + var organizationUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); + ValidateDeleteUser(organizationId, organizationUser); - await _organizationService.RemoveUserAsync(organizationId, organizationUserId, eventSystemUser); + await RepositoryDeleteUserAsync(organizationUser, null); + + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed, eventSystemUser); } - private async Task ValidateDeleteUserAsync(Guid organizationId, Guid organizationUserId) + public async Task RemoveUserAsync(Guid organizationId, Guid userId) + { + var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId); + ValidateDeleteUser(organizationId, organizationUser); + + await RepositoryDeleteUserAsync(organizationUser, null); + + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); + } + + public async Task>> RemoveUsersAsync(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 (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId)) + { + throw new BadRequestException("Organization must have at least one confirmed owner."); + } + + var deletingUserIsOwner = false; + if (deletingUserId.HasValue) + { + deletingUserIsOwner = await _currentContext.OrganizationOwner(organizationId); + } + + var result = new List>(); + var deletedUserIds = new List(); + foreach (var orgUser in filteredUsers) + { + try + { + if (deletingUserId.HasValue && orgUser.UserId == deletingUserId) + { + throw new BadRequestException("You cannot remove yourself."); + } + + if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue && !deletingUserIsOwner) + { + throw new BadRequestException("Only owners can delete other owners."); + } + + await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed); + + if (orgUser.UserId.HasValue) + { + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); + } + result.Add(Tuple.Create(orgUser, "")); + deletedUserIds.Add(orgUser.Id); + } + catch (BadRequestException e) + { + result.Add(Tuple.Create(orgUser, e.Message)); + } + + await _organizationUserRepository.DeleteManyAsync(deletedUserIds); + } + + return result; + } + + private void ValidateDeleteUser(Guid organizationId, OrganizationUser orgUser) { - var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); if (orgUser == null || orgUser.OrganizationId != organizationId) { throw new NotFoundException("User not found."); } } + + private async Task RepositoryDeleteUserAsync(OrganizationUser orgUser, Guid? deletingUserId) + { + if (deletingUserId.HasValue && orgUser.UserId == deletingUserId.Value) + { + throw new BadRequestException("You cannot remove yourself."); + } + + if (orgUser.Type == OrganizationUserType.Owner) + { + if (deletingUserId.HasValue && !await _currentContext.OrganizationOwner(orgUser.OrganizationId)) + { + throw new BadRequestException("Only owners can delete other owners."); + } + + if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(orgUser.OrganizationId, new[] { orgUser.Id }, includeProvider: true)) + { + throw new BadRequestException("Organization must have at least one confirmed owner."); + } + } + + await _organizationUserRepository.DeleteAsync(orgUser); + + if (orgUser.UserId.HasValue) + { + await DeleteAndPushUserRegistrationAsync(orgUser.OrganizationId, orgUser.UserId.Value); + } + } + + private async Task>> GetUserDeviceIdsAsync(Guid userId) + { + var devices = await _deviceRepository.GetManyByUserIdAsync(userId); + return devices + .Where(d => !string.IsNullOrWhiteSpace(d.PushToken)) + .Select(d => new KeyValuePair(d.Id.ToString(), d.Type)); + } + + private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId) + { + var devices = await GetUserDeviceIdsAsync(userId); + await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(devices, + organizationId.ToString()); + await _pushNotificationService.PushSyncOrgKeysAsync(userId); + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index cf8068c5b..c5a4b3da1 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -22,6 +22,7 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; private readonly ICollectionRepository _collectionRepository; private readonly IGroupRepository _groupRepository; + private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; public UpdateOrganizationUserCommand( IEventService eventService, @@ -31,7 +32,8 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, ICollectionRepository collectionRepository, - IGroupRepository groupRepository) + IGroupRepository groupRepository, + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) { _eventService = eventService; _organizationService = organizationService; @@ -41,6 +43,7 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _collectionRepository = collectionRepository; _groupRepository = groupRepository; + _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; } /// @@ -87,7 +90,7 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand await _organizationService.ValidateOrganizationCustomPermissionsEnabledAsync(user.OrganizationId, user.Type); if (user.Type != OrganizationUserType.Owner && - !await _organizationService.HasConfirmedOwnersExceptAsync(user.OrganizationId, new[] { user.Id })) + !await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(user.OrganizationId, new[] { user.Id })) { throw new BadRequestException("Organization must have at least one confirmed owner."); } diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index a406d75fb..646ae6616 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -52,20 +52,12 @@ public interface IOrganizationService Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId); Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, Guid confirmingUserId); - [Obsolete("IRemoveOrganizationUserCommand should be used instead. To be removed by EC-607.")] - Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId); - [Obsolete("IRemoveOrganizationUserCommand should be used instead. To be removed by EC-607.")] - Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, EventSystemUser systemUser); - Task RemoveUserAsync(Guid organizationId, Guid userId); - Task>> RemoveUsersAsync(Guid organizationId, - IEnumerable organizationUserIds, Guid? deletingUserId); Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId); Task ImportAsync(Guid organizationId, IEnumerable groups, IEnumerable newUsers, IEnumerable removeUserExternalIds, bool overwriteExisting, EventSystemUser eventSystemUser); Task DeleteSsoUserAsync(Guid userId, Guid? organizationId); Task UpdateOrganizationKeysAsync(Guid orgId, string publicKey, string privateKey); - Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true); Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId); Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); Task>> RevokeUsersAsync(Guid organizationId, diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 86854503c..f453a22b4 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -73,6 +73,7 @@ public class OrganizationService : IOrganizationService private readonly IFeatureService _featureService; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IOrganizationBillingService _organizationBillingService; + private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; public OrganizationService( IOrganizationRepository organizationRepository, @@ -107,7 +108,8 @@ public class OrganizationService : IOrganizationService IProviderRepository providerRepository, IFeatureService featureService, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IOrganizationBillingService organizationBillingService) + IOrganizationBillingService organizationBillingService, + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -142,6 +144,7 @@ public class OrganizationService : IOrganizationService _featureService = featureService; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _organizationBillingService = organizationBillingService; + _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; } public async Task ReplacePaymentMethodAsync(Guid organizationId, string paymentToken, @@ -1074,7 +1077,7 @@ public class OrganizationService : IOrganizationService } var invitedAreAllOwners = invites.All(i => i.invite.Type == OrganizationUserType.Owner); - if (!invitedAreAllOwners && !await HasConfirmedOwnersExceptAsync(organizationId, new Guid[] { }, includeProvider: true)) + if (!invitedAreAllOwners && !await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, new Guid[] { }, includeProvider: true)) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -1524,149 +1527,6 @@ public class OrganizationService : IOrganizationService } } - [Obsolete("IRemoveOrganizationUserCommand should be used instead. To be removed by EC-607.")] - public async Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId) - { - var orgUser = await RepositoryDeleteUserAsync(organizationId, organizationUserId, deletingUserId); - await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed); - } - - [Obsolete("IRemoveOrganizationUserCommand should be used instead. To be removed by EC-607.")] - public async Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, - EventSystemUser systemUser) - { - var orgUser = await RepositoryDeleteUserAsync(organizationId, organizationUserId, null); - await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed, systemUser); - } - - private async Task RepositoryDeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId) - { - var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); - if (orgUser == null || orgUser.OrganizationId != organizationId) - { - throw new BadRequestException("User not valid."); - } - - if (deletingUserId.HasValue && orgUser.UserId == deletingUserId.Value) - { - throw new BadRequestException("You cannot remove yourself."); - } - - if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue && - !await _currentContext.OrganizationOwner(organizationId)) - { - throw new BadRequestException("Only owners can delete other owners."); - } - - if (!await HasConfirmedOwnersExceptAsync(organizationId, new[] { organizationUserId }, includeProvider: true)) - { - throw new BadRequestException("Organization must have at least one confirmed owner."); - } - - await _organizationUserRepository.DeleteAsync(orgUser); - - if (orgUser.UserId.HasValue) - { - await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); - } - - return orgUser; - } - - public async Task RemoveUserAsync(Guid organizationId, Guid userId) - { - var orgUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId); - if (orgUser == null) - { - throw new NotFoundException(); - } - - if (!await HasConfirmedOwnersExceptAsync(organizationId, new[] { orgUser.Id })) - { - throw new BadRequestException("Organization must have at least one confirmed owner."); - } - - await _organizationUserRepository.DeleteAsync(orgUser); - await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed); - - if (orgUser.UserId.HasValue) - { - await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); - } - } - - public async Task>> RemoveUsersAsync(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 (!await HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId)) - { - throw new BadRequestException("Organization must have at least one confirmed owner."); - } - - var deletingUserIsOwner = false; - if (deletingUserId.HasValue) - { - deletingUserIsOwner = await _currentContext.OrganizationOwner(organizationId); - } - - var result = new List>(); - var deletedUserIds = new List(); - foreach (var orgUser in filteredUsers) - { - try - { - if (deletingUserId.HasValue && orgUser.UserId == deletingUserId) - { - throw new BadRequestException("You cannot remove yourself."); - } - - if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue && !deletingUserIsOwner) - { - throw new BadRequestException("Only owners can delete other owners."); - } - - await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Removed); - - if (orgUser.UserId.HasValue) - { - await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); - } - result.Add(Tuple.Create(orgUser, "")); - deletedUserIds.Add(orgUser.Id); - } - catch (BadRequestException e) - { - result.Add(Tuple.Create(orgUser, e.Message)); - } - - await _organizationUserRepository.DeleteManyAsync(deletedUserIds); - } - - return result; - } - - public async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true) - { - var confirmedOwners = await GetConfirmedOwnersAsync(organizationId); - var confirmedOwnersIds = confirmedOwners.Select(u => u.Id); - bool hasOtherOwner = confirmedOwnersIds.Except(organizationUsersId).Any(); - if (!hasOtherOwner && includeProvider) - { - return (await _providerUserRepository.GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed)).Any(); - } - return hasOtherOwner; - } - public async Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId) { // Org User must be the same as the calling user and the organization ID associated with the user must match passed org ID @@ -1963,13 +1823,6 @@ public class OrganizationService : IOrganizationService await _groupRepository.UpdateUsersAsync(group.Id, users); } - private async Task> GetConfirmedOwnersAsync(Guid organizationId) - { - var owners = await _organizationUserRepository.GetManyByOrganizationAsync(organizationId, - OrganizationUserType.Owner); - return owners.Where(o => o.Status == OrganizationUserStatusType.Confirmed); - } - private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId) { var devices = await GetUserDeviceIdsAsync(userId); @@ -2274,7 +2127,7 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Already revoked."); } - if (!await HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId, new[] { organizationUser.Id }, includeProvider: true)) + if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId, new[] { organizationUser.Id }, includeProvider: true)) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -2295,7 +2148,7 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Users invalid."); } - if (!await HasConfirmedOwnersExceptAsync(organizationId, organizationUserIds)) + if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, organizationUserIds)) { throw new BadRequestException("Organization must have at least one confirmed owner."); } diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index fab32aaff..7e689f034 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; @@ -26,6 +27,7 @@ public class PolicyService : IPolicyService private readonly IMailService _mailService; private readonly GlobalSettings _globalSettings; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; public PolicyService( IApplicationCacheService applicationCacheService, @@ -36,7 +38,8 @@ public class PolicyService : IPolicyService ISsoConfigRepository ssoConfigRepository, IMailService mailService, GlobalSettings globalSettings, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, + IRemoveOrganizationUserCommand removeOrganizationUserCommand) { _applicationCacheService = applicationCacheService; _eventService = eventService; @@ -47,6 +50,7 @@ public class PolicyService : IPolicyService _mailService = mailService; _globalSettings = globalSettings; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; + _removeOrganizationUserCommand = removeOrganizationUserCommand; } public async Task SaveAsync(Policy policy, IOrganizationService organizationService, Guid? savingUserId) @@ -284,7 +288,7 @@ public class PolicyService : IPolicyService "Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page."); } - await organizationService.RemoveUserAsync(policy.OrganizationId, orgUser.Id, + await _removeOrganizationUserCommand.RemoveUserAsync(policy.OrganizationId, orgUser.Id, savingUserId); await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( org.DisplayName(), orgUser.Email); @@ -300,7 +304,7 @@ public class PolicyService : IPolicyService && ou.OrganizationId != org.Id && ou.Status != OrganizationUserStatusType.Invited)) { - await organizationService.RemoveUserAsync(policy.OrganizationId, orgUser.Id, + await _removeOrganizationUserCommand.RemoveUserAsync(policy.OrganizationId, orgUser.Id, savingUserId); await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync( org.DisplayName(), orgUser.Email); diff --git a/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs b/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs index dffa9f65c..dda16e29f 100644 --- a/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs +++ b/src/Core/Auth/Services/Implementations/EmergencyAccessService.cs @@ -1,4 +1,5 @@ using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; @@ -33,6 +34,7 @@ public class EmergencyAccessService : IEmergencyAccessService private readonly IPasswordHasher _passwordHasher; private readonly IOrganizationService _organizationService; private readonly IDataProtectorTokenFactory _dataProtectorTokenizer; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; public EmergencyAccessService( IEmergencyAccessRepository emergencyAccessRepository, @@ -46,7 +48,8 @@ public class EmergencyAccessService : IEmergencyAccessService IPasswordHasher passwordHasher, GlobalSettings globalSettings, IOrganizationService organizationService, - IDataProtectorTokenFactory dataProtectorTokenizer) + IDataProtectorTokenFactory dataProtectorTokenizer, + IRemoveOrganizationUserCommand removeOrganizationUserCommand) { _emergencyAccessRepository = emergencyAccessRepository; _organizationUserRepository = organizationUserRepository; @@ -60,6 +63,7 @@ public class EmergencyAccessService : IEmergencyAccessService _globalSettings = globalSettings; _organizationService = organizationService; _dataProtectorTokenizer = dataProtectorTokenizer; + _removeOrganizationUserCommand = removeOrganizationUserCommand; } public async Task InviteAsync(User invitingUser, string email, EmergencyAccessType type, int waitTime) @@ -341,7 +345,7 @@ public class EmergencyAccessService : IEmergencyAccessService { if (o.Type != OrganizationUserType.Owner) { - await _organizationService.RemoveUserAsync(o.OrganizationId, grantor.Id); + await _removeOrganizationUserCommand.RemoveUserAsync(o.OrganizationId, grantor.Id); } } } diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 3e2946248..58eb65faf 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -146,6 +146,7 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); + services.AddScoped(); } // TODO: move to OrganizationSubscriptionServiceCollectionExtensions when OrganizationUser methods are moved out of diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 0135b5f1b..b15f5153e 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -40,10 +40,8 @@ public interface IUserService KdfType kdf, int kdfIterations, int? kdfMemory, int? kdfParallelism); Task RefreshSecurityStampAsync(User user, string masterPasswordHash); Task UpdateTwoFactorProviderAsync(User user, TwoFactorProviderType type, bool setEnabled = true, bool logEvent = true); - Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type, - IOrganizationService organizationService); - Task RecoverTwoFactorAsync(string email, string masterPassword, string recoveryCode, - IOrganizationService organizationService); + Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type); + Task RecoverTwoFactorAsync(string email, string masterPassword, string recoveryCode); Task GenerateUserTokenAsync(User user, string tokenProvider, string purpose); Task DeleteAsync(User user); Task DeleteAsync(User user, string token); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index fe04efa22..413437a59 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; @@ -65,6 +66,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; private readonly IFeatureService _featureService; private readonly IPremiumUserBillingService _premiumUserBillingService; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; public UserService( IUserRepository userRepository, @@ -98,7 +100,8 @@ public class UserService : UserManager, IUserService, IDisposable IStripeSyncService stripeSyncService, IDataProtectorTokenFactory orgUserInviteTokenDataFactory, IFeatureService featureService, - IPremiumUserBillingService premiumUserBillingService) + IPremiumUserBillingService premiumUserBillingService, + IRemoveOrganizationUserCommand removeOrganizationUserCommand) : base( store, optionsAccessor, @@ -138,6 +141,7 @@ public class UserService : UserManager, IUserService, IDisposable _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; _featureService = featureService; _premiumUserBillingService = premiumUserBillingService; + _removeOrganizationUserCommand = removeOrganizationUserCommand; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -827,8 +831,7 @@ public class UserService : UserManager, IUserService, IDisposable } } - public async Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type, - IOrganizationService organizationService) + public async Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type) { var providers = user.GetTwoFactorProviders(); if (!providers?.ContainsKey(type) ?? true) @@ -843,12 +846,11 @@ public class UserService : UserManager, IUserService, IDisposable if (!await TwoFactorIsEnabledAsync(user)) { - await CheckPoliciesOnTwoFactorRemovalAsync(user, organizationService); + await CheckPoliciesOnTwoFactorRemovalAsync(user); } } - public async Task RecoverTwoFactorAsync(string email, string secret, string recoveryCode, - IOrganizationService organizationService) + public async Task RecoverTwoFactorAsync(string email, string secret, string recoveryCode) { var user = await _userRepository.GetByEmailAsync(email); if (user == null) @@ -872,7 +874,7 @@ public class UserService : UserManager, IUserService, IDisposable await SaveUserAsync(user); await _mailService.SendRecoverTwoFactorEmail(user.Email, DateTime.UtcNow, _currentContext.IpAddress); await _eventService.LogUserEventAsync(user.Id, EventType.User_Recovered2fa); - await CheckPoliciesOnTwoFactorRemovalAsync(user, organizationService); + await CheckPoliciesOnTwoFactorRemovalAsync(user); return true; } @@ -1327,13 +1329,13 @@ public class UserService : UserManager, IUserService, IDisposable } } - private async Task CheckPoliciesOnTwoFactorRemovalAsync(User user, IOrganizationService organizationService) + private async Task CheckPoliciesOnTwoFactorRemovalAsync(User user) { var twoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication); var removeOrgUserTasks = twoFactorPolicies.Select(async p => { - await organizationService.RemoveUserAsync(p.OrganizationId, user.Id); + await _removeOrganizationUserCommand.RemoveUserAsync(p.OrganizationId, user.Id); var organization = await _organizationRepository.GetByIdAsync(p.OrganizationId); await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( organization.DisplayName(), user.Email); diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index 156df1476..25227fec7 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs @@ -7,6 +7,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Models.Business.Tokenables; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationApiKeys.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; @@ -49,6 +50,7 @@ public class OrganizationsControllerTests : IDisposable private readonly IProviderRepository _providerRepository; private readonly IProviderBillingService _providerBillingService; private readonly IDataProtectorTokenFactory _orgDeleteTokenDataFactory; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly OrganizationsController _sut; @@ -72,6 +74,7 @@ public class OrganizationsControllerTests : IDisposable _providerRepository = Substitute.For(); _providerBillingService = Substitute.For(); _orgDeleteTokenDataFactory = Substitute.For>(); + _removeOrganizationUserCommand = Substitute.For(); _sut = new OrganizationsController( _organizationRepository, @@ -91,7 +94,8 @@ public class OrganizationsControllerTests : IDisposable _pushNotificationService, _providerRepository, _providerBillingService, - _orgDeleteTokenDataFactory); + _orgDeleteTokenDataFactory, + _removeOrganizationUserCommand); } public void Dispose() @@ -120,13 +124,12 @@ public class OrganizationsControllerTests : IDisposable _ssoConfigRepository.GetByOrganizationIdAsync(orgId).Returns(ssoConfig); _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(user); - var exception = await Assert.ThrowsAsync( - () => _sut.Leave(orgId.ToString())); + var exception = await Assert.ThrowsAsync(() => _sut.Leave(orgId)); Assert.Contains("Your organization's Single Sign-On settings prevent you from leaving.", exception.Message); - await _organizationService.DidNotReceiveWithAnyArgs().RemoveUserAsync(default, default); + await _removeOrganizationUserCommand.DidNotReceiveWithAnyArgs().RemoveUserAsync(default, default); } [Theory] @@ -155,8 +158,9 @@ public class OrganizationsControllerTests : IDisposable _ssoConfigRepository.GetByOrganizationIdAsync(orgId).Returns(ssoConfig); _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(user); - await _organizationService.RemoveUserAsync(orgId, user.Id); - await _organizationService.Received(1).RemoveUserAsync(orgId, user.Id); + await _sut.Leave(orgId); + + await _removeOrganizationUserCommand.Received(1).RemoveUserAsync(orgId, user.Id); } [Theory, AutoData] diff --git a/test/Api.Test/Billing/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/Billing/Controllers/OrganizationsControllerTests.cs index 8fb648e89..ec6047fbf 100644 --- a/test/Api.Test/Billing/Controllers/OrganizationsControllerTests.cs +++ b/test/Api.Test/Billing/Controllers/OrganizationsControllerTests.cs @@ -4,8 +4,8 @@ using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Billing.Controllers; using Bit.Api.Models.Request.Organizations; using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; @@ -46,6 +46,7 @@ public class OrganizationsControllerTests : IDisposable private readonly IAddSecretsManagerSubscriptionCommand _addSecretsManagerSubscriptionCommand; private readonly IReferenceEventService _referenceEventService; private readonly ISubscriberService _subscriberService; + private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly OrganizationsController _sut; @@ -68,6 +69,7 @@ public class OrganizationsControllerTests : IDisposable _addSecretsManagerSubscriptionCommand = Substitute.For(); _referenceEventService = Substitute.For(); _subscriberService = Substitute.For(); + _removeOrganizationUserCommand = Substitute.For(); _sut = new OrganizationsController( _organizationRepository, @@ -91,36 +93,6 @@ public class OrganizationsControllerTests : IDisposable _sut?.Dispose(); } - [Theory] - [InlineAutoData(true, false)] - [InlineAutoData(false, true)] - [InlineAutoData(false, false)] - public async Task OrganizationsController_UserCanLeaveOrganizationThatDoesntProvideKeyConnector( - bool keyConnectorEnabled, bool userUsesKeyConnector, Guid orgId, User user) - { - var ssoConfig = new SsoConfig - { - Id = default, - Data = new SsoConfigurationData - { - MemberDecryptionType = keyConnectorEnabled - ? MemberDecryptionType.KeyConnector - : MemberDecryptionType.MasterPassword - }.Serialize(), - Enabled = true, - OrganizationId = orgId, - }; - - user.UsesKeyConnector = userUsesKeyConnector; - - _currentContext.OrganizationUser(orgId).Returns(true); - _ssoConfigRepository.GetByOrganizationIdAsync(orgId).Returns(ssoConfig); - _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(user); - - await _organizationService.RemoveUserAsync(orgId, user.Id); - await _organizationService.Received(1).RemoveUserAsync(orgId, user.Id); - } - [Theory, AutoData] public async Task OrganizationsController_PostUpgrade_UserCannotEditSubscription_ThrowsNotFoundException( Guid organizationId, diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs index 585c5fc8d..81e83d745 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteManagedOrganizationUserAccountCommandTests.cs @@ -40,7 +40,7 @@ public class DeleteManagedOrganizationUserAccountCommandTests Arg.Is>(ids => ids.Contains(organizationUser.Id))) .Returns(new Dictionary { { organizationUser.Id, true } }); - sutProvider.GetDependency() + sutProvider.GetDependency() .HasConfirmedOwnersExceptAsync( organizationUser.OrganizationId, Arg.Is>(ids => ids.Contains(organizationUser.Id)), @@ -184,7 +184,7 @@ public class DeleteManagedOrganizationUserAccountCommandTests .OrganizationOwner(organizationUser.OrganizationId) .Returns(true); - sutProvider.GetDependency() + sutProvider.GetDependency() .HasConfirmedOwnersExceptAsync( organizationUser.OrganizationId, Arg.Is>(ids => ids.Contains(organizationUser.Id)), @@ -399,8 +399,8 @@ public class DeleteManagedOrganizationUserAccountCommandTests .OrganizationOwner(orgUser.OrganizationId) .Returns(true); - sutProvider.GetDependency() - .HasConfirmedOwnersExceptAsync(orgUser.OrganizationId, Arg.Any>(), true) + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(orgUser.OrganizationId, Arg.Any>(), Arg.Any()) .Returns(false); // Act diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQueryTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQueryTests.cs new file mode 100644 index 000000000..77e99a8e2 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/HasConfirmedOwnersExceptQueryTests.cs @@ -0,0 +1,139 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class HasConfirmedOwnersExceptQueryTests +{ + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExcept_WithConfirmedOwner_WithNoException_ReturnsTrue( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) + .Returns(new List { owner }); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), true); + + Assert.True(result); + } + + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExcept_ExcludingConfirmedOwner_ReturnsFalse( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) + .Returns(new List { owner }); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List { owner.Id }, true); + + Assert.False(result); + } + + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExcept_WithInvitedOwner_ReturnsFalse( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Invited, OrganizationUserType.Owner)] OrganizationUser owner, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) + .Returns(new List { owner }); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), true); + + Assert.False(result); + } + + [Theory] + [BitAutoData(true)] + [BitAutoData(false)] + public async Task HasConfirmedOwnersExcept_WithConfirmedProviderUser_IncludeProviderTrue_ReturnsTrue( + bool includeProvider, + Organization organization, + ProviderUser providerUser, + SutProvider sutProvider) + { + providerUser.Status = ProviderUserStatusType.Confirmed; + + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organization.Id, ProviderUserStatusType.Confirmed) + .Returns(new List { providerUser }); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), includeProvider); + + Assert.Equal(includeProvider, result); + } + + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExceptAsync_WithConfirmedOwners_ReturnsTrue( + Guid organizationId, + IEnumerable organizationUsersId, + ICollection owners, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) + .Returns(owners); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); + + Assert.True(result); + } + + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExceptAsync_WithConfirmedProviders_ReturnsTrue( + Guid organizationId, + IEnumerable organizationUsersId, + ICollection providerUsers, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) + .Returns(new List()); + + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed) + .Returns(providerUsers); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); + + Assert.True(result); + } + + [Theory, BitAutoData] + public async Task HasConfirmedOwnersExceptAsync_WithNoConfirmedOwnersOrProviders_ReturnsFalse( + Guid organizationId, + IEnumerable organizationUsersId, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) + .Returns(new List()); + + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed) + .Returns(new List()); + + var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); + + Assert.False(result); + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs index 8d25f13cb..2d10ce626 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs @@ -1,9 +1,12 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +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; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -14,33 +17,38 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers; [SutProviderCustomize] public class RemoveOrganizationUserCommandTests { - [Theory] - [BitAutoData] - public async Task RemoveUser_Success(SutProvider sutProvider, Guid organizationId, Guid organizationUserId) + [Theory, BitAutoData] + public async Task RemoveUser_Success( + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser deletingUser, + SutProvider sutProvider) { - sutProvider.GetDependency() - .GetByIdAsync(organizationUserId) - .Returns(new OrganizationUser - { - Id = organizationUserId, - OrganizationId = organizationId - }); + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); - await sutProvider.Sut.RemoveUserAsync(organizationId, organizationUserId, null); + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); - await sutProvider.GetDependency().Received(1).RemoveUserAsync(organizationId, organizationUserId, null); + await sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId); + + await organizationUserRepository.Received(1).DeleteAsync(organizationUser); + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); } [Theory] [BitAutoData] - public async Task RemoveUser_NotFound_Throws(SutProvider sutProvider, Guid organizationId, Guid organizationUserId) + public async Task RemoveUser_NotFound_ThrowsException(SutProvider sutProvider, + Guid organizationId, Guid organizationUserId) { await Assert.ThrowsAsync(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, organizationUserId, null)); } [Theory] [BitAutoData] - public async Task RemoveUser_MismatchingOrganizationId_Throws(SutProvider sutProvider, Guid organizationId, Guid organizationUserId) + public async Task RemoveUser_MismatchingOrganizationId_ThrowsException( + SutProvider sutProvider, Guid organizationId, Guid organizationUserId) { sutProvider.GetDependency() .GetByIdAsync(organizationUserId) @@ -53,20 +61,249 @@ public class RemoveOrganizationUserCommandTests await Assert.ThrowsAsync(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, organizationUserId, null)); } - [Theory] - [BitAutoData] - public async Task RemoveUser_WithEventSystemUser_Success(SutProvider sutProvider, Guid organizationId, Guid organizationUserId, EventSystemUser eventSystemUser) + [Theory, BitAutoData] + public async Task RemoveUser_InvalidUser_ThrowsException( + OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) { - sutProvider.GetDependency() - .GetByIdAsync(organizationUserId) - .Returns(new OrganizationUser - { - Id = organizationUserId, - OrganizationId = organizationId - }); + var organizationUserRepository = sutProvider.GetDependency(); - await sutProvider.Sut.RemoveUserAsync(organizationId, organizationUserId, eventSystemUser); + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - await sutProvider.GetDependency().Received(1).RemoveUserAsync(organizationId, organizationUserId, eventSystemUser); + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(Guid.NewGuid(), organizationUser.Id, deletingUser.UserId)); + Assert.Contains("User not found.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUser_RemoveYourself_ThrowsException(OrganizationUser deletingUser, SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, deletingUser.Id, deletingUser.UserId)); + Assert.Contains("You cannot remove yourself.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUser_NonOwnerRemoveOwner_ThrowsException( + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, + [OrganizationUser(type: OrganizationUserType.Admin)] OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); + + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + currentContext.OrganizationAdmin(deletingUser.OrganizationId).Returns(true); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId)); + Assert.Contains("Only owners can delete other owners.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUser_RemovingLastOwner_ThrowsException( + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, + OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var hasConfirmedOwnersExceptQuery = sutProvider.GetDependency(); + + organizationUser.OrganizationId = deletingUser.OrganizationId; + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync( + deletingUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), Arg.Any()) + .Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, null)); + Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); + hasConfirmedOwnersExceptQuery + .Received(1) + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), true); + } + + [Theory, BitAutoData] + public async Task RemoveUser_WithEventSystemUser_Success( + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser, + EventSystemUser eventSystemUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); + + await sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.Id, eventSystemUser); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed, eventSystemUser); + } + + [Theory, BitAutoData] + public async Task RemoveUser_ByUserId_Success( + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository + .GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value) + .Returns(organizationUser); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()) + .Returns(true); + + await sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); + } + + [Theory, BitAutoData] + public async Task RemoveUser_ByUserId_NotFound_ThrowsException(SutProvider sutProvider, + Guid organizationId, Guid userId) + { + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, userId)); + } + + [Theory, BitAutoData] + public async Task RemoveUser_ByUserId_InvalidUser_ThrowsException(OrganizationUser organizationUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + organizationUserRepository.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value).Returns(organizationUser); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(Guid.NewGuid(), organizationUser.UserId.Value)); + Assert.Contains("User not found.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUser_ByUserId_RemovingLastOwner_ThrowsException( + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var hasConfirmedOwnersExceptQuery = sutProvider.GetDependency(); + + organizationUserRepository.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value).Returns(organizationUser); + hasConfirmedOwnersExceptQuery + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()) + .Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value)); + Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); + hasConfirmedOwnersExceptQuery + .Received(1) + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RemoveUsers_FilterInvalid_ThrowsException(OrganizationUser organizationUser, OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationUsers = new[] { organizationUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId)); + Assert.Contains("Users invalid.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUsers_RemoveYourself_ThrowsException( + OrganizationUser deletingUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationUsers = new[] { deletingUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(deletingUser.OrganizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); + Assert.Contains("You cannot remove yourself.", result[0].Item2); + } + + [Theory, BitAutoData] + public async Task RemoveUsers_NonOwnerRemoveOwner_ThrowsException( + [OrganizationUser(type: OrganizationUserType.Admin)] OrganizationUser deletingUser, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser orgUser2, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; + var organizationUsers = new[] { orgUser1 }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(deletingUser.OrganizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); + Assert.Contains("Only owners can delete other owners.", result[0].Item2); + } + + [Theory, BitAutoData] + public async Task RemoveUsers_LastOwner_ThrowsException( + [OrganizationUser(status: OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUser, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + + var organizationUsers = new[] { orgUser }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); + organizationUserRepository.GetManyByOrganizationAsync(orgUser.OrganizationId, OrganizationUserType.Owner).Returns(organizationUsers); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RemoveUsersAsync(orgUser.OrganizationId, organizationUserIds, null)); + Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveUsers_Success( + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser deletingUser, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser orgUser1, OrganizationUser orgUser2, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); + + orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; + var organizationUsers = new[] { orgUser1, orgUser2 }; + var organizationUserIds = organizationUsers.Select(u => u.Id); + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); + organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(deletingUser.OrganizationId, Arg.Any>()) + .Returns(true); + currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); + + await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index a7db63fb5..73bf00474 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -1,6 +1,7 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Entities; using Bit.Core.Enums; @@ -169,7 +170,7 @@ public class UpdateOrganizationUserCommandTests await organizationService.Received(1).ValidateOrganizationCustomPermissionsEnabledAsync( newUserData.OrganizationId, newUserData.Type); - await organizationService.Received(1).HasConfirmedOwnersExceptAsync( + await sutProvider.GetDependency().Received(1).HasConfirmedOwnersExceptAsync( newUserData.OrganizationId, Arg.Is>(i => i.Contains(newUserData.Id))); } @@ -187,7 +188,7 @@ public class UpdateOrganizationUserCommandTests newUser.UserId = oldUser.UserId; newUser.OrganizationId = oldUser.OrganizationId = organization.Id; organizationUserRepository.GetByIdAsync(oldUser.Id).Returns(oldUser); - organizationService + sutProvider.GetDependency() .HasConfirmedOwnersExceptAsync( oldUser.OrganizationId, Arg.Is>(i => i.Contains(oldUser.Id))) diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 357222669..147162c66 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -43,6 +43,8 @@ using Xunit; using Organization = Bit.Core.AdminConsole.Entities.Organization; using OrganizationUser = Bit.Core.Entities.OrganizationUser; +#nullable enable + namespace Bit.Core.Test.Services; [SutProviderCustomize] @@ -77,8 +79,9 @@ public class OrganizationServiceTests .Returns(existingUsers); organizationUserRepository.GetCountByOrganizationIdAsync(org.Id) .Returns(existingUsers.Count); - organizationUserRepository.GetManyByOrganizationAsync(org.Id, OrganizationUserType.Owner) - .Returns(existingUsers.Select(u => new OrganizationUser { Status = OrganizationUserStatusType.Confirmed, Type = OrganizationUserType.Owner, Id = u.Id }).ToList()); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(org.Id, Arg.Any>()) + .Returns(true); sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); // Mock tokenable factory to return a token that expires in 5 days @@ -147,8 +150,9 @@ public class OrganizationServiceTests var organizationUserRepository = sutProvider.GetDependency(); - organizationUserRepository.GetManyByOrganizationAsync(org.Id, OrganizationUserType.Owner) - .Returns(existingUsers.Select(u => new OrganizationUser { Status = OrganizationUserStatusType.Confirmed, Type = OrganizationUserType.Owner, Id = u.Id }).ToList()); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(org.Id, Arg.Any>()) + .Returns(true); SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); @@ -708,8 +712,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { invitor }); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); @@ -737,8 +742,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { invitor }); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); @@ -817,8 +823,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { invitor }); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); @@ -835,7 +842,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) ), OrganizationCustomize, BitAutoData] public async Task InviteUser_Passes(Organization organization, OrganizationUserInvite invite, string externalId, OrganizationUser invitor, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { // This method is only used to invite 1 user at a time @@ -851,8 +857,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationUserRepository = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { owner }); // Mock tokenable factory to return a token that expires in 5 days sutProvider.GetDependency() @@ -864,6 +868,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) } ); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); + SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); SetupOrgUserRepositoryCreateAsyncMock(organizationUserRepository); @@ -905,7 +913,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) ), OrganizationCustomize, BitAutoData] public async Task InviteUser_UserAlreadyInvited_Throws(Organization organization, OrganizationUserInvite invite, string externalId, OrganizationUser invitor, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { // This method is only used to invite 1 user at a time @@ -926,8 +933,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationUserRepository = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { owner }); // Mock tokenable factory to return a token that expires in 5 days sutProvider.GetDependency() @@ -939,6 +944,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) } ); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); + SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); SetupOrgUserRepositoryCreateAsyncMock(organizationUserRepository); @@ -987,7 +996,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) ), OrganizationCustomize, BitAutoData] public async Task InviteUsers_Passes(Organization organization, IEnumerable<(OrganizationUserInvite invite, string externalId)> invites, OrganizationUser invitor, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks @@ -1000,8 +1008,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationUserRepository = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { owner }); // Mock tokenable factory to return a token that expires in 5 days sutProvider.GetDependency() @@ -1013,6 +1019,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) } ); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); + SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); SetupOrgUserRepositoryCreateAsyncMock(organizationUserRepository); @@ -1034,7 +1044,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) ), OrganizationCustomize, BitAutoData] public async Task InviteUsers_WithEventSystemUser_Passes(Organization organization, EventSystemUser eventSystemUser, IEnumerable<(OrganizationUserInvite invite, string externalId)> invites, OrganizationUser invitor, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks @@ -1052,8 +1061,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new[] { owner }); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); SetupOrgUserRepositoryCreateAsyncMock(organizationUserRepository); SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); @@ -1181,196 +1191,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) sutProvider.GetDependency().ManageUsers(organization.Id).Returns(true); } - [Theory, BitAutoData] - public async Task RemoveUser_InvalidUser(OrganizationUser organizationUser, OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - - organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUserAsync(Guid.NewGuid(), organizationUser.Id, deletingUser.UserId)); - Assert.Contains("User not valid.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUser_RemoveYourself(OrganizationUser deletingUser, SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - - organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, deletingUser.Id, deletingUser.UserId)); - Assert.Contains("You cannot remove yourself.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUser_NonOwnerRemoveOwner( - [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, - [OrganizationUser(type: OrganizationUserType.Admin)] OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var currentContext = sutProvider.GetDependency(); - - organizationUser.OrganizationId = deletingUser.OrganizationId; - organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - currentContext.OrganizationAdmin(deletingUser.OrganizationId).Returns(true); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId)); - Assert.Contains("Only owners can delete other owners.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUser_LastOwner( - [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, - OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - - organizationUser.OrganizationId = deletingUser.OrganizationId; - organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) - .Returns(new[] { organizationUser }); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, null)); - Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUser_Success( - OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var currentContext = sutProvider.GetDependency(); - - organizationUser.OrganizationId = deletingUser.OrganizationId; - organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) - .Returns(new[] { deletingUser, organizationUser }); - currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); - - await sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId); - - await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); - } - - [Theory, BitAutoData] - public async Task RemoveUser_WithEventSystemUser_Success( - OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser deletingUser, EventSystemUser eventSystemUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var currentContext = sutProvider.GetDependency(); - - organizationUser.OrganizationId = deletingUser.OrganizationId; - organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) - .Returns(new[] { deletingUser, organizationUser }); - currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); - - await sutProvider.Sut.RemoveUserAsync(deletingUser.OrganizationId, organizationUser.Id, eventSystemUser); - - await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed, eventSystemUser); - } - - [Theory, BitAutoData] - public async Task RemoveUsers_FilterInvalid(OrganizationUser organizationUser, OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var organizationUsers = new[] { organizationUser }; - var organizationUserIds = organizationUsers.Select(u => u.Id); - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId)); - Assert.Contains("Users invalid.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUsers_RemoveYourself( - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUser, - OrganizationUser deletingUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var organizationUsers = new[] { deletingUser }; - var organizationUserIds = organizationUsers.Select(u => u.Id); - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); - organizationUserRepository.GetManyByOrganizationAsync(default, default).ReturnsForAnyArgs(new[] { orgUser }); - - var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); - Assert.Contains("You cannot remove yourself.", result[0].Item2); - } - - [Theory, BitAutoData] - public async Task RemoveUsers_NonOwnerRemoveOwner( - [OrganizationUser(type: OrganizationUserType.Admin)] OrganizationUser deletingUser, - [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser orgUser1, - [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser orgUser2, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - - orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; - var organizationUsers = new[] { orgUser1 }; - var organizationUserIds = organizationUsers.Select(u => u.Id); - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); - organizationUserRepository.GetManyByOrganizationAsync(default, default).ReturnsForAnyArgs(new[] { orgUser2 }); - - var result = await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); - Assert.Contains("Only owners can delete other owners.", result[0].Item2); - } - - [Theory, BitAutoData] - public async Task RemoveUsers_LastOwner( - [OrganizationUser(status: OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUser, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - - var organizationUsers = new[] { orgUser }; - var organizationUserIds = organizationUsers.Select(u => u.Id); - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); - organizationUserRepository.GetManyByOrganizationAsync(orgUser.OrganizationId, OrganizationUserType.Owner).Returns(organizationUsers); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveUsersAsync(orgUser.OrganizationId, organizationUserIds, null)); - Assert.Contains("Organization must have at least one confirmed owner.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveUsers_Success( - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser deletingUser, - [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser orgUser1, OrganizationUser orgUser2, - SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var currentContext = sutProvider.GetDependency(); - - orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; - var organizationUsers = new[] { orgUser1, orgUser2 }; - var organizationUserIds = organizationUsers.Select(u => u.Id); - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); - organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) - .Returns(new[] { deletingUser, orgUser1 }); - currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); - - await sutProvider.Sut.RemoveUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); - } - [Theory, BitAutoData] public async Task ConfirmUser_InvalidStatus(OrganizationUser confirmingUser, [OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser orgUser, string key, @@ -1858,17 +1678,24 @@ OrganizationUserInvite invite, SutProvider sutProvider) await applicationCacheService.DidNotReceiveWithAnyArgs().DeleteOrganizationAbilityAsync(default); } - private void RestoreRevokeUser_Setup(Organization organization, OrganizationUser restoringUser, - OrganizationUser organizationUser, SutProvider sutProvider, - OrganizationUserType restoringUserType = OrganizationUserType.Owner) + private void RestoreRevokeUser_Setup( + Organization organization, + OrganizationUser? requestingOrganizationUser, + OrganizationUser targetOrganizationUser, + SutProvider sutProvider) { + if (requestingOrganizationUser != null) + { + requestingOrganizationUser.OrganizationId = organization.Id; + } + targetOrganizationUser.OrganizationId = organization.Id; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - sutProvider.GetDependency().GetByIdAsync(organizationUser.OrganizationId).Returns(organization); - sutProvider.GetDependency().OrganizationOwner(organization.Id).Returns(true); - sutProvider.GetDependency().ManageUsers(organization.Id).Returns(true); - var organizationUserRepository = sutProvider.GetDependency(); - organizationUserRepository.GetManyByOrganizationAsync(organizationUser.OrganizationId, restoringUserType) - .Returns(new[] { restoringUser }); + sutProvider.GetDependency().OrganizationOwner(organization.Id).Returns(requestingOrganizationUser != null && requestingOrganizationUser.Type is OrganizationUserType.Owner); + sutProvider.GetDependency().ManageUsers(organization.Id).Returns(requestingOrganizationUser != null && (requestingOrganizationUser.Type is OrganizationUserType.Owner or OrganizationUserType.Admin)); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); } [Theory, BitAutoData] @@ -1887,10 +1714,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) } [Theory, BitAutoData] - public async Task RevokeUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + public async Task RevokeUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) { - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); + RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); @@ -1917,10 +1743,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) } [Theory, BitAutoData] - public async Task RestoreUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + public async Task RestoreUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) { - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); + RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); @@ -1953,11 +1778,12 @@ OrganizationUserInvite invite, SutProvider sutProvider) [Theory] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Custom)] - public async Task RestoreUser_AdminRestoreOwner_Fails(OrganizationUserType restoringUserType, Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser restoringUser, + public async Task RestoreUser_AdminRestoreOwner_Fails(OrganizationUserType restoringUserType, + Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser restoringUser, [OrganizationUser(OrganizationUserStatusType.Revoked, OrganizationUserType.Owner)] OrganizationUser organizationUser, SutProvider sutProvider) { restoringUser.Type = restoringUserType; - RestoreRevokeUser_Setup(organization, restoringUser, organizationUser, sutProvider, OrganizationUserType.Admin); + RestoreRevokeUser_Setup(organization, restoringUser, organizationUser, sutProvider); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); @@ -2028,9 +1854,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) SutProvider sutProvider) { organizationUser.Email = null; - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) @@ -2040,6 +1863,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); + var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); @@ -2152,14 +1979,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) SutProvider sutProvider) { organizationUser.Email = null; - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); + var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); @@ -2199,58 +2027,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExcept_WithConfirmedOwner_ReturnsTrue(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new List { owner }); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), true); - - Assert.True(result); - } - - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExcept_ExcludingConfirmedOwner_ReturnsFalse(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new List { owner }); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List { owner.Id }, true); - - Assert.False(result); - } - - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExcept_WithInvitedOwner_ReturnsFalse(Organization organization, [OrganizationUser(OrganizationUserStatusType.Invited, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new List { owner }); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), true); - - Assert.False(result); - } - - [Theory] - [BitAutoData(true)] - [BitAutoData(false)] - public async Task HasConfirmedOwnersExcept_WithConfirmedProviderUser_IncludeProviderTrue_ReturnsTrue(bool includeProvider, Organization organization, ProviderUser providerUser, SutProvider sutProvider) - { - providerUser.Status = ProviderUserStatusType.Confirmed; - - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organization.Id, ProviderUserStatusType.Confirmed) - .Returns(new List { providerUser }); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organization.Id, new List(), includeProvider); - - Assert.Equal(includeProvider, result); - } - [Theory] [BitAutoData(PlanType.TeamsAnnually)] [BitAutoData(PlanType.TeamsMonthly)] @@ -2474,61 +2250,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("custom users can only grant the same custom permissions that they have.", exception.Message.ToLowerInvariant()); } - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExceptAsync_WithConfirmedOwners_ReturnsTrue( - Guid organizationId, - IEnumerable organizationUsersId, - ICollection owners, - SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) - .Returns(owners); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); - - Assert.True(result); - } - - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExceptAsync_WithConfirmedProviders_ReturnsTrue( - Guid organizationId, - IEnumerable organizationUsersId, - ICollection providerUsers, - SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) - .Returns(new List()); - - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed) - .Returns(providerUsers); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); - - Assert.True(result); - } - - [Theory, BitAutoData] - public async Task HasConfirmedOwnersExceptAsync_WithNoConfirmedOwnersOrProviders_ReturnsFalse( - Guid organizationId, - IEnumerable organizationUsersId, - SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organizationId, OrganizationUserType.Owner) - .Returns(new List()); - - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organizationId, ProviderUserStatusType.Confirmed) - .Returns(new List()); - - var result = await sutProvider.Sut.HasConfirmedOwnersExceptAsync(organizationId, organizationUsersId); - - Assert.False(result); - } - [Theory] [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index 81b3fd459..fb08a32f2 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services.Implementations; using Bit.Core.Auth.Entities; @@ -353,6 +354,7 @@ public class PolicyServiceTests }); var organizationService = Substitute.For(); + var removeOrganizationUserCommand = sutProvider.GetDependency(); var utcNow = DateTime.UtcNow; @@ -360,20 +362,20 @@ public class PolicyServiceTests await sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId); - await organizationService.Received() + await removeOrganizationUserCommand.Received() .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWithout2FA.Id, savingUserId); await sutProvider.GetDependency().Received() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserAcceptedWithout2FA.Email); - await organizationService.DidNotReceive() + await removeOrganizationUserCommand.DidNotReceive() .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserInvited.Id, savingUserId); await sutProvider.GetDependency().DidNotReceive() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserInvited.Email); - await organizationService.DidNotReceive() + await removeOrganizationUserCommand.DidNotReceive() .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWith2FA.Id, savingUserId); await sutProvider.GetDependency().DidNotReceive() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserAcceptedWith2FA.Email); - await organizationService.DidNotReceive() + await removeOrganizationUserCommand.DidNotReceive() .RemoveUserAsync(policy.OrganizationId, orgUserDetailAdmin.Id, savingUserId); await sutProvider.GetDependency().DidNotReceive() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailAdmin.Email); @@ -467,6 +469,7 @@ public class PolicyServiceTests }); var organizationService = Substitute.For(); + var removeOrganizationUserCommand = sutProvider.GetDependency(); var savingUserId = Guid.NewGuid(); @@ -475,7 +478,7 @@ public class PolicyServiceTests Assert.Contains("Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - await organizationService.DidNotReceiveWithAnyArgs() + await removeOrganizationUserCommand.DidNotReceiveWithAnyArgs() .RemoveUserAsync(organizationId: default, organizationUserId: default, deletingUserId: default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 098d4d279..480c7b639 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,5 +1,6 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; @@ -263,7 +264,8 @@ public class UserServiceTests sutProvider.GetDependency(), new FakeDataProtectorTokenFactory(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); var actualIsVerified = await sut.VerifySecretAsync(user, secret);