diff --git a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs index 4d292281dd..afbfa50bb4 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs @@ -6,7 +6,6 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Scim.Models; using Bit.Scim.Users.Interfaces; using Bit.Scim.Utilities; @@ -22,29 +21,28 @@ namespace Bit.Scim.Controllers.v2; public class UsersController : Controller { private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IOrganizationService _organizationService; private readonly IGetUsersListQuery _getUsersListQuery; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IPatchUserCommand _patchUserCommand; private readonly IPostUserCommand _postUserCommand; private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; + private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand; - public UsersController( - IOrganizationUserRepository organizationUserRepository, - IOrganizationService organizationService, + public UsersController(IOrganizationUserRepository organizationUserRepository, IGetUsersListQuery getUsersListQuery, IRemoveOrganizationUserCommand removeOrganizationUserCommand, IPatchUserCommand patchUserCommand, IPostUserCommand postUserCommand, - IRestoreOrganizationUserCommand restoreOrganizationUserCommand) + IRestoreOrganizationUserCommand restoreOrganizationUserCommand, + IRevokeOrganizationUserCommand revokeOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; - _organizationService = organizationService; _getUsersListQuery = getUsersListQuery; _removeOrganizationUserCommand = removeOrganizationUserCommand; _patchUserCommand = patchUserCommand; _postUserCommand = postUserCommand; _restoreOrganizationUserCommand = restoreOrganizationUserCommand; + _revokeOrganizationUserCommand = revokeOrganizationUserCommand; } [HttpGet("{id}")] @@ -101,7 +99,7 @@ public class UsersController : Controller } else if (!model.Active && orgUser.Status != OrganizationUserStatusType.Revoked) { - await _organizationService.RevokeUserAsync(orgUser, EventSystemUser.SCIM); + await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM); } // Have to get full details object for response model diff --git a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs index 3d7082aacc..6c983611ee 100644 --- a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs +++ b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs @@ -1,8 +1,8 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Scim.Models; using Bit.Scim.Users.Interfaces; @@ -11,20 +11,19 @@ namespace Bit.Scim.Users; public class PatchUserCommand : IPatchUserCommand { private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IOrganizationService _organizationService; private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; private readonly ILogger _logger; + private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand; - public PatchUserCommand( - IOrganizationUserRepository organizationUserRepository, - IOrganizationService organizationService, + public PatchUserCommand(IOrganizationUserRepository organizationUserRepository, IRestoreOrganizationUserCommand restoreOrganizationUserCommand, - ILogger logger) + ILogger logger, + IRevokeOrganizationUserCommand revokeOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; - _organizationService = organizationService; _restoreOrganizationUserCommand = restoreOrganizationUserCommand; _logger = logger; + _revokeOrganizationUserCommand = revokeOrganizationUserCommand; } public async Task PatchUserAsync(Guid organizationId, Guid id, ScimPatchModel model) @@ -80,7 +79,7 @@ public class PatchUserCommand : IPatchUserCommand } else if (!active && orgUser.Status != OrganizationUserStatusType.Revoked) { - await _organizationService.RevokeUserAsync(orgUser, EventSystemUser.SCIM); + await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM); return true; } return false; diff --git a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs index 44a43d16b7..f391c93fe3 100644 --- a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs +++ b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs @@ -1,10 +1,10 @@ using System.Text.Json; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Scim.Models; using Bit.Scim.Users; using Bit.Scim.Utilities; @@ -101,7 +101,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RevokeUserAsync(organizationUser, EventSystemUser.SCIM); + await sutProvider.GetDependency().Received(1).RevokeUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -129,7 +129,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RevokeUserAsync(organizationUser, EventSystemUser.SCIM); + await sutProvider.GetDependency().Received(1).RevokeUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -149,7 +149,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreUserAsync(default, EventSystemUser.SCIM); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RevokeUserAsync(default, EventSystemUser.SCIM); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RevokeUserAsync(default, EventSystemUser.SCIM); } [Theory] diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 55f1c9de14..3365e754ca 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/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] 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] public async Task> BulkRevokeAsync(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) { - return await RestoreOrRevokeUsersAsync(orgId, model, _organizationService.RevokeUsersAsync); + return await RestoreOrRevokeUsersAsync(orgId, model, _revokeOrganizationUserCommand.RevokeUsersAsync); } [HttpPatch("{id}/restore")] diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeOrganizationUserCommand.cs new file mode 100644 index 0000000000..01ad2f05d2 --- /dev/null +++ b/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>> RevokeUsersAsync(Guid organizationId, + IEnumerable organizationUserIds, Guid? revokingUserId); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommand.cs new file mode 100644 index 0000000000..f24e0ae265 --- /dev/null +++ b/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>> RevokeUsersAsync(Guid organizationId, + IEnumerable 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>(); + + 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; + } +} diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index bec9507adf..05c84c731c 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -35,10 +35,6 @@ public interface IOrganizationService IEnumerable newUsers, IEnumerable removeUserExternalIds, bool overwriteExisting, EventSystemUser eventSystemUser); Task DeleteSsoUserAsync(Guid userId, Guid? organizationId); - Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId); - Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); - Task>> RevokeUsersAsync(Guid organizationId, - IEnumerable organizationUserIds, Guid? revokingUserId); Task ReplaceAndUpdateCacheAsync(Organization org, EventType? orgEvent = null); Task<(bool canScale, string failureReason)> CanScaleAsync(Organization organization, int seatsToAdd); diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 84fb9532dc..b1a07338a3 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/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 _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 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>> RevokeUsersAsync(Guid organizationId, - IEnumerable 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>(); - - 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 diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index e28831e0ab..998354b9f8 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -127,6 +127,7 @@ public static class OrganizationServiceCollectionExtensions { services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommandTests.cs new file mode 100644 index 0000000000..b16a80d7a2 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeOrganizationUserCommandTests.cs @@ -0,0 +1,83 @@ +using Bit.Core.AdminConsole.Entities; +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.Platform.Push; +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; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class RevokeOrganizationUserCommandTests +{ + + [Theory, BitAutoData] + public async Task RevokeUser_Success( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser] OrganizationUser organizationUser, + SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); + + await sutProvider.Sut.RevokeUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + + [Theory, BitAutoData] + public async Task RevokeUser_WithEventSystemUser_Success( + Organization organization, + [OrganizationUser] OrganizationUser organizationUser, + EventSystemUser eventSystemUser, + SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); + + await sutProvider.Sut.RevokeUserAsync(organizationUser, eventSystemUser); + + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked, eventSystemUser); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + + 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().OrganizationOwner(organization.Id).Returns(requestingOrganizationUser != null && requestingOrganizationUser.Type is OrganizationUserType.Owner); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organization.Id, Arg.Any>()) + .Returns(true); + } +} diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 3271ea559b..923eaae871 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -15,7 +15,6 @@ using Bit.Core.Models.Business; using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; -using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; @@ -961,62 +960,6 @@ public class OrganizationServiceTests Assert.Contains("Seat limit has been reached. Contact your provider to purchase additional seats.", failureMessage); } - 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().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] - public async Task RevokeUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - await sutProvider.Sut.RevokeUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RevokeAsync(organizationUser.Id); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); - await sutProvider.GetDependency() - .Received(1) - .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); - } - - [Theory, BitAutoData] - public async Task RevokeUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); - - await sutProvider.Sut.RevokeUserAsync(organizationUser, eventSystemUser); - - await sutProvider.GetDependency() - .Received(1) - .RevokeAsync(organizationUser.Id); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked, eventSystemUser); - await sutProvider.GetDependency() - .Received(1) - .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); - } [Theory] [BitAutoData(PlanType.TeamsAnnually)]