[AC-607] Extract IOrganizationService.DeleteUserAsync into IRemoveOrganizationUserCommand (#4803)

* 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

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)

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<bool> HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable<Guid> 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<IEnumerable<OrganizationUser>> GetConfirmedOwnersAsync(Guid organizationId)
+    {
+        var owners = await _organizationUserRepository.GetManyByOrganizationAsync(organizationId,
+            OrganizationUserType.Owner);
+        return owners.Where(o => o.Status == OrganizationUserStatusType.Confirmed);
+    }
+}

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IHasConfirmedOwnersExceptQuery.cs

@@ -0,0 +1,12 @@
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
+
+public interface IHasConfirmedOwnersExceptQuery
+{
+    /// <summary>
+    /// Checks if an organization has any confirmed owners except for the ones in the <paramref name="organizationUsersId"/> list.
+    /// </summary>
+    /// <param name="organizationId">The organization ID.</param>
+    /// <param name="organizationUsersId">The organization user IDs to exclude.</param>
+    /// <param name="includeProvider">Whether to include the provider users in the count.</param>
+    Task<bool> HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable<Guid> organizationUsersId, bool includeProvider = true);
+}

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<List<Tuple<OrganizationUser, string>>> RemoveUsersAsync(Guid organizationId,
+        IEnumerable<Guid> organizationUserIds, Guid? deletingUserId);
 }

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<List<Tuple<OrganizationUser, string>>> RemoveUsersAsync(Guid organizationId,
+        IEnumerable<Guid> 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<Tuple<OrganizationUser, string>>();
+        var deletedUserIds = new List<Guid>();
+        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<IEnumerable<KeyValuePair<string, DeviceType>>> GetUserDeviceIdsAsync(Guid userId)
+    {
+        var devices = await _deviceRepository.GetManyByUserIdAsync(userId);
+        return devices
+            .Where(d => !string.IsNullOrWhiteSpace(d.PushToken))
+            .Select(d => new KeyValuePair<string, DeviceType>(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);
+    }
 }

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;
     }
 
     /// <summary>
@@ -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.");
         }