[PM-15052] Add RevokeOrganizationUserCommand (#6111)

src/Api/AdminConsole/Controllers/OrganizationUsersController.cs

@@ -18,7 +18,6 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.Auth.Enums;
 using Bit.Core.Auth.Repositories;
-using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
 using Bit.Core.Billing.Pricing;
 using Bit.Core.Context;
 using Bit.Core.Enums;
@@ -57,7 +56,6 @@ public class OrganizationUsersController : Controller
     private readonly IApplicationCacheService _applicationCacheService;
     private readonly ISsoConfigRepository _ssoConfigRepository;
     private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery;
-    private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
     private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand;
     private readonly IDeleteClaimedOrganizationUserAccountCommand _deleteClaimedOrganizationUserAccountCommand;
     private readonly IGetOrganizationUsersClaimedStatusQuery _getOrganizationUsersClaimedStatusQuery;
@@ -67,9 +65,9 @@ public class OrganizationUsersController : Controller
     private readonly IConfirmOrganizationUserCommand _confirmOrganizationUserCommand;
     private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand;
     private readonly IInitPendingOrganizationCommand _initPendingOrganizationCommand;
+    private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand;
 
-    public OrganizationUsersController(
-        IOrganizationRepository organizationRepository,
+    public OrganizationUsersController(IOrganizationRepository organizationRepository,
         IOrganizationUserRepository organizationUserRepository,
         IOrganizationService organizationService,
         ICollectionRepository collectionRepository,
@@ -85,7 +83,6 @@ public class OrganizationUsersController : Controller
         IApplicationCacheService applicationCacheService,
         ISsoConfigRepository ssoConfigRepository,
         IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery,
-        ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery,
         IRemoveOrganizationUserCommand removeOrganizationUserCommand,
         IDeleteClaimedOrganizationUserAccountCommand deleteClaimedOrganizationUserAccountCommand,
         IGetOrganizationUsersClaimedStatusQuery getOrganizationUsersClaimedStatusQuery,
@@ -94,7 +91,8 @@ public class OrganizationUsersController : Controller
         IPricingClient pricingClient,
         IConfirmOrganizationUserCommand confirmOrganizationUserCommand,
         IRestoreOrganizationUserCommand restoreOrganizationUserCommand,
-        IInitPendingOrganizationCommand initPendingOrganizationCommand)
+        IInitPendingOrganizationCommand initPendingOrganizationCommand,
+        IRevokeOrganizationUserCommand revokeOrganizationUserCommand)
     {
         _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
@@ -112,7 +110,6 @@ public class OrganizationUsersController : Controller
         _applicationCacheService = applicationCacheService;
         _ssoConfigRepository = ssoConfigRepository;
         _organizationUserUserDetailsQuery = organizationUserUserDetailsQuery;
-        _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
         _removeOrganizationUserCommand = removeOrganizationUserCommand;
         _deleteClaimedOrganizationUserAccountCommand = deleteClaimedOrganizationUserAccountCommand;
         _getOrganizationUsersClaimedStatusQuery = getOrganizationUsersClaimedStatusQuery;
@@ -122,6 +119,7 @@ public class OrganizationUsersController : Controller
         _confirmOrganizationUserCommand = confirmOrganizationUserCommand;
         _restoreOrganizationUserCommand = restoreOrganizationUserCommand;
         _initPendingOrganizationCommand = initPendingOrganizationCommand;
+        _revokeOrganizationUserCommand = revokeOrganizationUserCommand;
     }
 
     [HttpGet("{id}")]
@@ -545,7 +543,7 @@ public class OrganizationUsersController : Controller
     [Authorize<ManageUsersRequirement>]
     public async Task RevokeAsync(Guid orgId, Guid id)
     {
-        await RestoreOrRevokeUserAsync(orgId, id, _organizationService.RevokeUserAsync);
+        await RestoreOrRevokeUserAsync(orgId, id, _revokeOrganizationUserCommand.RevokeUserAsync);
     }
 
     [HttpPatch("revoke")]
@@ -553,7 +551,7 @@ public class OrganizationUsersController : Controller
     [Authorize<ManageUsersRequirement>]
     public async Task<ListResponseModel<OrganizationUserBulkResponseModel>> BulkRevokeAsync(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model)
     {
-        return await RestoreOrRevokeUsersAsync(orgId, model, _organizationService.RevokeUsersAsync);
+        return await RestoreOrRevokeUsersAsync(orgId, model, _revokeOrganizationUserCommand.RevokeUsersAsync);
     }
 
     [HttpPatch("{id}/restore")]

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

@@ -0,0 +1,12 @@
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
+
+public interface IRevokeOrganizationUserCommand
+{
+    Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId);
+    Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser);
+    Task<List<Tuple<OrganizationUser, string>>> RevokeUsersAsync(Guid organizationId,
+        IEnumerable<Guid> organizationUserIds, Guid? revokingUserId);
+}

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommand.cs

@@ -0,0 +1,135 @@
+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.Platform.Push;
+using Bit.Core.Repositories;
+using Bit.Core.Services;
+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers;
+
+public class RevokeOrganizationUserCommand(
+    IEventService eventService,
+    IPushNotificationService pushNotificationService,
+    IOrganizationUserRepository organizationUserRepository,
+    ICurrentContext currentContext,
+    IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery)
+    : IRevokeOrganizationUserCommand
+{
+    public async Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId)
+    {
+        if (revokingUserId.HasValue && organizationUser.UserId == revokingUserId.Value)
+        {
+            throw new BadRequestException("You cannot revoke yourself.");
+        }
+
+        if (organizationUser.Type == OrganizationUserType.Owner && revokingUserId.HasValue &&
+            !await currentContext.OrganizationOwner(organizationUser.OrganizationId))
+        {
+            throw new BadRequestException("Only owners can revoke other owners.");
+        }
+
+        await RepositoryRevokeUserAsync(organizationUser);
+        await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked);
+
+        if (organizationUser.UserId.HasValue)
+        {
+            await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
+        }
+    }
+
+    public async Task RevokeUserAsync(OrganizationUser organizationUser,
+        EventSystemUser systemUser)
+    {
+        await RepositoryRevokeUserAsync(organizationUser);
+        await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked,
+            systemUser);
+
+        if (organizationUser.UserId.HasValue)
+        {
+            await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
+        }
+    }
+
+    private async Task RepositoryRevokeUserAsync(OrganizationUser organizationUser)
+    {
+        if (organizationUser.Status == OrganizationUserStatusType.Revoked)
+        {
+            throw new BadRequestException("Already revoked.");
+        }
+
+        if (!await hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId,
+                new[] { organizationUser.Id }, includeProvider: true))
+        {
+            throw new BadRequestException("Organization must have at least one confirmed owner.");
+        }
+
+        await organizationUserRepository.RevokeAsync(organizationUser.Id);
+        organizationUser.Status = OrganizationUserStatusType.Revoked;
+    }
+
+    public async Task<List<Tuple<OrganizationUser, string>>> RevokeUsersAsync(Guid organizationId,
+        IEnumerable<Guid> organizationUserIds, Guid? revokingUserId)
+    {
+        var orgUsers = await organizationUserRepository.GetManyAsync(organizationUserIds);
+        var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId)
+            .ToList();
+
+        if (!filteredUsers.Any())
+        {
+            throw new BadRequestException("Users invalid.");
+        }
+
+        if (!await hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, organizationUserIds))
+        {
+            throw new BadRequestException("Organization must have at least one confirmed owner.");
+        }
+
+        var deletingUserIsOwner = false;
+        if (revokingUserId.HasValue)
+        {
+            deletingUserIsOwner = await currentContext.OrganizationOwner(organizationId);
+        }
+
+        var result = new List<Tuple<OrganizationUser, string>>();
+
+        foreach (var organizationUser in filteredUsers)
+        {
+            try
+            {
+                if (organizationUser.Status == OrganizationUserStatusType.Revoked)
+                {
+                    throw new BadRequestException("Already revoked.");
+                }
+
+                if (revokingUserId.HasValue && organizationUser.UserId == revokingUserId)
+                {
+                    throw new BadRequestException("You cannot revoke yourself.");
+                }
+
+                if (organizationUser.Type == OrganizationUserType.Owner && revokingUserId.HasValue &&
+                    !deletingUserIsOwner)
+                {
+                    throw new BadRequestException("Only owners can revoke other owners.");
+                }
+
+                await organizationUserRepository.RevokeAsync(organizationUser.Id);
+                organizationUser.Status = OrganizationUserStatusType.Revoked;
+                await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked);
+                if (organizationUser.UserId.HasValue)
+                {
+                    await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
+                }
+
+                result.Add(Tuple.Create(organizationUser, ""));
+            }
+            catch (BadRequestException e)
+            {
+                result.Add(Tuple.Create(organizationUser, e.Message));
+            }
+        }
+
+        return result;
+    }
+}

src/Core/AdminConsole/Services/IOrganizationService.cs

@@ -35,10 +35,6 @@ public interface IOrganizationService
         IEnumerable<ImportedOrganizationUser> newUsers, IEnumerable<string> removeUserExternalIds,
         bool overwriteExisting, EventSystemUser eventSystemUser);
     Task DeleteSsoUserAsync(Guid userId, Guid? organizationId);
-    Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId);
-    Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser);
-    Task<List<Tuple<OrganizationUser, string>>> RevokeUsersAsync(Guid organizationId,
-        IEnumerable<Guid> organizationUserIds, Guid? revokingUserId);
     Task ReplaceAndUpdateCacheAsync(Organization org, EventType? orgEvent = null);
     Task<(bool canScale, string failureReason)> CanScaleAsync(Organization organization, int seatsToAdd);
 

src/Core/AdminConsole/Services/Implementations/OrganizationService.cs

@@ -21,7 +21,6 @@ using Bit.Core.Billing.Constants;
 using Bit.Core.Billing.Enums;
 using Bit.Core.Billing.Extensions;
 using Bit.Core.Billing.Pricing;
-using Bit.Core.Billing.Services;
 using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
@@ -44,13 +43,9 @@ public class OrganizationService : IOrganizationService
 {
     private readonly IOrganizationRepository _organizationRepository;
     private readonly IOrganizationUserRepository _organizationUserRepository;
-    private readonly ICollectionRepository _collectionRepository;
     private readonly IGroupRepository _groupRepository;
     private readonly IMailService _mailService;
     private readonly IPushNotificationService _pushNotificationService;
-    private readonly IPushRegistrationService _pushRegistrationService;
-    private readonly IDeviceRepository _deviceRepository;
-    private readonly ILicensingService _licensingService;
     private readonly IEventService _eventService;
     private readonly IApplicationCacheService _applicationCacheService;
     private readonly IPaymentService _paymentService;
@@ -58,7 +53,6 @@ public class OrganizationService : IOrganizationService
     private readonly IPolicyService _policyService;
     private readonly ISsoUserRepository _ssoUserRepository;
     private readonly IGlobalSettings _globalSettings;
-    private readonly IOrganizationApiKeyRepository _organizationApiKeyRepository;
     private readonly ICurrentContext _currentContext;
     private readonly ILogger<OrganizationService> _logger;
     private readonly IProviderOrganizationRepository _providerOrganizationRepository;
@@ -75,13 +69,9 @@ public class OrganizationService : IOrganizationService
     public OrganizationService(
         IOrganizationRepository organizationRepository,
         IOrganizationUserRepository organizationUserRepository,
-        ICollectionRepository collectionRepository,
         IGroupRepository groupRepository,
         IMailService mailService,
         IPushNotificationService pushNotificationService,
-        IPushRegistrationService pushRegistrationService,
-        IDeviceRepository deviceRepository,
-        ILicensingService licensingService,
         IEventService eventService,
         IApplicationCacheService applicationCacheService,
         IPaymentService paymentService,
@@ -89,7 +79,6 @@ public class OrganizationService : IOrganizationService
         IPolicyService policyService,
         ISsoUserRepository ssoUserRepository,
         IGlobalSettings globalSettings,
-        IOrganizationApiKeyRepository organizationApiKeyRepository,
         ICurrentContext currentContext,
         ILogger<OrganizationService> logger,
         IProviderOrganizationRepository providerOrganizationRepository,
@@ -106,13 +95,9 @@ public class OrganizationService : IOrganizationService
     {
         _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
-        _collectionRepository = collectionRepository;
         _groupRepository = groupRepository;
         _mailService = mailService;
         _pushNotificationService = pushNotificationService;
-        _pushRegistrationService = pushRegistrationService;
-        _deviceRepository = deviceRepository;
-        _licensingService = licensingService;
         _eventService = eventService;
         _applicationCacheService = applicationCacheService;
         _paymentService = paymentService;
@@ -120,7 +105,6 @@ public class OrganizationService : IOrganizationService
         _policyService = policyService;
         _ssoUserRepository = ssoUserRepository;
         _globalSettings = globalSettings;
-        _organizationApiKeyRepository = organizationApiKeyRepository;
         _currentContext = currentContext;
         _logger = logger;
         _providerOrganizationRepository = providerOrganizationRepository;
@@ -1453,122 +1437,6 @@ public class OrganizationService : IOrganizationService
         return true;
     }
 
-    public async Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId)
-    {
-        if (revokingUserId.HasValue && organizationUser.UserId == revokingUserId.Value)
-        {
-            throw new BadRequestException("You cannot revoke yourself.");
-        }
-
-        if (organizationUser.Type == OrganizationUserType.Owner && revokingUserId.HasValue &&
-            !await _currentContext.OrganizationOwner(organizationUser.OrganizationId))
-        {
-            throw new BadRequestException("Only owners can revoke other owners.");
-        }
-
-        await RepositoryRevokeUserAsync(organizationUser);
-        await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked);
-
-        if (organizationUser.UserId.HasValue)
-        {
-            await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
-        }
-    }
-
-    public async Task RevokeUserAsync(OrganizationUser organizationUser,
-        EventSystemUser systemUser)
-    {
-        await RepositoryRevokeUserAsync(organizationUser);
-        await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked,
-            systemUser);
-
-        if (organizationUser.UserId.HasValue)
-        {
-            await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
-        }
-    }
-
-    private async Task RepositoryRevokeUserAsync(OrganizationUser organizationUser)
-    {
-        if (organizationUser.Status == OrganizationUserStatusType.Revoked)
-        {
-            throw new BadRequestException("Already revoked.");
-        }
-
-        if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId,
-                new[] { organizationUser.Id }, includeProvider: true))
-        {
-            throw new BadRequestException("Organization must have at least one confirmed owner.");
-        }
-
-        await _organizationUserRepository.RevokeAsync(organizationUser.Id);
-        organizationUser.Status = OrganizationUserStatusType.Revoked;
-    }
-
-    public async Task<List<Tuple<OrganizationUser, string>>> RevokeUsersAsync(Guid organizationId,
-        IEnumerable<Guid> organizationUserIds, Guid? revokingUserId)
-    {
-        var orgUsers = await _organizationUserRepository.GetManyAsync(organizationUserIds);
-        var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId)
-            .ToList();
-
-        if (!filteredUsers.Any())
-        {
-            throw new BadRequestException("Users invalid.");
-        }
-
-        if (!await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationId, organizationUserIds))
-        {
-            throw new BadRequestException("Organization must have at least one confirmed owner.");
-        }
-
-        var deletingUserIsOwner = false;
-        if (revokingUserId.HasValue)
-        {
-            deletingUserIsOwner = await _currentContext.OrganizationOwner(organizationId);
-        }
-
-        var result = new List<Tuple<OrganizationUser, string>>();
-
-        foreach (var organizationUser in filteredUsers)
-        {
-            try
-            {
-                if (organizationUser.Status == OrganizationUserStatusType.Revoked)
-                {
-                    throw new BadRequestException("Already revoked.");
-                }
-
-                if (revokingUserId.HasValue && organizationUser.UserId == revokingUserId)
-                {
-                    throw new BadRequestException("You cannot revoke yourself.");
-                }
-
-                if (organizationUser.Type == OrganizationUserType.Owner && revokingUserId.HasValue &&
-                    !deletingUserIsOwner)
-                {
-                    throw new BadRequestException("Only owners can revoke other owners.");
-                }
-
-                await _organizationUserRepository.RevokeAsync(organizationUser.Id);
-                organizationUser.Status = OrganizationUserStatusType.Revoked;
-                await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked);
-                if (organizationUser.UserId.HasValue)
-                {
-                    await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value);
-                }
-
-                result.Add(Tuple.Create(organizationUser, ""));
-            }
-            catch (BadRequestException e)
-            {
-                result.Add(Tuple.Create(organizationUser, e.Message));
-            }
-        }
-
-        return result;
-    }
-
     public static OrganizationUserStatusType GetPriorActiveOrganizationUserStatusType(OrganizationUser organizationUser)
     {
         // Determine status to revert back to

src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs

@@ -127,6 +127,7 @@ public static class OrganizationServiceCollectionExtensions
     {
         services.AddScoped<IRemoveOrganizationUserCommand, RemoveOrganizationUserCommand>();
         services.AddScoped<IRevokeNonCompliantOrganizationUserCommand, RevokeNonCompliantOrganizationUserCommand>();
+        services.AddScoped<IRevokeOrganizationUserCommand, RevokeOrganizationUserCommand>();
         services.AddScoped<IUpdateOrganizationUserCommand, UpdateOrganizationUserCommand>();
         services.AddScoped<IUpdateOrganizationUserGroupsCommand, UpdateOrganizationUserGroupsCommand>();
         services.AddScoped<IDeleteClaimedOrganizationUserAccountCommand, DeleteClaimedOrganizationUserAccountCommand>();